-
Notifications
You must be signed in to change notification settings - Fork 257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test ResolutionErrors[45679] with new resolver #5333
Test ResolutionErrors[45679] with new resolver #5333
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that the bug fixes ultimately feel like simplifications.
The couple of reductions in type inference power are slightly unfortunate, but the workarounds seem like better style to me, so I'm not really worried about them.
Pull Request is not mergeable
Pull Request is not mergeable
Pull Request is not mergeable
Pull Request is not mergeable
Pull Request is not mergeable
Pull Request is not mergeable
This PR changes the remaining 5 of the 10
dafny0/ResolutionErrors*.dfy
tests so that they test not only the legacy resolver, but also the resolver refresh.this
from the scope when resolvingstatic const
fieldsthis
from the scope when resolving type constraints and type witnessesdafny0/ResolutionErrors[45679].dfy
TernaryExpr.PrefixEqUsesNat
, which wasn't neededRegression
The one place where the new type inference was not able to do as well as the old is the code snippet
from
dafny0/ResolutionErrors4.dfy
. Here, the new resolver does not thread the element type ofd
through the array-to-sequence conversionc[25..50]
, and therefore the.x
gives an error.The workaround is to declare the type of
c
explicitly in the code.I don't see a quick fix for this in the type inference. A good improvement, which I believe would also fix this problem, is to do member resolution (like
.x
) lazily instead of eagerly. This would allow the type-inference machinery to start making some decisions, which will lead to it figuring out a type forc
, after which types for the other expressions, including the.x
, can be inferred. I would love to see this improvement in the type inference, and the new resolver has "guarded constraints" that exist in order to lazy inference. However, such a change will take some effort, because the methods that currently try to do member lookup too easily generate errors (rather than saying "please try again later"). Such a change is out of scope for this PR.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.