Add a hint suggesting to use ! when a 'a ref is provided but a 'a was expected#1542
Add a hint suggesting to use ! when a 'a ref is provided but a 'a was expected#1542Armael wants to merge 4 commits intoocaml:trunkfrom
! when a 'a ref is provided but a 'a was expected#1542Conversation
! when a 'a ref is provided but a 'a was expected! when a 'a ref is provided but a 'a was expected
|
While I like the idea (I haven't look at the implementation yet), one thing I'm a bit worried is the case where Is it possible to check that |
|
Good point. What should we do if it turns out |
|
The message could also suggest |
Just to be pedantic here, |
4da70c2 to
8a0bbd1
Compare
|
I added a commit that disambiguates |
8a0bbd1 to
90d3079
Compare
|
Any news about this? I think I implemented the suggestions above (I think this still needs review tho, especially wrt the hack mentioned in my initial post). |
|
|
||
| let is_ref_path p = | ||
| match p with | ||
| | Pdot (Pident id, "ref", _) -> Ident.same id ident_pervasives |
There was a problem hiding this comment.
I believe that is whatnon_shadowed_pervasive is for, and it's more compatible with the Stdlib PR.
| longident lid expected provided | ||
| | Label_mismatch(lid, trace) -> | ||
| report_unification_error ppf env trace | ||
| ~explain_mismatch:false |
There was a problem hiding this comment.
If I understand correctly, this commit disables all explanations for mismatched labels (ie., wrongly used record fields). This is much more than what the commit message says, and I'm not sure why this was done at all. Do you have a example where the explanation end up wrong for record fields?
There was a problem hiding this comment.
See the second part of my first message and the second quoted block: it happens in a test of the testsuite. As I said midly, "it feels a bit hackish".
| Error: This expression has type int ref | ||
| but an expression was expected of type int | ||
| Hint: Did you forget to use `!' to get the content of a reference somewhere? | ||
| |}];; |
There was a problem hiding this comment.
Can you add a small test for the cases where either ! or the ref types are redefined?
|
I'll ease the job of the maintainers and close this one... |
This is an other small bit extracted from #102 . This PR adds a hint "did you forget to use !" as an explanation for a type mismatch between
'a refand'a.The first commit is as one might expect.
I'm less confident about the way the second commit is implemented. The idea is to disable the hint in the following case:
This case appears in the
explanationfunction as a mismatch between'a refandt(for labelcontents), but for a label mismatch the!hint obviously does not make sense.The current patch does disable the hint in that case, but it feels a bit hackish.