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

const_evaluatable_checked: extend predicate collection #77049

Merged
merged 6 commits into from
Sep 24, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Sep 22, 2020

We now walk the hir instead of using ty so that we get better spans here, While I am still not completely sure if that's
what we want in the end, it does seem a lot closer to the final goal than the previous version.

We also look into type aliases (and use a TypeVisitor here), about which I am not completely sure, but we will see how well this works.

We also look into fn decls, so the following should work now.

fn test<T>() -> [u8; std::mem::size_of::<T>()] {
    [0; std::mem::size_of::<T>()]
}

Additionally, we visit the optional trait and self type of impls.

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2020
--> $DIR/from-sig-fail.rs:4:35
|
LL | fn test<const N: usize>() -> [u8; N - 1] {
| ^^^^^ attempt to compute `0_usize - 1_usize` which would overflow
Copy link
Contributor

@oli-obk oli-obk Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we somehow tell the user that this is speciically about test<0>?

I mean ideally we could report this on the call site

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should. Will do so in another PR though.

@lcnr
Copy link
Contributor Author

lcnr commented Sep 22, 2020

Added a commit which assigns the correct DefId in nominal obligations, I was unable to test these changes without const_evaluatable_checked so I left them as part of this PR.

We probably need a perf run for that, but we should be able to optimize it without issue.

@lcnr lcnr changed the title const_evaluatable_checked: look at fn_decl and use hir instead of ty const_evaluatable_checked: extend predicate collection Sep 22, 2020
@lcnr lcnr force-pushed the const-eval-function-signature branch from 88c5983 to 7f8d8ae Compare September 22, 2020 16:00
@lcnr
Copy link
Contributor Author

lcnr commented Sep 23, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Sep 23, 2020

⌛ Trying commit 7f8d8ae26ed96b7e283f215b870bd0a8323354cb with merge 8b29e63e47b49b0147a2ca5973bf66f00e90c2b2...

@bors
Copy link
Contributor

bors commented Sep 23, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: 8b29e63e47b49b0147a2ca5973bf66f00e90c2b2 (8b29e63e47b49b0147a2ca5973bf66f00e90c2b2)

@rust-timer
Copy link
Collaborator

Queued 8b29e63e47b49b0147a2ca5973bf66f00e90c2b2 with parent f6d5920, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (8b29e63e47b49b0147a2ca5973bf66f00e90c2b2): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

@oli-obk
Copy link
Contributor

oli-obk commented Sep 23, 2020

@bors rollup-

@oli-obk
Copy link
Contributor

oli-obk commented Sep 23, 2020

r=me with a test demonstrating the type alias situation

@oli-obk
Copy link
Contributor

oli-obk commented Sep 23, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Sep 23, 2020

📌 Commit 7f8d8ae26ed96b7e283f215b870bd0a8323354cb has been approved by oli-obk

@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 Sep 23, 2020
@bors
Copy link
Contributor

bors commented Sep 24, 2020

☔ The latest upstream changes (presumably #77102) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 24, 2020
@lcnr lcnr force-pushed the const-eval-function-signature branch from 7f8d8ae to 21edd10 Compare September 24, 2020 07:07
@lcnr
Copy link
Contributor Author

lcnr commented Sep 24, 2020

rebased and updated test output

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Sep 24, 2020

📌 Commit 21edd10 has been approved by oli-obk

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 24, 2020
@bors
Copy link
Contributor

bors commented Sep 24, 2020

⌛ Testing commit 21edd10 with merge 3a4da87...

@bors
Copy link
Contributor

bors commented Sep 24, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing 3a4da87 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 24, 2020
@bors bors merged commit 3a4da87 into rust-lang:master Sep 24, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 24, 2020
@lcnr lcnr deleted the const-eval-function-signature branch September 24, 2020 14:33
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.

6 participants