Skip to content

Yeet propagate_ambiguity - #162935

Merged
rust-bors[bot] merged 4 commits into
rust-lang:mainfrom
adwinwhite:abby-ambig
Sep 28, 2026
Merged

rust-bors[bot] merged 4 commits into
rust-lang:mainfrom
adwinwhite:abby-ambig

Conversation

@adwinwhite

@adwinwhite adwinwhite commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Fixes rust-lang/project-assumptions-on-binders#29

When assumptions computation fails, we want to force the goal response to be ambiguous since we can't evaluate placeholder constraints.
We used to do this via LeafRegionConstraint::Ambiguity and propagates it everywhere.

This PR simplifies that by tracking whether we should force ambiguity in a more direct way. We just check whether we have computed assumptions for relevant universes.
This also clarifies the meaning of LeafRegionConstraint::Ambiguity which only represents true ambiguity (forever ambiguity no matter inference progress).

This doesn't solve the problem that we're being conservative about forcing ambiguity. Maybe we can have falses in some universes even if other universes don't have assumptions. We can be smart about this in the future.

Unsure part: we can also have ambiguity from non-lifetime placeholder. Unsure what to do with that. Still trying to understand it.

r? @BoxyUwU

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Sep 18, 2026
@rustbot

rustbot commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

BoxyUwU is currently at their maximum review capacity.
They may take a while to respond.

@rust-log-analyzer

This comment has been minimized.

@adwinwhite
adwinwhite marked this pull request as ready for review September 18, 2026 06:48
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 18, 2026
@rust-log-analyzer

This comment has been minimized.

@BoxyUwU BoxyUwU left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sick! this looks right to me :3 thanks

View changes since this review


let constraint = ((smallest_universe + 1)..=largest_universe)
.map(|u| UniverseIndex::from_usize(u))
if !self

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a comment explaining why we use ambiguity here

@BoxyUwU

BoxyUwU commented Sep 23, 2026

Copy link
Copy Markdown
Member

@rustbot author

@rustbot rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 23, 2026
@rustbot

rustbot commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rust-bors

This comment has been minimized.

@adwinwhite

Copy link
Copy Markdown
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 28, 2026
@BoxyUwU

BoxyUwU commented Sep 28, 2026

Copy link
Copy Markdown
Member

@bors r+ rollup

thx adwin

@rust-bors

rust-bors Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

📌 Commit c901622 has been approved by BoxyUwU

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 28, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 28, 2026
Yeet `propagate_ambiguity`

Fixes rust-lang/project-assumptions-on-binders#29

When assumptions computation fails, we want to force the goal response to be ambiguous since we can't evaluate placeholder constraints.
We used to do this via `LeafRegionConstraint::Ambiguity` and propagates it everywhere.

This PR simplifies that by tracking whether we should force ambiguity in a more direct way. We just check whether we have computed assumptions for relevant universes.
This also clarifies the meaning of `LeafRegionConstraint::Ambiguity` which only represents true ambiguity (forever ambiguity no matter inference progress).

This doesn't solve the problem that we're being conservative about forcing ambiguity. Maybe we can have `false`s in  some universes even if other universes don't have assumptions. We can be smart about this in the future.

Unsure part: we can also have ambiguity from non-lifetime placeholder. Unsure what to do with that. Still trying to understand it.

r? @BoxyUwU
rust-bors Bot pushed a commit that referenced this pull request Sep 28, 2026
…uwer

Rollup of 5 pull requests

Successful merges:

 - #162917 (fix ice for unresolved inherent delegation)
 - #163209 (x perf takes database path)
 - #162935 (Yeet `propagate_ambiguity`)
 - #163289 (Ensure llvm worker threads have sufficient stack space)
 - #163303 (Remove `PartialOrd`/`Ord` impls for `Span`/`SpanData`)
rust-bors Bot pushed a commit that referenced this pull request Sep 28, 2026
…uwer

Rollup of 9 pull requests

Successful merges:

 - #162915 (Fix maximum `send` length on non-{Mac, Windows} platforms)
 - #162917 (fix ice for unresolved inherent delegation)
 - #163209 (x perf takes database path)
 - #162373 (Move the foreign module #[link] ABI check to attribute parsing)
 - #162829 (regression test for inherent associated const ICE)
 - #162935 (Yeet `propagate_ambiguity`)
 - #163289 (Ensure llvm worker threads have sufficient stack space)
 - #163303 (Remove `PartialOrd`/`Ord` impls for `Span`/`SpanData`)
 - #163421 (triagebot: Subscribe me to changes in test-float-parse)
@rust-bors
rust-bors Bot merged commit 7987cc7 into rust-lang:main Sep 28, 2026
13 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Sep 28, 2026
rust-bors Bot pushed a commit that referenced this pull request Sep 28, 2026
Rollup merge of #162935 - adwinwhite:abby-ambig, r=BoxyUwU

Yeet `propagate_ambiguity`

Fixes rust-lang/project-assumptions-on-binders#29

When assumptions computation fails, we want to force the goal response to be ambiguous since we can't evaluate placeholder constraints.
We used to do this via `LeafRegionConstraint::Ambiguity` and propagates it everywhere.

This PR simplifies that by tracking whether we should force ambiguity in a more direct way. We just check whether we have computed assumptions for relevant universes.
This also clarifies the meaning of `LeafRegionConstraint::Ambiguity` which only represents true ambiguity (forever ambiguity no matter inference progress).

This doesn't solve the problem that we're being conservative about forcing ambiguity. Maybe we can have `false`s in  some universes even if other universes don't have assumptions. We can be smart about this in the future.

Unsure part: we can also have ambiguity from non-lifetime placeholder. Unsure what to do with that. Still trying to understand it.

r? @BoxyUwU
@rust-timer

Copy link
Copy Markdown
Collaborator

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (2be20bd): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.7%, -0.1%] 19
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

Results (secondary -4.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-4.0% [-4.0%, -4.0%] 1
All ❌✅ (primary) - - 0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: missing data
Artifact size: 406.43 MiB -> 406.35 MiB (-0.02%)

pull Bot pushed a commit to xtqqczze/rust-lang-miri that referenced this pull request Sep 29, 2026
…uwer

Rollup of 9 pull requests

Successful merges:

 - rust-lang/rust#162915 (Fix maximum `send` length on non-{Mac, Windows} platforms)
 - rust-lang/rust#162917 (fix ice for unresolved inherent delegation)
 - rust-lang/rust#163209 (x perf takes database path)
 - rust-lang/rust#162373 (Move the foreign module #[link] ABI check to attribute parsing)
 - rust-lang/rust#162829 (regression test for inherent associated const ICE)
 - rust-lang/rust#162935 (Yeet `propagate_ambiguity`)
 - rust-lang/rust#163289 (Ensure llvm worker threads have sufficient stack space)
 - rust-lang/rust#163303 (Remove `PartialOrd`/`Ord` impls for `Span`/`SpanData`)
 - rust-lang/rust#163421 (triagebot: Subscribe me to changes in test-float-parse)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ambiguity handling is weird

5 participants