Skip to content

don't normalize where-clauses when checking well-formedness#148477

Merged
bors merged 4 commits intorust-lang:mainfrom
lqd:crater-wfcheck
Dec 12, 2025
Merged

don't normalize where-clauses when checking well-formedness#148477
bors merged 4 commits intorust-lang:mainfrom
lqd:crater-wfcheck

Conversation

@lqd
Copy link
Member

@lqd lqd commented Nov 4, 2025

WfCheck checks where-clauses after normalization, and we'd like to see what would break if it didn't for rust-lang/trait-system-refactor-initiative#255

r? ghost

@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. labels Nov 4, 2025
@lqd
Copy link
Member Author

lqd commented Nov 4, 2025

@bors try

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 4, 2025
crater: don't normalize where-clauses when checking well-formedness
@rust-bors
Copy link
Contributor

rust-bors bot commented Nov 4, 2025

☀️ Try build successful (CI)
Build commit: f3d95de (f3d95de532264a1215023baa665c0028aecf74b1, parent: 90b65889799733f21ebdf59d96411aa531c5900a)

@lqd
Copy link
Member Author

lqd commented Nov 4, 2025

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-148477 created and queued.
🤖 Automatically detected try build f3d95de
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 4, 2025
@craterbot
Copy link
Collaborator

🚧 Experiment pr-148477 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-148477 is completed!
📊 7 regressed and 7 fixed (730342 total)
📊 1888 spurious results on the retry-regessed-list.txt, consider a retry1 if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

  1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-148477/retry-regressed-list.txt

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 8, 2025
@bors
Copy link
Collaborator

bors commented Nov 9, 2025

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

@lcnr
Copy link
Contributor

lcnr commented Nov 10, 2025

Affected projects:

2 dependencies of modcholesky, same as rust-lang/trait-system-refactor-initiative#255

pub struct View<A>(A);
pub trait Data {
    type Elem;
}
impl<'a, A> Data for View<&'a A> {
    type Elem = A;
}

pub fn repro<'a, T>()
where
    <View<&'a T> as Data>::Elem: Sized,
{

https://github.com/vilicvane/unipipe/tree/9019a4b03df6ed222975b2774d686d5803860568

pub struct LifetimeTestPipe2<'a, T> {
    prefix: &'a str,
    transformer: &'a dyn Fn(&T) -> String,
    buffer: Vec<String>,
}

impl<'a, T> unipipe::UniPipe for LifetimeTestPipe2<'a, T> {
    type Input = T;
    type Output = String;

    fn next(&mut self, input: Option<Self::Input>) -> impl Into<Output<Self::Output>> {
        // ...
    }
}
#[unipipe::unipipe(iterator, try_iterator)]
impl<'a, T> LifetimeTestPipe2<'a, T> {
    pub fn new(prefix: &'a str, transformer: &'a dyn Fn(&T) -> String) -> Self {
        Self {
            prefix,
            transformer,
            buffer: Vec::new(),
        }
    }
}

expands to something containing

pub trait LifetimeTestPipe2UniPipeIteratorExt<'a, T>:
    Iterator<Item = <LifetimeTestPipe2<'a, T> as ::unipipe::UniPipe>::Input> + Sized
{
    // ...
}
impl<'a, T, TIterator> LifetimeTestPipe2UniPipeIteratorExt<'a, T> for TIterator where
    TIterator: Iterator<Item = <LifetimeTestPipe2<'a, T> as ::unipipe::UniPipe>::Input>
{
}

pub trait LifetimeTestPipe2UniPipeTryIteratorExt<'a, T, TError>:
    Iterator<Item = Result<<LifetimeTestPipe2<'a, T> as ::unipipe::UniPipe>::Input, TError>> + Sized
{
    // ...
}
impl<'a, T, TError, TIterator> LifetimeTestPipe2UniPipeTryIteratorExt<'a, T, TError> for TIterator where
    TIterator:
        Iterator<Item = Result<<LifetimeTestPipe2<'a, T> as ::unipipe::UniPipe>::Input, TError>>
{
}

That one is annoying. The macros can't really know the implied bounds of LifetimeTestPipe2, so I avoiding these errors is really challenging :<

@lcnr
Copy link
Contributor

lcnr commented Nov 10, 2025

I think we should also stop normalizing in check_associated_type_bounds before checking that the where-clauses are wf.

let wf_obligations = bounds.iter_identity_copied().flat_map(|(bound, bound_span)| {
let normalized_bound = wfcx.normalize(span, None, bound);
traits::wf::clause_obligations(
wfcx.infcx,
wfcx.param_env,
wfcx.body_def_id,
normalized_bound,
bound_span,
)
});

@lcnr lcnr added the I-types-nominated Nominated for discussion during a types team meeting. label Nov 10, 2025
@lqd
Copy link
Member Author

lqd commented Nov 12, 2025

And probably these

let pred = wfcx.normalize(sp, None, pred);
from the default args as well, right?

@rust-log-analyzer

This comment has been minimized.

@lcnr
Copy link
Contributor

lcnr commented Nov 13, 2025

wrt to normalizing obligations for default params before proving them 😅

we do need to normalize obligations before proving them in the old solver and this already doesn't normalize WellFormed obligations.

if p.allow_normalization() && needs_normalization(self.selcx.infcx, &p) {

PredicateKind::Clause(ClauseKind::WellFormed(_)) | PredicateKind::AliasRelate(..) => {
false
}

The only "issue" is when normalizing obligations (or types - which we currently sometimes do intentionally because of implied bounds stuff) before checking that the obligation is well-formed, and this only happens before calling clause_obligations. In a sense calling clause_obligations (and proving its nested obligations) is the same as a emitting a ClauseKind::WellFormed(some_where_clause) goal if we were to support that

@lcnr
Copy link
Contributor

lcnr commented Nov 13, 2025

@bors try

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 13, 2025
crater: don't normalize where-clauses when checking well-formedness
@rust-bors
Copy link
Contributor

rust-bors bot commented Nov 13, 2025

☀️ Try build successful (CI)
Build commit: 04ea1e1 (04ea1e1f1a12cfa912c228ca278237d92d0bb6df, parent: 5dbf4069dc98bbbca98dd600a65f50c258fbfd56)

@lcnr
Copy link
Contributor

lcnr commented Nov 13, 2025

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-148477-1 created and queued.
🤖 Automatically detected try build 04ea1e1
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2025
@craterbot craterbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 11, 2025
@lqd

This comment was marked as outdated.

@craterbot

This comment was marked as outdated.

@lqd

This comment was marked as resolved.

@craterbot

This comment was marked as resolved.

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2025
@craterbot

This comment was marked as resolved.

@craterbot
Copy link
Collaborator

🎉 Experiment pr-148477-3 is completed!
📊 3 regressed and 0 fixed (112 total)
📊 32 spurious results on the retry-regessed-list.txt, consider a retry1 if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

  1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-148477-3/retry-regressed-list.txt

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Dec 11, 2025
@lqd
Copy link
Member Author

lqd commented Dec 11, 2025

1769 spurious results on that crater run feels kinda high to me, would it be possible to rerun that a couple times until its like <100 or sth. i dont expect it to really change the calculus here but if it did find like One extra regression then we can open a PR

The same 3 regressions were in the new runs above, and the spurious results are down to 32.

@BoxyUwU
Copy link
Member

BoxyUwU commented Dec 11, 2025

@bors r=lcnr,BoxyUwU

@bors
Copy link
Collaborator

bors commented Dec 11, 2025

📌 Commit 99f7e78 has been approved by lcnr,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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2025
@bors
Copy link
Collaborator

bors commented Dec 12, 2025

⌛ Testing commit 99f7e78 with merge 2a3a62d...

@bors bors mentioned this pull request Dec 12, 2025
@bors
Copy link
Collaborator

bors commented Dec 12, 2025

☀️ Test successful - checks-actions
Approved by: lcnr,BoxyUwU
Pushing 2a3a62d to main...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 12, 2025
@bors bors merged commit 2a3a62d into rust-lang:main Dec 12, 2025
13 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 12, 2025
@github-actions
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 c4dc70e (parent) -> 2a3a62d (this PR)

Test differences

Show 6 test diffs

Stage 1

  • [ui] tests/ui/wf/wf-where-clauses-pre-normalization.rs: [missing] -> pass (J1)

Stage 2

  • [ui] tests/ui/wf/wf-where-clauses-pre-normalization.rs: [missing] -> pass (J0)
  • [crashes] tests/crashes/133613.rs: pass -> ignore (ignored if rustc wasn't built with debug assertions) (J2)

Additionally, 3 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 2a3a62d26e9e5badb806ac6a43bb307ff472b514 --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-apple-various: 3421.5s -> 4885.1s (+42.8%)
  2. dist-x86_64-apple: 7406.1s -> 8403.1s (+13.5%)
  3. dist-aarch64-llvm-mingw: 5521.4s -> 6184.9s (+12.0%)
  4. x86_64-gnu-gcc: 3201.3s -> 2842.0s (-11.2%)
  5. x86_64-gnu-aux: 7028.4s -> 6381.7s (-9.2%)
  6. pr-check-2: 2389.4s -> 2183.4s (-8.6%)
  7. aarch64-gnu-llvm-20-2: 2448.5s -> 2244.3s (-8.3%)
  8. pr-check-1: 1555.6s -> 1682.8s (+8.2%)
  9. tidy: 169.9s -> 156.9s (-7.6%)
  10. dist-aarch64-msvc: 6356.7s -> 5892.7s (-7.3%)
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.

@lqd lqd deleted the crater-wfcheck branch December 12, 2025 13:08
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2a3a62d): 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 0.2%, secondary 0.7%)

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

mean range count
Regressions ❌
(primary)
2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)
0.7% [0.6%, 0.7%] 2
Improvements ✅
(primary)
-1.8% [-1.8%, -1.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [-1.8%, 2.3%] 2

Cycles

Results (secondary 5.2%)

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

Binary size

Results (primary 0.0%, secondary 0.1%)

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

mean range count
Regressions ❌
(primary)
0.0% [0.0%, 0.0%] 11
Regressions ❌
(secondary)
0.1% [0.0%, 0.1%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [0.0%, 0.0%] 11

Bootstrap: 473.76s -> 472.42s (-0.28%)
Artifact size: 389.30 MiB -> 389.33 MiB (0.01%)

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Feb 5, 2026
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Mar 7, 2026
Pkgsrc changes:
 * Update version & checksums.
 * Adapt patches to new vendored crates.

This has so far just been verified to build on NetBSD/amd64.

Upstream changes relative to 1.93.1:

Version 1.94.0 (2026-03-05)
==========================

Language
--------
- [Impls and impl items inherit `dead_code` lint level of the
  corresponding traits and trait items]
  (rust-lang/rust#144113)
- [Stabilize additional 29 RISC-V target features including large
  portions of the RVA22U64 / RVA23U64 profiles]
  (rust-lang/rust#145948)
- [Add warn-by-default `unused_visibilities` lint for visibility
  on `const _` declarations]
  (rust-lang/rust#147136)
- [Update to Unicode 17]
  (rust-lang/rust#148321)
- [Avoid incorrect lifetime errors for closures]
  (rust-lang/rust#148329)

Platform Support
----------------
- [Add `riscv64im-unknown-none-elf` as a tier 3 target]
  (rust-lang/rust#148790)

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

[platform-support-doc]: https://doc.rust-lang.org/rustc/platform-support.html

Libraries
---------
- [Relax `T: Ord` bound for some `BinaryHeap<T>` methods.]
  (rust-lang/rust#149408)

Stabilized APIs
---------------
- [`<[T]>::array_windows`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.array_windows)
- [`<[T]>::element_offset`]
  (https://doc.rust-lang.org/stable/std/primitive.slice.html#method.element_offset)
- [`LazyCell::get`]
  (https://doc.rust-lang.org/stable/std/cell/struct.LazyCell.html#method.get)
- [`LazyCell::get_mut`]
  (https://doc.rust-lang.org/stable/std/cell/struct.LazyCell.html#method.get_mut)
- [`LazyCell::force_mut`]
  (https://doc.rust-lang.org/stable/std/cell/struct.LazyCell.html#method.force_mut)
- [`LazyLock::get`]
  (https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html#method.get)
- [`LazyLock::get_mut`]
  (https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html#method.get_mut)
- [`LazyLock::force_mut`]
  (https://doc.rust-lang.org/stable/std/sync/struct.LazyLock.html#method.force_mut)
- [`impl TryFrom<char> for usize`]
  (https://doc.rust-lang.org/stable/std/convert/trait.TryFrom.html#impl-TryFrom%3Cchar%3E-for-usize)
- [`std::iter::Peekable::next_if_map`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Peekable.html#method.next_if_map)
- [`std::iter::Peekable::next_if_map_mut`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Peekable.html#method.next_if_map_mut)
- [x86 `avx512fp16` intrinsics]
  (rust-lang/rust#127213)
  (excluding those that depend directly on the unstable `f16` type)
- [AArch64 NEON fp16 intrinsics]
  (rust-lang/rust#136306)
  (excluding those that depend directly on the unstable `f16` type)
- [`f32::consts::EULER_GAMMA`]
  (https://doc.rust-lang.org/stable/std/f32/consts/constant.EULER_GAMMA.html)
- [`f64::consts::EULER_GAMMA`]
  (https://doc.rust-lang.org/stable/std/f64/consts/constant.EULER_GAMMA.html)
- [`f32::consts::GOLDEN_RATIO`]
  (https://doc.rust-lang.org/stable/std/f32/consts/constant.GOLDEN_RATIO.html)
- [`f64::consts::GOLDEN_RATIO`]
  (https://doc.rust-lang.org/stable/std/f64/consts/constant.GOLDEN_RATIO.html)

These previously stable APIs are now stable in const contexts:

- [`f32::mul_add`]
  (https://doc.rust-lang.org/stable/std/primitive.f32.html#method.mul_add)
- [`f64::mul_add`]
  (https://doc.rust-lang.org/stable/std/primitive.f64.html#method.mul_add)

Cargo
-----
- Stabilize the config include key. The top-level include config
  key allows loading additional config files, enabling better
  organization, sharing, and management of Cargo configurations
  across projects and environments. [docs]
  (https://doc.rust-lang.org/nightly/cargo/reference/config.html#including-extra-configuration-files)
  [#16284] (rust-lang/cargo#16284)
- Stabilize the pubtime field in registry index. This records when
  a crate version was published and enables time-based dependency
  resolution in the future. Note that crates.io will gradually backfill
  existing packages when a new version is published. Not all crates
  have pubtime yet. [#16369]
  (rust-lang/cargo#16369) [#16372]
  (rust-lang/cargo#16372)
- Cargo now parses [TOML v1.1](https://toml.io/en/v1.1.0) for
  manifests and configuration files. Note that using these features
  in Cargo.toml will raise your development MSRV, but the published
  manifest remains compatible with older parsers. [#16415]
  (rust-lang/cargo#16415)
- [Make `CARGO_BIN_EXE_<crate>` available at runtime ]
  (rust-lang/cargo#16421)

Compatibility Notes
-------------------
- [Forbid freely casting lifetime bounds of `dyn`-types]
  (rust-lang/rust#136776)
- [Make closure capturing have consistent and correct behaviour around patterns]
  (rust-lang/rust#138961)
  Some finer details of how precise closure captures get affected
  by pattern matching have been changed. In some cases, this can
  cause a non-move closure that was previously capturing an entire
  variable by move, to now capture only part of that variable by
  move, and other parts by borrow. This can cause the borrow checker
  to complain where it previously didn't, or cause `Drop` to run
  at a different point in time.
- [Standard library macros are now imported via prelude, not via injected `
  #[macro_use]`] (rust-lang/rust#139493)
  This will raise an error if macros of the same name are glob
  imported.  For example if a crate defines their own `matches`
  macro and then glob imports that, it's now ambiguous whether the
  custom or standard library `matches` is meant and an explicit
  import of the name is required to resolve the ambiguity.  One
  exception is `core::panic` and `std::panic`, if their import is
  ambiguous a new warning ([`ambiguous_panic_imports`]
  (rust-lang/rust#147319)) is raised.
  This may raise a new warning ([`ambiguous_panic_imports`]
  (rust-lang/rust#147319)) on `#![no_std]`
  code glob importing the std crate.  Both `core::panic!` and
  `std::panic!` are then in scope and which is used is ambiguous.
- [Don't strip shebang in expression-context `include!(…)`s]
  (rust-lang/rust#146377)
  This can cause previously working includes to no longer compile
  if they included files which started with a shebang.
- [Ambiguous glob reexports are now also visible cross-crate]
  (rust-lang/rust#147984)
  This unifies behavior between local and cross-crate errors on
  these exports, which may introduce new ambiguity errors.
- [Don't normalize where-clauses before checking well-formedness]
  (rust-lang/rust#148477)
- [Introduce a future compatibility warning on codegen attributes
  on body-free trait methods]
  (rust-lang/rust#148756) These attributes
  currently have no effect in this position.
- [On Windows `std::time::SystemTime::checked_sub_duration` will
  return `None` for times before the Windows epoch (1/1/1601)]
  (rust-lang/rust#148825)
- [Lifetime identifiers such as `'a` are now NFC normalized]
  (rust-lang/rust#149192).
- [Overhaul filename handling for cross-compiler consistency]
  (rust-lang/rust#149709)
  Any paths emitted by compiler now always respect the relative-ness
  of the paths and `--remap-path-prefix` given originally.  One
  side-effect of this change is that paths emitted for local crates
  in Cargo (path dependencies and workspace members) are no longer
  absolute but relative when emitted as part of a diagnostic in a
  downstream crate.

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

- [Switch to `annotate-snippets` for error emission]
  (rust-lang/rust#150032)
  This should preserve mostly the same outputs in rustc error messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. 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-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.