Skip to content

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Sep 18, 2025

fixes #146467 cc @amandasystems

our understanding here is as follows: region constraints from computing implied bounds gets ConstraintCategory::Internal. If there's a higher-ranked subtyping errors while computing implied bounds we then ended up with only ConstraintCategory::Internal and ConstraintCategory::OutlivesUnnameablePlaceholder(_) constraints.

The path was something like

  • 'placeholderU2: 'placeholderU1 (Internal)
  • 'placeholderU1: 'static (OutlivesUnnameablePlaceholder('placeholderU2))

It's generally somewhat subtle here as ideally relating placeholders doesn't introduce 'static constraints. Relating the placeholders themselves will always error regardless, cc #142623.


separately fixes #145925 (comment) by updating the location for deferred closure requirements inside of promoteds. I am not updating their category as doing so is 1) effort and 2) imo actually undesirable 🤔 see the comments in TypeChecker::check_promoted cc @lqd

r? lqd

@rustbot 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 Sep 18, 2025
@lcnr lcnr changed the title fix 2 small borrowck issues fix 2 borrowck issues Sep 18, 2025
@rust-log-analyzer

This comment has been minimized.

@lcnr lcnr force-pushed the fix-placeholder-ice branch from 76a4b28 to 2d3f828 Compare September 18, 2025 09:50
@apiraino
Copy link
Contributor

@lcnr IIUC this fixes #146467 and also replaces #142623, correct?

@amandasystems
Copy link
Contributor

[...] and also replaces #142623, correct?

It doesn't do all of what #142623 does but it slightly overlaps.

@amandasystems
Copy link
Contributor

amandasystems commented Sep 18, 2025

@lcnr Please take care that this addresses the larger reproductions of #146467 as well, I noticed they actually trigger different code paths so they're not identical. I fixed the smaller example first and then realised the larger ones still errored.

@lcnr lcnr force-pushed the fix-placeholder-ice branch from 2d3f828 to 3b2bbcd Compare September 18, 2025 11:57
@lqd
Copy link
Member

lqd commented Sep 19, 2025

Yay, thanks so much. This looks good to me at first glance, but will have to review in a couple days, sorry for the delay.

Copy link
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

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

Looking into this more, the categories were kind of strange with the nested bodies in promoteds right now: the results are now more consistent with the non-promoted cases. I agree we shouldn't change them back and if any confusion arises, it will be more due to the existing inconsistencies of a return category turning into a boring category just because it's from a promoted, than the opposite.

r=me with typo fixed.

(As for a test for the locations change, I should mention that I tested this manually, and that we also have a couple of UI tests that exercise this value, when using polonius. Enabling them with -Zpolonius=next will be the non-regression test cases, and I'll do that myself in another PR.)

View changes since this review

@lqd
Copy link
Member

lqd commented Sep 24, 2025

Please take care that this addresses the larger reproductions of #146467 as well

As far as I can tell, it does.

Co-authored-by: Rémy Rakic <[email protected]>
@lcnr
Copy link
Contributor Author

lcnr commented Sep 24, 2025

As far as I can tell, it does.

jup, i've also tested this

@bors r=lqd

@bors
Copy link
Collaborator

bors commented Sep 24, 2025

📌 Commit 3378997 has been approved by lqd

It is now in the queue for this repository.

@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 24, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 24, 2025
fix 2 borrowck issues

fixes rust-lang#146467 cc `@amandasystems`

our understanding here is as follows: region constraints from computing implied bounds gets `ConstraintCategory::Internal`. If there's a higher-ranked subtyping errors while computing implied bounds we then ended up with only `ConstraintCategory::Internal` and `ConstraintCategory::OutlivesUnnameablePlaceholder(_)` constraints.

The path was something like
- `'placeholderU2: 'placeholderU1` (`Internal`)
- `'placeholderU1: 'static` (`OutlivesUnnameablePlaceholder('placeholderU2)`)

It's generally somewhat subtle here as ideally relating placeholders doesn't introduce `'static` constraints. Relating the placeholders themselves will always error regardless, cc rust-lang#142623.

---

separately fixes rust-lang#145925 (comment) by updating the location for deferred closure requirements inside of promoteds. I am not updating their category as doing so is 1) effort and 2) imo actually undesirable 🤔 see the comments in `TypeChecker::check_promoted` cc `@lqd`

r? lqd
bors added a commit that referenced this pull request Sep 24, 2025
Rollup of 9 pull requests

Successful merges:

 - #146711 (fix 2 borrowck issues)
 - #146735 (unstably constify float mul_add methods)
 - #146857 (revert change removing `has_infer` check. Commit conservatively patch…)
 - #146897 (fix ICE in rustdoc::invalid_html_tags)
 - #146915 (Make missed precondition-free float intrinsics safe)
 - #146932 (Switch next-solver related rustc dependencies of r-a to crates.io ones)
 - #146959 (temporary-lifetime-extension-tuple-ctor.rs: make usable on all editions)
 - #146964 (library: std: sys: pal: uefi: Add some comments)
 - #146969 (const-eval: better wording for errors involving maybe-null pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 24, 2025
Rollup of 8 pull requests

Successful merges:

 - #146711 (fix 2 borrowck issues)
 - #146857 (revert change removing `has_infer` check. Commit conservatively patch…)
 - #146897 (fix ICE in rustdoc::invalid_html_tags)
 - #146915 (Make missed precondition-free float intrinsics safe)
 - #146932 (Switch next-solver related rustc dependencies of r-a to crates.io ones)
 - #146959 (temporary-lifetime-extension-tuple-ctor.rs: make usable on all editions)
 - #146964 (library: std: sys: pal: uefi: Add some comments)
 - #146969 (const-eval: better wording for errors involving maybe-null pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3150538 into rust-lang:master Sep 24, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 24, 2025
rust-timer added a commit that referenced this pull request Sep 24, 2025
Rollup merge of #146711 - lcnr:fix-placeholder-ice, r=lqd

fix 2 borrowck issues

fixes #146467 cc ``@amandasystems``

our understanding here is as follows: region constraints from computing implied bounds gets `ConstraintCategory::Internal`. If there's a higher-ranked subtyping errors while computing implied bounds we then ended up with only `ConstraintCategory::Internal` and `ConstraintCategory::OutlivesUnnameablePlaceholder(_)` constraints.

The path was something like
- `'placeholderU2: 'placeholderU1` (`Internal`)
- `'placeholderU1: 'static` (`OutlivesUnnameablePlaceholder('placeholderU2)`)

It's generally somewhat subtle here as ideally relating placeholders doesn't introduce `'static` constraints. Relating the placeholders themselves will always error regardless, cc #142623.

---

separately fixes #145925 (comment) by updating the location for deferred closure requirements inside of promoteds. I am not updating their category as doing so is 1) effort and 2) imo actually undesirable 🤔 see the comments in `TypeChecker::check_promoted` cc ``@lqd``

r? lqd
github-actions bot pushed a commit to rust-lang/rust-analyzer that referenced this pull request Sep 25, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang/rust#146711 (fix 2 borrowck issues)
 - rust-lang/rust#146857 (revert change removing `has_infer` check. Commit conservatively patch…)
 - rust-lang/rust#146897 (fix ICE in rustdoc::invalid_html_tags)
 - rust-lang/rust#146915 (Make missed precondition-free float intrinsics safe)
 - rust-lang/rust#146932 (Switch next-solver related rustc dependencies of r-a to crates.io ones)
 - rust-lang/rust#146959 (temporary-lifetime-extension-tuple-ctor.rs: make usable on all editions)
 - rust-lang/rust#146964 (library: std: sys: pal: uefi: Add some comments)
 - rust-lang/rust#146969 (const-eval: better wording for errors involving maybe-null pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE Illegal placeholder constraint blamed; should have redirected to other region relation
7 participants