Ignore but do not assume region obligations from unifying headers in negative coherence #117994

compiler-errors · 2023-11-16T22:56:05Z

Partly addresses a FIXME that was added in #112875. Just as we can throw away the nested trait/projection obligations from unifying two impl headers, we can also just throw away the region obligations too.

I removed part of the FIXME that was incorrect, namely:

Given that the only region constraints we get are involving inference regions in the root, it shouldn't matter, but still sus.

This is not true when unifying fn(A) and for<'b> fn(&'b B) which ends up with placeholder region outlives from non-root universes. I'm pretty sure this is okay, though it would be nice if we were to use them as assumptions. See the explicit revision of the test I committed, which still fails.

Fixes #117986

r? lcnr, feel free to reassign tho.

lcnr · 2023-11-17T09:15:30Z

@bors r+ rollup

bors · 2023-11-17T09:15:33Z

📌 Commit d688b86 has been approved by lcnr

It is now in the queue for this repository.

…-in-coherence, r=lcnr Ignore but do not assume region obligations from unifying headers in negative coherence Partly addresses a FIXME that was added in rust-lang#112875. Just as we can throw away the nested trait/projection obligations from unifying two impl headers, we can also just throw away the region obligations too. I removed part of the FIXME that was incorrect, namely: > Given that the only region constraints we get are involving inference regions in the root, it shouldn't matter, but still sus. This is not true when unifying `fn(A)` and `for<'b> fn(&'b B)` which ends up with placeholder region outlives from non-root universes. I'm pretty sure this is okay, though it would be nice if we were to use them as assumptions. See the `explicit` revision of the test I committed, which still fails. Fixes rust-lang#117986 r? lcnr, feel free to reassign tho.

TaKO8Ki · 2023-11-18T19:47:29Z

This pull request may have caused this error.

#118041 (comment)

compiler-errors · 2023-11-18T19:48:49Z

This PR conflicts with #117992. I will re-approve and land each separately, I think.

@bors r-

aliemjay · 2023-11-19T03:35:01Z

compiler/rustc_trait_selection/src/traits/coherence.rs

+    let _ = infcx.take_registered_region_obligations();
+    let _ = infcx.take_and_reset_region_constraints();


I think it would more more clear if they are explicitly dropped along with equate_obligations before forking in impl_intersection_has_negative_obligation.

drop(equate_obligations); drop(infcx.take_registered_region_obligations()); drop(infcx.take_and_reset_region_constraints());

…negative coherence

compiler-errors · 2023-11-19T19:35:42Z

@bors r=lcnr

Updated to use drop explicitly, and to drop the region obligations only once.

bors · 2023-11-19T19:35:45Z

📌 Commit 488dcb7 has been approved by lcnr

It is now in the queue for this repository.

…-in-coherence, r=lcnr Ignore but do not assume region obligations from unifying headers in negative coherence Partly addresses a FIXME that was added in rust-lang#112875. Just as we can throw away the nested trait/projection obligations from unifying two impl headers, we can also just throw away the region obligations too. I removed part of the FIXME that was incorrect, namely: > Given that the only region constraints we get are involving inference regions in the root, it shouldn't matter, but still sus. This is not true when unifying `fn(A)` and `for<'b> fn(&'b B)` which ends up with placeholder region outlives from non-root universes. I'm pretty sure this is okay, though it would be nice if we were to use them as assumptions. See the `explicit` revision of the test I committed, which still fails. Fixes rust-lang#117986 r? lcnr, feel free to reassign tho.

…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#115526 (Add arm64e-apple-ios & arm64e-apple-darwin targets) - rust-lang#115691 (Add `$message_type` field to distinguish json diagnostic outputs) - rust-lang#117828 (Avoid iterating over hashmaps in astconv) - rust-lang#117832 (interpret: simplify handling of shifts by no longer trying to handle signed and unsigned shift amounts in the same branch) - rust-lang#117891 (Recover `dyn` and `impl` after `for<...>`) - rust-lang#117957 (if available use a Child's pidfd for kill/wait) - rust-lang#117994 (Ignore but do not assume region obligations from unifying headers in negative coherence) - rust-lang#118068 (subtree update cg_gcc 2023/11/17) r? `@ghost` `@rustbot` modify labels: rollup

…mpiler-errors Rollup of 8 pull requests Successful merges: - rust-lang#117828 (Avoid iterating over hashmaps in astconv) - rust-lang#117832 (interpret: simplify handling of shifts by no longer trying to handle signed and unsigned shift amounts in the same branch) - rust-lang#117891 (Recover `dyn` and `impl` after `for<...>`) - rust-lang#117957 (if available use a Child's pidfd for kill/wait) - rust-lang#117988 (Handle attempts to have multiple `cfg`d tail expressions) - rust-lang#117994 (Ignore but do not assume region obligations from unifying headers in negative coherence) - rust-lang#118000 (Make regionck care about placeholders in outlives components) - rust-lang#118068 (subtree update cg_gcc 2023/11/17) r? `@ghost` `@rustbot` modify labels: rollup

…tthiaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#117828 (Avoid iterating over hashmaps in astconv) - rust-lang#117832 (interpret: simplify handling of shifts by no longer trying to handle signed and unsigned shift amounts in the same branch) - rust-lang#117891 (Recover `dyn` and `impl` after `for<...>`) - rust-lang#117957 (if available use a Child's pidfd for kill/wait) - rust-lang#117988 (Handle attempts to have multiple `cfg`d tail expressions) - rust-lang#117994 (Ignore but do not assume region obligations from unifying headers in negative coherence) - rust-lang#118000 (Make regionck care about placeholders in outlives components) - rust-lang#118068 (subtree update cg_gcc 2023/11/17) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#117994 - compiler-errors:throw-away-regions-in-coherence, r=lcnr Ignore but do not assume region obligations from unifying headers in negative coherence Partly addresses a FIXME that was added in rust-lang#112875. Just as we can throw away the nested trait/projection obligations from unifying two impl headers, we can also just throw away the region obligations too. I removed part of the FIXME that was incorrect, namely: > Given that the only region constraints we get are involving inference regions in the root, it shouldn't matter, but still sus. This is not true when unifying `fn(A)` and `for<'b> fn(&'b B)` which ends up with placeholder region outlives from non-root universes. I'm pretty sure this is okay, though it would be nice if we were to use them as assumptions. See the `explicit` revision of the test I committed, which still fails. Fixes rust-lang#117986 r? lcnr, feel free to reassign tho.

rustbot assigned lcnr Nov 16, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 16, 2023

compiler-errors mentioned this pull request Nov 17, 2023

Make regionck care about placeholders in outlives components #118000

Merged

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 Nov 17, 2023

matthiaskrgr mentioned this pull request Nov 17, 2023

Rollup of 7 pull requests #118019

Closed

TaKO8Ki mentioned this pull request Nov 18, 2023

Rollup of 8 pull requests #118041

Closed

compiler-errors mentioned this pull request Nov 18, 2023

Rollup of 9 pull requests #118049

Closed

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 Nov 18, 2023

aliemjay reviewed Nov 19, 2023

View reviewed changes

Ignore but do not assume region obligations from unifying headers in …

488dcb7

…negative coherence

rust-cloud-vms bot force-pushed the throw-away-regions-in-coherence branch from d688b86 to 488dcb7 Compare November 19, 2023 19:23

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 Nov 19, 2023

compiler-errors mentioned this pull request Nov 20, 2023

Rollup of 8 pull requests #118078

Closed

compiler-errors mentioned this pull request Nov 20, 2023

Don't require intercrate mode for negative coherence #117992

Merged

compiler-errors mentioned this pull request Nov 20, 2023

Rollup of 8 pull requests #118082

Merged

bors merged commit 40a781b into rust-lang:master Nov 20, 2023
11 checks passed

rustbot added this to the 1.76.0 milestone Nov 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ignore but do not assume region obligations from unifying headers in negative coherence #117994

Ignore but do not assume region obligations from unifying headers in negative coherence #117994

compiler-errors commented Nov 16, 2023

lcnr commented Nov 17, 2023

bors commented Nov 17, 2023

TaKO8Ki commented Nov 18, 2023

compiler-errors commented Nov 18, 2023

aliemjay Nov 19, 2023

compiler-errors commented Nov 19, 2023

bors commented Nov 19, 2023

		let _ = infcx.take_registered_region_obligations();
		let _ = infcx.take_and_reset_region_constraints();

Ignore but do not assume region obligations from unifying headers in negative coherence #117994

Ignore but do not assume region obligations from unifying headers in negative coherence #117994

Conversation

compiler-errors commented Nov 16, 2023

lcnr commented Nov 17, 2023

bors commented Nov 17, 2023

TaKO8Ki commented Nov 18, 2023

compiler-errors commented Nov 18, 2023

aliemjay Nov 19, 2023

Choose a reason for hiding this comment

compiler-errors commented Nov 19, 2023

bors commented Nov 19, 2023