Conflicting error message whether to be or not to be a function

[sim642]

Problem

While working on a ppx deriver and trying to figure out where I need to insert Ptyp_polys to make the generated code typecheck, I happened on this conflicting error message:

Error: This expression should not be a function, the expected type is
       Ppx_deriving_ord_helper.t -> Ppx_deriving_runtime.int

It's saying that the (function ...) expression should be a function (i.e. of a function type), but at the same time not be a function!

When I printed out the generated code and tried compiling that directly, the error suddenly disappeared, confusing things even further, so I cannot even provide simple reproducible example code for it.

Hypothesis

Since I was messing around with Ptyp_poly, I realized I'm inserting some with empty lists for type parameters. Avoiding those made the confusing error disappear.
I'm not familiar with the compiler code, but I have a hypothesis how this happened.

Here Tpoly is completely unhandled, so the Not_a_function error is raised:

ocaml/typing/ctype.ml

Lines 3225 to 3237 in d0eba6b

	match get_desc t with
	| Tvar _ ->
	let t', t1, t2 = function_type (get_level t) in
	link_type t t';
	(t1, t2)
	| Tarrow(l', t1, t2, _) ->
	if l = l' || !Clflags.classic && l = Nolabel && not (is_optional l')
	then (t1, t2)
	else raise (Filter_arrow_failed
	(Label_mismatch
	{ got = l; expected = l'; expected_type = t }))
	| _ ->
	raise (Filter_arrow_failed Not_a_function)

And when printing the expected type for the error message, the Tpoly with empty type parameter list is quietly thrown away:

ocaml/typing/printtyp.ml

Lines 1153 to 1154 in d0eba6b

	| Tpoly (ty, []) ->
	tree_of_typexp mode ty

Similarly, Pprintast ignores them, which is why manually compiling the generated and printed code suddenly worked:

ocaml/parsing/pprintast.ml

Lines 306 to 307 in d0eba6b

	| Ptyp_poly ([], ct) ->
	core_type ctxt f ct

Solutions

I can think of the following things that could be changed:

1. Also handle Tpoly (_, []) in Ctype.filter_arrow.
2. Somehow distinguish Tpoly (ty, []) in error message types.

[Octachron]

Producing an invalid AST is a programming mistake on the ppx extension side.
The typechecker is already complex enough without handling such broken ASTs.

The right fix might be to make the ast_invariant check rejects parsetrees with invalid Tpoly nodes.

[sim642]

Are such ASTs supposed to be invalid though? Because when looking around, there's lots of other places where Tpoly (_, []) is being handled. And there's at least one place in the parser where Ptyp_poly ([], _) is introduced. So it isn't entirely apparent that these are invalid, but it's also unclear what these are supposed to mean.

[Octachron]

Yes, those parsetree ASTs are invalid.

The single use of Ptyp_poly([], ...) in the parser is simulating a location stack when duplicating a type constraint. Nowadays, it should be replaced by a call to reloc_typ.

The typechecker introduces internally such nodes, but that is even more reason to not let ppxs introduce those nodes erroneously.

[github-actions]

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

[Octachron]

See #12119 and #11148 that will raise a better error message on invalid Ptyp_poly nodes.