Misleading comment in cparser/StructPassing.ml

[sixcy]

In the case of a struct passing using SP_split_args, this code of cparser/StructPassing.ml is actually misleading

let classify_param env ty =
  if is_composite_type env ty then begin
    match !struct_passing_style with
    | SP_ref_callee -> Param_unchanged
    | SP_ref_caller -> Param_ref_caller
    | _ ->
      match sizeof env ty, alignof env ty with
      | Some sz, Some al ->
          Param_flattened ((sz + 3) / 4, sz, al)
      | _, _ ->
          Param_unchanged  (* should not happen *)
  end else
Param_unchanged

The should not happen part actually happens for the test in test/regression/struct2.c :

struct B;
int f(struct B);
struct B { double d; };
int g() { struct B b; return f(b); }

This can be tested by replacing Param_unchanged (*should not happen *) by a failwith "Should not happen", then attempting to compile test/regression/struct2.c, on the x86_32-linux architecture.

[sixcy]

Thankfully, at first glance there seems to be no hidden bug.

As I understand it, when parsing int f(struct B), this classify_param is called - since no information is available yet for struct B, the sizeof and alignof return None. classify_param is then returning Param_unchanged.

However, classify_param is called again when parsing f(b), where the structure has this time the relevant information available. So there seems to be no misbehaviour here.

[xavierleroy]

You,re right that the comment was written with function definitions in mind (where incomplete structs cannot occur) and doesn't match what happens with function declarations (as in your example). At least a better comment is in order.