Rigidly project missing item due to guaranteed impossible sized predicate #139000
Conversation
rustbot
added
S-waiting-on-review
compiler/rustc_next_trait_solver/src/solve/normalizes_to/mod.rs
Outdated
Show resolved
Hide resolved
|
A somewhat annoying issue 🤔 the fix feels quite cumbersome and slightly brittle to me. I think the delayed bug doesn't provide too much value. We now do allow missing associated items in some cases. Instead of just using I think returning a rigid alias during coherence is unsound here, with the following setup:
So we should return ambiguity if we're in coherence |
|
Maybe add a function |
|
This can only occur when there's an impossible predicate right? I wouldn't expect coherence to matter as it would need to involve some kind of edit: yeah i spent more time thinking about this and I am not sure what example you're thinking of 🤷♀️ |
|
Yeah, I'm not sure how this could be turned into a coherence bug. I did pull this out into a query since it's nicer than having to recompute the struct tail, especially in the new trait solver. @bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
compiler-errors
force-pushed
the
rigid-missing-item
branch
from
b893193 to
77e2d4a
Compare
Rigidly project missing item due to guaranteed impossible sized predicate This is a somewhat involved change, but it amounts to treating missing impl items due to guaranteed impossible where clauses (dyn/str/slice sized, cc rust-lang#135480) as *rigid projections* rather than projecting to an error term, since that was preventing either reporting a proper error (in an empty param env) *or* successfully type checking the code (in the presence of trivially false where clauses). Fixes rust-lang#138970 r? `@lcnr` `@oli-obk`
|
ugh race condition @bors r- retry |
bors
added
S-waiting-on-author
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…r=<try> Rigidly project missing item due to guaranteed impossible sized predicate This is a somewhat involved change, but it amounts to treating missing impl items due to guaranteed impossible where clauses (dyn/str/slice sized, cc rust-lang#135480) as *rigid projections* rather than projecting to an error term, since that was preventing either reporting a proper error (in an empty param env) *or* successfully type checking the code (in the presence of trivially false where clauses). Fixes rust-lang#138970 r? `@lcnr` `@oli-obk`
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (a640b79): comparison URL. Overall result: ❌ regressions - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (secondary 3.8%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 777.614s -> 777.094s (-0.07%) |
rustbot
removed
the
S-waiting-on-perf
|
r=me on the impl, checking it out locally to play with my concern wrt to correctness in coherence |
|
wrt to the correctness issue in coherence. I was thinking of // overlap error on nightly, compiles with this PR
struct MaybeUnsized<T: Overlap<U>, U>(<T as Overlap<U>>::MaybeUnsized);
trait ReqSized {
type Missing1
where
Self: Sized;
type Missing2
where
Self: Sized;
}
impl<T> ReqSized for MaybeUnsized<T, u32> {}
struct W<T: ?Sized>(T);
trait Eq<T> {}
impl<T> Eq<T> for W<T> {}
trait RelateReqSized {}
impl<T: ReqSized> RelateReqSized for T
where
W<T::Missing1>: Eq<T::Missing2> {}
// These two impls overlap if `MaybeUnsized<u32, u32>: Sized` holds
// and `T::Missing1` and `T::Missing2` are the same type.
//
// The check that that `MaybeUnsized<u32, u32>` is not `Sized`
// is done in a generic context where we just use the first impl. So
// even though the second impl would also be applicable, we treat the
// missing associated items as rigid.
trait Overlap<U> {
type MaybeUnsized: ?Sized;
}
impl<T> Overlap<u32> for T {
type MaybeUnsized = str;
}
impl<U> Overlap<U> for u32
where
MaybeUnsized<U, u32>: RelateReqSized,
{
type MaybeUnsized = u32;
}I think it would be very difficult to exploit this issue, but I do think this behavior is buggy and it should continue to emit an overlap error. |
|
from @BoxyUwU when chatting with her in priv:
|
compiler-errors
force-pushed
the
rigid-missing-item
branch
from
77e2d4a to
7629908
Compare
…em in error reporting
compiler-errors
force-pushed
the
rigid-missing-item
branch
from
7629908 to
6cd724b
Compare
|
@bors r=lcnr |
|
☀️ Test successful - checks-actions |
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 6813f95 (parent) -> 9d28fe3 (this PR) Test differencesShow 56 test diffsStage 1
Stage 2
Additionally, 38 doctest diffs were found. These are ignored, as they are noisy. Job group index
Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
|
Finished benchmarking commit (9d28fe3): comparison URL. Overall result: ❌✅ regressions and improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -0.7%, secondary 3.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 780.636s -> 780.245s (-0.05%) |
This is a somewhat involved change, but it amounts to treating missing impl items due to guaranteed impossible where clauses (dyn/str/slice sized, cc #135480) as rigid projections rather than projecting to an error term, since that was preventing either reporting a proper error (in an empty param env) or successfully type checking the code (in the presence of trivially false where clauses).
Fixes #138970
r? @lcnr @oli-obk