A principled approach to erasing typedefs from C types (and canonicalising C types)

[jorisdral]

Resolves #1142.
Resolves #1050.

This PR builds on top of PR #1139, so that one should be merged first.

This PR is an experiment to come up with a more principle approach to erasing typedefs from C types where necessary. One such example occurs when we want to check if a type is a function type so that we can decide whether to use Ptr or FunPtr in the Haskell code for the bindings. To do that reliably we should erase typedefs and const qualifiers.

Most of the effort in this PR is related to adding some type safety when inspecting C types. Meaning, if you want to check that a type is a canonical function type, one would first map the full C type to a canonical type and then check if it is a function type. There is currently nothing preventing us from inspecting the full C type when we should be inspecting the corresponding canonical C type instead, but the types should hopefully make it clearer that we should do the mapping before the inspecting.

There are two commits in this PR, each with a separate attempt at implementing this. Attempt 1 was with higher-kinded datatypes, which I didn't like so much, and it is not so complete as Attempt 2. Attempt 2 uses a Trees That Grow style, though it's more like a Trees That Shrink style.

[bolt12]

#1139 merged :)

[edsko]

Nice, feel free to merge this once the changes we discussed are done.

For the record, dealing correctly with macros is outsourced to #1200.

[jorisdral]

Comments from a review together with @edsko

[jorisdral]

Suggested change

	C.TypeTypedef (TypedefRefRegular nm uTy) -> do
	(uTyDepIds, uTy') <- resolve uTy
	let mkTypeTypedef = C.TypeTypedef . flip TypedefRefRegular uTy'
	(tdDepIds, td) <- auxN mkTypeTypedef nm C.NameKindOrdinary
	pure (uTyDepIds `Set.union` tdDepIds, td)
	C.TypeTypedef (TypedefRefRegular nm uTy) -> do
	(_uTyDepIds, uTy') <- resolve uTy
	let mkTypeTypedef = C.TypeTypedef . flip TypedefRefRegular uTy'
	(tdDepIds, td) <- auxN mkTypeTypedef nm C.NameKindOrdinary
	pure (tdDepIds, td)

[jorisdral]

@TravisCardwell does it make sense that we do not include the dependencies of the underlying type in the result?

[jorisdral]

Some context: references to typedefs are annotated with the type Type p underlying the typedef. Applying pass p' after p, we still have to convert the annotation from Type p to Type p'. We do this here by calling resolve on the annotation, but not returning the dependencies.

[TravisCardwell]

The Set C.QualPrelimDeclId returned are dependencies of the declaration currently being processed by resolveDeep, as represented by edges in the UseDeclGraph.

Example:

typedef struct point {
    double x;
    double y;
} point;

struct rect {
    point tl;
    point br;
};

graph TD;
  v1["PrelimDeclIdNamed "point""]
  v0["PrelimDeclIdNamed "point""]
  v2["PrelimDeclIdNamed "rect""]
  v1-->|"UsedInTypedef ByValue"|v0
  v2-->|"UsedInField ByValue "br""|v1
  v2-->|"UsedInField ByValue "tl""|v1

Loading

Imagine the resolveDeep call for the struct rect declaration. The two fields are typedefs, so they are processed by this code. Why are we recursing on the underlying type? I do not think doing so is correct, as we are processing the use sites of external bindings in struct rect, not type typedefs themselves.

Returning the dependencies of the underlying type is indeed not necessary, as there are no edges between PrelimDeclIdNamed "rect" and the bottom PrelimDeclIdNamed "point".

Hmmm, that UseDeclGraph does not distinguish between struct point and point. Perhaps it is an issue in what we are displaying? Hopefully it is not a bug.

BTW, I implemented the hs-bindgen-cli info use-decl-graph command in order to get the above. I will throw it into a PR really quick so that you can use it as well.

[edsko]

Not traversing isn't really an option, unless we do some type trickery. Perhaps we should have a chat about this with the three of us.

[jorisdral]

I don't understand the resolve bindings spec pass well enough to understand why it would be incorrect to recurse into the underlying type. Is it harmful? We're mostly piggy-backing off of the recursive call to resolve so that we get a Type ResolveBindingSpecs from Type NameAnon

[TravisCardwell]

Ack! Recursing on the underlying type is indeed needed when processing the typedef itself, such as point in the above example. 🙇