-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
extend stable hasher to support CanonicalTy
#49091
Conversation
51bb574
to
f02dc74
Compare
FloatVar(a), | ||
FreshTy(a), | ||
FreshIntTy(a), | ||
FreshFloatTy(a), |
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.
The Fresh*
variants are valid to hash?
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.
Yes.
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.
They are not confined to the local tcx:
Lines 112 to 124 in 3b6412b
match infer { | |
ty::FreshTy(_) | | |
ty::FreshIntTy(_) | | |
ty::FreshFloatTy(_) | | |
ty::CanonicalTy(_) => { | |
self.add_flags(TypeFlags::HAS_CANONICAL_VARS); | |
} | |
ty::TyVar(_) | | |
ty::IntVar(_) | | |
ty::FloatVar(_) => { | |
self.add_flags(TypeFlags::KEEP_IN_LOCAL_TCX) | |
} |
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.
It just happens that we don't ever put them in query keys right now (and probably never will, but whatever).
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.
(To be honest, even the TyVar
and so forth would be ok to hash -- in the sense that they are "stable-ish" -- it's just that it truly ought to be impossible... it would indicate some other problem if we did see them that would be worth knowing about.)
Well, I take that back. It's a bit dubious given that their meaning depends on the surrounding infcx. Anyway, ought to be impossible. =)
Thanks for the explanation! @bors r+ |
📌 Commit f02dc74 has been approved by |
@bors p=1 -- important regression to button down |
⌛ Testing commit f02dc74 with merge a5bc1141a0683804890d19bea6a08e3e6d9a0dfb... |
💔 Test failed - status-travis |
@bors retry
|
⌛ Testing commit f02dc74 with merge 86091adfe8e5752967963ee8417ce1aa8898cf1a... |
💔 Test failed - status-travis |
@bors retry |
⌛ Testing commit f02dc74 with merge 678f786e713c168cbfcd6f1324202514cec7ef45... |
💔 Test failed - status-travis |
@bors retry Somethings wrong with Travis. |
…aelwoerister extend stable hasher to support `CanonicalTy` Fixes #49043 r? @michaelwoerister
☀️ Test successful - status-appveyor, status-travis |
Fixes #49043
r? @michaelwoerister