layout: handle rigid aliases without params#151814
layout: handle rigid aliases without params#151814rust-bors[bot] merged 3 commits intorust-lang:mainfrom
Conversation
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
lcnr
added
beta-nominated
lcnr
force-pushed
the
silent-layout-error
branch
2 times, most recently
from
bb2a5e1 to
3b7aeda
Compare
lcnr
force-pushed
the
silent-layout-error
branch
from
3b7aeda to
4bb3f0a
Compare
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
T-rustdoc-frontend
lcnr
force-pushed
the
silent-layout-error
branch
from
27d6dbd to
0035ae3
Compare
| // Due to trivial bounds, this can even be the case if the alias does not reference | ||
| // any generic parameters, e.g. a `for<'a> u32: Trait<'a>` where-bound means that | ||
| // `<u32 as Trait<'static>>::Assoc` is rigid. | ||
| let err = if ty.has_param() || !cx.typing_env.param_env.caller_bounds().is_empty() { |
There was a problem hiding this comment.
This seems like a weird fix? Like, just because there were where clauses, doesn't mean they they were even relevant to the alias, right? Like, this would mean that <u32 as Trait<'static>>::Assoc would be "too generic", if there is a where clause like where u8: Foo, but "unknown" if there isn't that bound, even though that where clause seemingly makes no difference?
I'm not sure I have a better easy solution in mind. As a heuristic, we could see if there is a bound like X: Y<...> if the alias is <X as Y<...>>::Z - this is not quite enough, because that doesn't guarantee that it could be normalized even if not too-generic, but is better than a blank check.
There was a problem hiding this comment.
I agree, I don't care as the else branch is actually unreachable as normalization should have failed for non-wf aliases, so the only way you can get an alias in layout_of is if it is still rigid.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
There was a problem hiding this comment.
Before 5dfbf67 (the commit that revealed the bug this fixes) we had this in the middle of borrowck:
&Rvalue::NullaryOp(NullOp::SizeOf | NullOp::AlignOf, ty) => {
let trait_ref =
ty::TraitRef::new(tcx, tcx.require_lang_item(LangItem::Sized, span), [ty]);
self.prove_trait_ref(
trait_ref,
location.to_locations(),
ConstraintCategory::SizedBound,
);
}Was that .prove_trait_ref(..., ..., ConstraintCategory::SizedBound) what was "rescuing" this code from having to think about this?
There was a problem hiding this comment.
I don't think so, the code broken by his compiled previously and this check could have at most resulted in errors.
The reason this regressed is that we've added new associated constants to the standard library causing us to evaluate things and to check their layout in more places
aliases may be rigid even if they don't reference params. If the alias isn't well-formed, trying to normalize it as part of the input should have already failed
lcnr
force-pushed
the
silent-layout-error
branch
from
0035ae3 to
337abba
Compare
|
So, r=me if you aren't concerned @lcnr |
|
The only thing this accepting too much code is that we are not guaranteed to ICE when encountering a bug. That seems acceptable to me. @bors r=jackh726 |
rust-bors
bot
added
S-waiting-on-bors
layout: handle rigid aliases without params fixes rust-lang#151791 r? types
layout: handle rigid aliases without params fixes rust-lang#151791 r? types
layout: handle rigid aliases without params fixes rust-lang#151791 r? types
Rollup of 17 pull requests Successful merges: - #142415 (Add note when inherent impl for a alias type defined outside of the crate) - #142680 (Fix passing/returning structs with the 64-bit SPARC ABI) - #150768 (Don't compute FnAbi for LLVM intrinsics in backends) - #151152 (Add FCW for derive helper attributes that will conflict with built-in attributes) - #151814 (layout: handle rigid aliases without params) - #151863 (Borrowck: simplify diagnostics for placeholders) - #152159 (Add note for `?Sized` params in int-ptr casts diag) - #152434 (Clarify names of `QueryVTable` functions for "executing" a query) - #152478 (Remove tm_factory field from CodegenContext) - #152498 (Partially revert "resolve: Update `NameBindingData::vis` in place") - #152316 (fix: add continue) - #152394 (Correctly check if a macro call is actually a macro call in rustdoc highlighter) - #152425 (Port #![test_runner] to the attribute parser) - #152481 (Use cg_ssa's produce_final_output_artifacts in cg_clif) - #152485 (fix issue#152482) - #152495 (Clean up some subdiagnostics) - #152502 (Implement `BinaryHeap::from_raw_vec`)
|
Beta backport approved as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels handled by them. It didn't make to the dot release 1.93.1 but maybe we're happier with this baking a little bit before backporting to a dot release ( @rustbot label +beta-accepted |
rustbot
added
the
beta-accepted
cuviper
removed
the
beta-nominated
[beta] backports, plus stable versions in stdarch - Replace `stdarch` version placeholders with 1.94 - Parse ident with allowing recovery when trying to diagnose #151249 - Revert enabling `outline-atomics` on various platforms #151896 - Revert doc attribute parsing errors to future warnings #151952 - Remove the 4 failing tests from rustdoc-gui #152194 - Remove rustdoc GUI flaky test #152116 - Align `ArrayWindows` trait impls with `Windows` #151613 - Fix suppression of `unused_assignment` in binding of `unused_variable` #151556 - layout: handle rigid aliases without params #151814 - Fix missing unused_variables lint when using a match guard #151990 - Partially revert "resolve: Update `NameBindingData::vis` in place" #152498 - [BETA]: parse array lengths without stripping const blocks #152237 r? cuviper
7 participants
fixes #151791
r? types