Treat types in unnormalized function signatures as well-formed #88312
Conversation
rust-highfive
added
the
S-waiting-on-review
| @@ -271,9 +274,9 @@ impl UniversalRegionRelationsBuilder<'cx, 'tcx> { | |||
| canonicalized_query: None, | |||
| } | |||
| }); | |||
| let constraints2 = self.add_implied_bounds(ty); | |||
| let constraints_implied_2 = self.add_implied_bounds(ty); | |||
There was a problem hiding this comment.
I wonder if we need this set
There was a problem hiding this comment.
In principle, it should be a subset of the other, right?
There was a problem hiding this comment.
I'm not exactly sure. I'm thinking of some variation of
trait Foo {
type Bar<'a> where Self: 'a;
fn foo(_: Self::Bar<'a>);
}
struct S<T> { ... }
impl<T> Foo for S<T> {
type Bar<'a> where Self: 'a = &'a T;
fn foo(_: Self::Bar<'a>) {}
}and trying to imagining if we could somehow run into a situation where T: 'a. But I suppose we would get that from Self: 'a
I can try to remove this (and other places where we add normalized inputs to WF tys list) and see if anything breaks.
There was a problem hiding this comment.
Okay, so we can't remove this, because of cases like
trait Foo {
type Bar;
fn foo(&self) -> &Self::Bar;
}
impl Foo for () {
type Bar = ();
fn foo(&self) ->&() {}
}rust/library/core/src/iter/adapters/mod.rs
Line 210 in 20997f6
There might be some combination of reverting the changes here that allow things to work fine, but for consistency, I think it's better to treat these all the same.
There was a problem hiding this comment.
Interesting. That example makes sense. I really want to do that "how do WF rules in a formal sense" write up.
There was a problem hiding this comment.
I guess I would say -- the example makes sense, but really it points to a shortcoming elsewhere in our system. That is, in some sense, we ought to be able to in the assumption that Self::Source: 'a and then normalize that assumption to get that S: 'a. I expect this is largely equivalent.
There was a problem hiding this comment.
Can you add a comment with this example?
|
r=me with comment |
|
@bors r=nikomatsakis |
|
📌 Commit 97bf80d has been approved by |
bors
added
S-waiting-on-bors
Treat types in unnormalized function signatures as well-formed Fixes rust-lang#87748 r? `@nikomatsakis`
Treat types in unnormalized function signatures as well-formed Fixes rust-lang#87748 r? ``@nikomatsakis``
Treat types in unnormalized function signatures as well-formed Fixes rust-lang#87748 r? ```@nikomatsakis```
|
☀️ Test successful - checks-actions |
|
@nikomatsakis @jackh726 This change lead to large regressions in instruction counts in many real world crates. The query affected by this seems to be additional calls to As part of the performance triage process, I'm marking this as a performance regression. If you believe this performance regression to be justifiable or once you have an issue or PR that addresses this regression, please mark this PR with the label perf-regression-triaged. @rustbot label +perf-regression |
|
This change certainly added more implied bounds, so there may just be more to sort through, but there may also be some oversights, like failure to remove duplicates. Merits more investigation for sure. |
|
I'll do some experiments to see if I can fix the perf regressions. |
Use FxHashSet instead of Vec for well formed tys Trying to recover perf from rust-lang#88312 r? `@ghost`
|
We won back perf in #88771. It's not 100% of what we lost, but about the best that can be done probably. |
jackh726
added
the
perf-regression-triaged
Don't treat unnormalized function arguments as well-formed Partial revert of rust-lang#88312 r? `@pnkfelix` cc `@nikomatsakis`
Don't treat unnormalized function arguments as well-formed Partial revert of rust-lang#88312 r? `@pnkfelix` cc `@nikomatsakis`
Don't treat unnormalized function arguments as well-formed Partial revert of rust-lang#88312 r? ``@pnkfelix`` cc ``@nikomatsakis``
Fixes #87748
r? @nikomatsakis