Skip to content
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

Simplify eager normalization of constants #63820

Merged
merged 1 commit into from
Aug 28, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Aug 22, 2019

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 22, 2019
@nikomatsakis
Copy link
Contributor

Looks good but I have one nlggling concern -- though maybe think it's pre-existing. Can we have type inference variables in the type of the constant (or some other place?). In that case we might want to invoke infcx.fully_resolve or some such thing before invoking eval?

@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 27, 2019

It's not possible to end up with an unevaluated ty::Const whose ty or substs field has inference variables. We may get there in the future once we have generic constants, but for now that can't happen.

I can add a debug assert assert here to make sure of that though.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 27, 2019

📌 Commit 181ed55 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 27, 2019
@nikomatsakis
Copy link
Contributor

@oli-obk

I can add a debug assert assert here to make sure of that though.

oh, I thought you meant you had added such an assert -- in any case, I don't think it'd be catastrophic if an inference variable leaked through. We might ICE or be overly conservative, but I think not worse than that.

@bors
Copy link
Collaborator

bors commented Aug 28, 2019

⌛ Testing commit 181ed55 with merge ac21131...

bors added a commit that referenced this pull request Aug 28, 2019
@bors
Copy link
Collaborator

bors commented Aug 28, 2019

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing ac21131 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 28, 2019
@bors bors merged commit 181ed55 into rust-lang:master Aug 28, 2019
@oli-obk oli-obk deleted the eager_const_eval branch March 16, 2021 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants