-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Don't run const propagation on items with inconsistent bounds #67914
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
63bef0c
to
89308bb
Compare
Can you add the test without the feature gate in the issue? |
@matthewjasper: Updated |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
9868744
to
9951450
Compare
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.
r=me with nit addressed
src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs
Outdated
Show resolved
Hide resolved
@bors r+ |
📌 Commit 7f26d32 has been approved by |
@matthewjasper: The test failed locally for me - can you approve the new commit I just pushed? |
@bors r+ |
📌 Commit b4125f0 has been approved by |
…, r=matthewjasper Don't run const propagation on items with inconsistent bounds Fixes rust-lang#67696 Using `#![feature(trivial_bounds)]`, it's possible to write functions with unsatisfiable 'where' clauses, making them uncallable. However, the user can act as if these 'where' clauses are true inside the body of the function, leading to code that would normally be impossible to write. Since const propgation can run even without any user-written calls to a function, we need to explcitly check for these uncallable functions.
⌛ Testing commit b4125f0 with merge 9e9b8f83a61360e787996820980907933ef04773... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
Using `#![feature(trivial_bounds)]`, it's possible to write functions with unsatisfiable 'where' clauses, making them uncallable. However, the user can act as if these 'where' clauses are true inside the body of the function, leading to code that would normally be impossible to write. Since const propgation can run even without any user-written calls to a function, we need to explcitly check for these uncallable functions.
This avoids a strange linker error that we get with only "--emit=mir" and "check-pass"
b4125f0
to
e390b91
Compare
@matthewjasper: I've switched over to use |
// could) are fine. However, false positives (running const-prop on | ||
// an item with unsatisfiable bounds) can lead to us generating invalid | ||
// MIR. | ||
if !tcx.substitute_normalize_and_test_predicates(( |
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.
If the substs are empty, can we use TraitQueryMode::Standard
? Doing so should ensure that substitute_normalize_and_test_predicates
isn't invoked twice when we know that the result will be the same irrelevant of the TraitQueryMode
, right?
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.
When you say "invoked twice", do you mean substitute_normalize_and_test_predicates
being invoked from somewhere else with the same DefId
and Substs
, but a different TraitQueryMode
?
I think we could still have overflow with some really weird code (e.g a macro-generated impl Foo for A where B: Foo
, impl Foo for B where C: Foo
, etc). I'm not certain if we would end up with a "legitimate" overflow in that case (i.e. caused by something other than const-prop).
I think it's better to always use canonical mode here. Emitting an error here is always wrong, and could be very misleading to users (they might assume that their code has an issue, when it's really fine).
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.
Ah, that makes sense. Thanks
@bors r=matthewjasper,oli-obk |
📌 Commit e390b91 has been approved by |
…, r=matthewjasper,oli-obk Don't run const propagation on items with inconsistent bounds Fixes rust-lang#67696 Using `#![feature(trivial_bounds)]`, it's possible to write functions with unsatisfiable 'where' clauses, making them uncallable. However, the user can act as if these 'where' clauses are true inside the body of the function, leading to code that would normally be impossible to write. Since const propgation can run even without any user-written calls to a function, we need to explcitly check for these uncallable functions.
…, r=matthewjasper,oli-obk Don't run const propagation on items with inconsistent bounds Fixes rust-lang#67696 Using `#![feature(trivial_bounds)]`, it's possible to write functions with unsatisfiable 'where' clauses, making them uncallable. However, the user can act as if these 'where' clauses are true inside the body of the function, leading to code that would normally be impossible to write. Since const propgation can run even without any user-written calls to a function, we need to explcitly check for these uncallable functions.
Rollup of 12 pull requests Successful merges: - #67784 (Reset Formatter flags on exit from pad_integral) - #67914 (Don't run const propagation on items with inconsistent bounds) - #68141 (use winapi for non-stdlib Windows bindings) - #68211 (Add failing example for E0170 explanation) - #68219 (Untangle ZST validation from integer validation and generalize it to all zsts) - #68222 (Update the wasi-libc bundled with libstd) - #68226 (Avoid calling tcx.hir().get() on CRATE_HIR_ID) - #68227 (Update to a version of cmake with windows arm64 support) - #68229 (Update iovec to a version with no winapi dependency) - #68230 (Update libssh2-sys to a version that can build for aarch64-pc-windows…) - #68231 (Better support for cross compilation on Windows.) - #68233 (Update compiler_builtins with changes to fix 128 bit integer remainder for aarch64 windows.) Failed merges: r? @ghost
Fixes #67696
Using
#![feature(trivial_bounds)]
, it's possible to write functionswith unsatisfiable 'where' clauses, making them uncallable. However, the
user can act as if these 'where' clauses are true inside the body of the
function, leading to code that would normally be impossible to write.
Since const propgation can run even without any user-written calls to a
function, we need to explcitly check for these uncallable functions.