Skip to content

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Aug 21, 2025

This changes the impl of NormalizesTo for opaque types to be structural during HIR typeck. The previous impl equated region variables of the opaque type key with existing entries which can result in spurious leak check errors and also results in mismatches with MIR borrowck, theoretically causing ICE.

The approach is very similar to #145244 in MIR typeck:

  • we collect all uses of opaque types during HIR typeck
  • before writeback, we search for defining uses
    • the opaque type key has fully universal generic args modulo regions
    • the hidden type has no infer vars
  • we use these defining uses to compute the concrete type for the opaque and map it to the definition site
  • we use this concrete type to check the type of all uses of opaques during HIR typeck. This also constrains infer vars in non-defining uses

Fixes rust-lang/trait-system-refactor-initiative#135, fixes rust-lang/trait-system-refactor-initiative#49.

r? @BoxyUwU

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Aug 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2025

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@BoxyUwU BoxyUwU 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 Aug 21, 2025
@bors
Copy link
Collaborator

bors commented Aug 23, 2025

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

@lcnr lcnr force-pushed the non-defining-uses-hir-typeck branch from a78b9d1 to da0cc61 Compare August 25, 2025 08:36
@rustbot
Copy link
Collaborator

rustbot commented Aug 25, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

erase regions also anonymizes bound vars, which is undesirable
@lcnr lcnr force-pushed the non-defining-uses-hir-typeck branch 8 times, most recently from e08e9d6 to 4ca8028 Compare August 25, 2025 10:45
@lcnr lcnr force-pushed the non-defining-uses-hir-typeck branch from 4ca8028 to d6a18e1 Compare August 25, 2025 12:20
@lcnr
Copy link
Contributor Author

lcnr commented Aug 25, 2025

@bors r=BoxyUwU rollup=never

@bors
Copy link
Collaborator

bors commented Aug 25, 2025

📌 Commit d6a18e1 has been approved by BoxyUwU

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 25, 2025
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
npm WARN deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm WARN deprecated @humanwhocodes/[email protected]: Use @eslint/object-schema instead
npm WARN deprecated [email protected]: Rimraf versions prior to v4 are no longer supported
npm WARN deprecated [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options.
npm ERR! code E403
npm ERR! 403 403 Forbidden - GET https://registry.npmjs.org/yargs/-/yargs-17.7.2.tgz
npm ERR! 403 In most cases, you or one of your dependencies are requesting
npm ERR! 403 a package version that is forbidden by your security policy, or
npm ERR! 403 on a server you do not have access to.

npm ERR! A complete log of this run can be found in: /home/user/.npm/_logs/2025-08-25T13_28_48_241Z-debug-0.log
npm install did not exit successfully

thread 'main' panicked at src/tools/rustdoc-gui-test/src/main.rs:69:10:
unable to install browser-ui-test: Custom { kind: Other, error: "npm install returned exit code exit status: 1" }
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/788da80fcfcef3f34c90def5baa32813e39a1a41/library/std/src/panicking.rs:697:5
   1: core::panicking::panic_fmt
             at /rustc/788da80fcfcef3f34c90def5baa32813e39a1a41/library/core/src/panicking.rs:75:14

@lcnr lcnr closed this Aug 25, 2025
@lcnr lcnr reopened this Aug 25, 2025
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 25, 2025
@lcnr
Copy link
Contributor Author

lcnr commented Aug 25, 2025

@bors r=BoxyUwU

@bors
Copy link
Collaborator

bors commented Aug 25, 2025

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Aug 25, 2025

📌 Commit d6a18e1 has been approved by BoxyUwU

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 25, 2025
@rust-log-analyzer
Copy link
Collaborator

The job tidy failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
npm WARN deprecated [email protected]: Rimraf versions prior to v4 are no longer supported
npm WARN deprecated @humanwhocodes/[email protected]: Use @eslint/object-schema instead
npm WARN deprecated @humanwhocodes/[email protected]: Use @eslint/config-array instead
npm WARN deprecated [email protected]: Glob versions prior to v9 are no longer supported
npm ERR! code E429
npm ERR! 429 Too Many Requests - GET https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz

npm ERR! A complete log of this run can be found in: /home/user/.npm/_logs/2025-08-25T13_49_03_563Z-debug-0.log
tidy error: IO error: npm install returned exit code exit status: 1
npm install did not exit successfully
some tidy checks failed
Command `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools-bin/rust-tidy /checkout /checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo /checkout/obj/build 4 /node/bin/npm --extra-checks=py,cpp,js,spellcheck` failed with exit code 1
Created at: src/bootstrap/src/core/build_steps/tool.rs:1583:23
Executed at: src/bootstrap/src/core/build_steps/test.rs:1225:29

Command has failed. Rerun with -v to see more details.
Bootstrap failed while executing `test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp,js,spellcheck`
Build completed unsuccessfully in 0:08:45
  local time: Mon Aug 25 13:55:15 UTC 2025
  network time: Mon, 25 Aug 2025 13:55:15 GMT
##[error]Process completed with exit code 1.

@BoxyUwU
Copy link
Member

BoxyUwU commented Aug 25, 2025

@bors retry

@bors
Copy link
Collaborator

bors commented Aug 25, 2025

⌛ Testing commit d6a18e1 with merge d327d65...

@bors
Copy link
Collaborator

bors commented Aug 26, 2025

☀️ Test successful - checks-actions
Approved by: BoxyUwU
Pushing d327d65 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 26, 2025
@bors bors merged commit d327d65 into rust-lang:master Aug 26, 2025
18 of 21 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 26, 2025
Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 54c5812 (parent) -> d327d65 (this PR)

Test differences

Show 32 test diffs

Stage 1

  • [ui] tests/ui/traits/next-solver/opaques/different-bound-vars.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/traits/next-solver/opaques/different-bound-vars.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/traits/next-solver/opaques/non-defining-use-hir-typeck.rs: [missing] -> pass (J0)
  • [ui] tests/ui/traits/next-solver/opaques/universal-args-non-defining.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/traits/next-solver/opaques/universal-args-non-defining.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/type-alias-impl-trait/generic_nondefining_use.rs: pass -> [missing] (J0)
  • [ui] tests/ui/type-alias-impl-trait/generic_nondefining_use.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/type-alias-impl-trait/generic_nondefining_use.rs#next: [missing] -> pass (J0)
  • [ui] tests/ui/type-alias-impl-trait/normalize-args-before-defining-use-check.rs#current: [missing] -> pass (J0)
  • [ui] tests/ui/type-alias-impl-trait/normalize-args-before-defining-use-check.rs#next: [missing] -> pass (J0)

Stage 2

  • [ui] tests/ui/traits/next-solver/opaques/different-bound-vars.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/traits/next-solver/opaques/different-bound-vars.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/traits/next-solver/opaques/non-defining-use-hir-typeck.rs: [missing] -> pass (J1)
  • [ui] tests/ui/traits/next-solver/opaques/universal-args-non-defining.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/traits/next-solver/opaques/universal-args-non-defining.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/type-alias-impl-trait/generic_nondefining_use.rs: pass -> [missing] (J1)
  • [ui] tests/ui/type-alias-impl-trait/generic_nondefining_use.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/type-alias-impl-trait/generic_nondefining_use.rs#next: [missing] -> pass (J1)
  • [ui] tests/ui/type-alias-impl-trait/normalize-args-before-defining-use-check.rs#current: [missing] -> pass (J1)
  • [ui] tests/ui/type-alias-impl-trait/normalize-args-before-defining-use-check.rs#next: [missing] -> pass (J1)

Additionally, 12 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard d327d651e2583eb601978179f2ca9808f5e243bb --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. dist-powerpc64-linux: 10959.3s -> 6123.5s (-44.1%)
  2. pr-check-1: 1365.1s -> 1637.8s (20.0%)
  3. dist-android: 2603.1s -> 2086.8s (-19.8%)
  4. dist-apple-various: 5557.1s -> 4860.2s (-12.5%)
  5. x86_64-gnu-llvm-19: 2470.3s -> 2768.1s (12.1%)
  6. x86_64-gnu-tools: 3326.7s -> 3695.7s (11.1%)
  7. x86_64-gnu-miri: 4517.6s -> 4986.3s (10.4%)
  8. x86_64-gnu-stable: 6701.4s -> 7396.4s (10.4%)
  9. dist-aarch64-apple: 6165.2s -> 5533.4s (-10.2%)
  10. i686-gnu-2: 5382.8s -> 5933.0s (10.2%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d327d65): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 1.6%, secondary 1.6%)

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

mean range count
Regressions ❌
(primary)
2.6% [1.2%, 5.4%] 3
Regressions ❌
(secondary)
3.7% [2.3%, 5.2%] 2
Improvements ✅
(primary)
-1.5% [-1.5%, -1.5%] 1
Improvements ✅
(secondary)
-2.5% [-2.5%, -2.5%] 1
All ❌✅ (primary) 1.6% [-1.5%, 5.4%] 4

Cycles

Results (secondary -1.4%)

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)
-1.4% [-1.4%, -1.4%] 1
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 469.875s -> 466.213s (-0.78%)
Artifact size: 378.38 MiB -> 378.38 MiB (0.00%)

@lcnr lcnr deleted the non-defining-uses-hir-typeck branch August 26, 2025 08:08
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. 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.

non-defining uses with non-param args will be break not normalizing tait parameters before checking if it's a defining usage
6 participants