Fix: Inconsistent type vars in "unbound type var" message#11194
Fix: Inconsistent type vars in "unbound type var" message#11194voodoos wants to merge 4 commits intoocaml:trunkfrom
Conversation
The type var should stay coherent in a same error message. This is not the case for "variables unbound" messages.
voodoos
changed the title
|
Hi @Octachron not sure if you are the correct person to ping, sorry if that is not the case :-) |
|
Thanks for the reminder! I have added the PR to my review stack for next week. |
| let ty1 = | ||
| if real then ty0 else Btype.newgenty(Tobject(ty0, ref None)) | ||
| in | ||
| Printtyp.prepare_for_printing [ty; ty1]; |
There was a problem hiding this comment.
Moving this preparation here makes it a no-op: printer is either Printtyp.class_declaration or Printtyp.cltype_declaration and both of them call Printtyp.reset_context undoing the work done by this call.
Moreover, this matters because there is at least one case where Printtyp.class_declaration does not print itself the method type and thus may not mark cycles which may lead to a stack overflow in the printer(with this PR):
class ['a] c = object method m: (<x:'a; f:'b> as 'b) -> unit = fun _ -> () end
class d = ['a] cThere was a problem hiding this comment.
With the way that the printing classes are wired, I fear that the easiest solution would be to add an optional ?(reset=true) argument to Printtyp.prepare_for_printing and use it in the suberror message printer.
There was a problem hiding this comment.
Ok, thanks for your review and these insights, I will give it more thoughts.
|
@voodoos : did you have the time to work on this PR? If not would you mind if I submitted my version of the fix? |
|
I didn't sadly. Feel free to close this and propose your own :-) |
…_context #11194: fix an inconsistency in type variable names in error messages
|
Superseded by #11609 . |
2 participants
When upgrading Merlin for 4.14 our test-suite catched the following issue:
In this example the printed type for
method bis inconsistent.This is due to an extraneous reset of the printer introduced in #10488
This PR add a test illustrating the issue and a fix proposition.