Fix srtp constraint printing #9728
Conversation
Corrected wrong formatted print of a trait member constraint Signed-off-by: realvictorprm <[email protected]>
Signed-off-by: realvictorprm <[email protected]>
Signed-off-by: realvictorprm <[email protected]>
Signed-off-by: realvictorprm <[email protected]>
|
Hmm, I'm now suddenly locally getting this: Haven't seen it before in this solution. I understand the error, but the file is auto-generated. Wonder if it has something to do with me merging 2 years of stuff. Hope CI doesn't see this ;). EDIT: |
|
Some errors like the following, but these were expected, as these tests were added after this change and are related to the stringification of constraints. I'll look into them - - static member Neg117.Superpower.Transformer.Transform : ^f * Neg117.TargetB.TargetB * Neg117.Superpower.Transformer -> (Neg117.TargetB.TransformerKind -> ^f) when (Neg117.TargetB.TargetB or ^f) : (static member Transform : ^f * Neg117.TargetB.TargetB -> Neg117.TargetB.TransformerKind -> ^f) // Argument at index 1 doesn't match
+ - static member Neg117.Superpower.Transformer.Transform : ^f * Neg117.TargetB.TargetB * Neg117.Superpower.Transformer -> (Neg117.TargetB.TransformerKind -> ^f) when (Neg117.TargetB.TargetB or ^f) : (static member Transform : ^f * Neg117.TargetB.TargetB -> (Neg117.TargetB.TransformerKind -> ^f)) // Argument at index 1 doesn't match
diff between [D:\a\1\s\tests\fsharp\typecheck\sigs\neg117.bsl] and [D:\a\1\s\tests\fsharp\typecheck\sigs\neg117.err]
line 10 |
|
The relevant part of the above changed message is: - when (Neg117.TargetB.TargetB or ^f) : (static member Transform : ^f * Neg117.TargetB.TargetB -> Neg117.TargetB.TransformerKind -> ^f)
+ when (Neg117.TargetB.TargetB or ^f) : (static member Transform : ^f * Neg117.TargetB.TargetB -> (Neg117.TargetB.TransformerKind -> ^f)) Note the extra parens that now correctly indicate a function is returned. It now mimics the tooltip:
Same for the other failed test, and a fix to pick the correct directory for the new test. |
|
Interesting, with the 2 years worth of merging, instead of 3x a version of the FS0001 type error, it now has 1x FS0001 and 2x FS0043, which reminded me of this old issue: #3508. I'm however at a loss why the error is mentioned 3x in the
(the 3x reporting was already like this when the PR was first created, except that the error numbers are different now, and this is not related to this PR per se, this PR only fixes the correct signature for the overloads, or missing constraints). This makes me wonder what the difference is between FS0001 and FS0043, considering the texts are equal (as also observed in #3508). Maybe we're issuing the wrong error number with the right text, or vv? |
|
Green, @cartermp, this is ready. @realvictorprm, do you want to review this? Or you think it's good as is? |
* Fix dotnet#5768 Corrected wrong formatted print of a trait member constraint Signed-off-by: realvictorprm <[email protected]> * Adding regression test. Signed-off-by: realvictorprm <[email protected]> * Fix test to expect an error. Signed-off-by: realvictorprm <[email protected]> * Apply review, move and add new test to typecheck\sigs. Signed-off-by: realvictorprm <[email protected]> * Rename neg111 to neg112 to ease merging * Bring layoutReturnType into scope, and use instead of layoutTypeWithInfoAndPrec * Fix test neg112 to use proper directory * Fix tests that suggest overloads to use parens around returned functions * Mention correct file and col numbers in new neg112 test Co-authored-by: realvictorprm <[email protected]>
Fixes #5768
TLDR: srtp contraints returning a function did not get pretty-printed for overloads-suggestions like
'a -> 'b -> ('x -> 'y), instead, you'd see'a -> 'b -> 'x -> 'y. That is, the parens were omitted if a function was returned. Same is true for type or constraint missing errors.This continues #5948. It was 2 years behind master, and creating a new PR seemed a simpler way to get this done. The changes w.r.t. that PR are small:
neg111toneg112andneg111.fstoneg112.fsandneg111.bsltoneg112.bslLet's see what CI makes of this now ;).
@realvictorprm, this was originally your PR and I branched off your branch to keep the history intact. I hope it's ok that I try to continue & finish it ;). Since I cannot make changes to an existing PR, I decided to make a new one. I don't claim to know all details of your fix, but from the look of it, it seemed quite straight-forward and it was already reviewed & accepted as-is.