Skip to content

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

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
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
test [crashes] tests/crashes/131373.rs ... ok
test [crashes] tests/crashes/131886.rs ... ok
test [crashes] tests/crashes/132126.rs ... ok
test [crashes] tests/crashes/132960.rs ... ok
2025-11-12T17:46:04.636491Z ERROR compiletest::runtest: fatal error, panic: "crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`."
test [crashes] tests/crashes/132765.rs ... ok
test [crashes] tests/crashes/133965.rs ... ignored, ignored if rustc wasn't built with debug assertions
test [crashes] tests/crashes/133613.rs ... FAILED
test [crashes] tests/crashes/134061.rs ... ignored, ignored if rustc wasn't built with debug assertions
test [crashes] tests/crashes/134479.rs ... ignored, gcc backend is marked as ignore
---

---- [crashes] tests/crashes/133613.rs stdout ----
------rustc stdout------------------------------

------rustc stderr------------------------------
error[E0658]: return type notation is experimental
##[error] --> /checkout/tests/crashes/133613.rs:6:47
  |
6 |     fn stream(&self) -> impl IntFactory<stream(..): IntFactory<stream(..): Send>>;
  |                                               ^^^^
  |
  = note: see issue #109417 <https://github.com/rust-lang/rust/issues/109417> for more information
  = help: add `#![feature(return_type_notation)]` to the crate attributes to enable
  = note: this compiler was built on 2025-11-12; consider upgrading it if it is out of date

error[E0658]: return type notation is experimental
##[error] --> /checkout/tests/crashes/133613.rs:6:70
  |
6 |     fn stream(&self) -> impl IntFactory<stream(..): IntFactory<stream(..): Send>>;
  |                                                                      ^^^^
  |
  = note: see issue #109417 <https://github.com/rust-lang/rust/issues/109417> for more information
  = help: add `#![feature(return_type_notation)]` to the crate attributes to enable
  = note: this compiler was built on 2025-11-12; consider upgrading it if it is out of date
---
  |
3 | struct Wrapper<'a>();
  |                ^^ unused lifetime parameter
  |
  = help: consider removing `'a`, referring to it in a field, or using a marker such as `PhantomData`

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0392, E0601, E0658.
For more information about an error, try `rustc --explain E0392`.

------------------------------------------

error: crashtest no longer crashes/triggers ICE, hooray! Please give it a meaningful name, add a doc-comment to the start of the test explaining why it exists and move it to tests/ui or wherever you see fit. Adding 'Fixes #<issueNr>' to your PR description ensures that the corresponding ticket is auto-closed upon merge. If you want to see verbose output, set `COMPILETEST_VERBOSE_CRASHES=1`.

thread '[crashes] tests/crashes/133613.rs' panicked at src/tools/compiletest/src/runtest/crashes.rs:17:18:
fatal error
stack backtrace:
   5: __rustc::rust_begin_unwind

For more information how to resolve CI failures of this job, visit this link.

@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

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
Copy link
Collaborator

🚧 Experiment pr-148477-1 is now running

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I-types-nominated Nominated for discussion during a types team meeting. S-waiting-on-crater Status: Waiting on a crater run to be completed. 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.

6 participants