-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Assemble const bounds via normal item bounds in old solver too #143235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
trait Baz: const Bar {} | ||
|
||
trait Foo { | ||
// Well-formedenss of `Baz` requires `<Self as Foo>::Bar: const Bar`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Well-formedenss of `Baz` requires `<Self as Foo>::Bar: const Bar`. | |
// Well-formedness of `Baz` requires `<Self as Foo>::Bar: const Bar`. |
} | ||
|
||
let is_match = | ||
infcx.probe(|_| match_candidate(selcx, obligation, data, true, |_, _| {}).is_ok()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
infcx.probe(|_| match_candidate(selcx, obligation, data, true, |_, _| {}).is_ok()); | |
infcx.probe(|_| match_candidate(selcx, obligation, data, false, |_, _| {}).is_ok()); |
In trying to use this patch locally with my experimental const sizedness changes, I find that I get errors in libcore
when this does normalisation.
I don't have a minimal example unfortunately, but the issue was when:
obligation.predicate.trait_ref
is<<char as str::pattern::Pattern>::Searcher<'_> as marker::MetaSized>
data.trait_ref
was<<char as str::pattern::Pattern>::Searcher<'_> as marker::MetaSized>
- ..then normalised to
<str::pattern::CharSearcher<'_> as marker::MetaSized>
- ..then normalised to
- ..and then
is_match
ends up beingfalse
I don't know that it's the correct solution but changing this to false
allows the crate to compile successfully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, that change is not right in general and would cause additional failures later on since predicates that we get from the item bounds need to be normalized after substitution.
I think you should identify where the error was being emitted, what phase/step of checking we're doing, the param-env, why the goal predicate is not being normalized, etc, since it doesn't have much to do with this PR I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went back and looked into this further, much easier now I've got a sense of what to look for.
Unnormalised obligations were coming from the builtin impl for const sizedness that I was adding, in particular when the final field of a struct was a projection. On that branch, I've started normalising the obligations in evaluate_host_effect_from_builtin_impls
, which is similar to what ends up happening for trait predicates in confirm_builtin_candidate
after the built-in impl for sizedness there, so it seems like a reasonable thing to do.
I would submit a patch for that separately but not sure how to craft a test case for it with only the builtin impl for Destruct
that is in-tree today, so will keep it in my branch for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidtwco: You can just put up a PR without a test. I can either come up with a test or I'll approve it without one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I still need to work out some issues with the change and recursive structs, but will put a patch up after resolving that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
let mut consider_ty = obligation.predicate.self_ty(); | ||
while let ty::Alias(kind @ (ty::Projection | ty::Opaque), alias_ty) = *consider_ty.kind() { | ||
for clause in tcx.item_bounds(alias_ty.def_id).iter_instantiated(tcx, alias_ty.args) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why can't this use the is_conditionally_const
"fast path"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the constness of these item bounds is not dependent on the constness of the associated item. If I have:
trait Foo {
type Assoc: const A + ~const B;
}
<T as Foo>::Assoc
is always const A
, but only const B
if T: const Foo
.
@bors r+ |
Rollup of 11 pull requests Successful merges: - #131923 (Derive `Copy` and `Hash` for `IntErrorKind`) - #138340 (Remove some unsized tuple impls now that we don't support unsizing tuples anymore) - #141219 (Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global`) - #142212 (bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends`) - #142237 (Detect more cases of unused_parens around types) - #142964 (Attribute rework: a parser for single attributes without arguments) - #143070 (Rewrite `macro_rules!` parser to not use the MBE engine itself) - #143235 (Assemble const bounds via normal item bounds in old solver too) - #143261 (Feed `explicit_predicates_of` instead of `predicates_of`) - #143276 (loop match: handle opaque patterns) - #143306 (Add `track_caller` attributes to trace origin of Clippy lints) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various
Rollup of 11 pull requests Successful merges: - #131923 (Derive `Copy` and `Hash` for `IntErrorKind`) - #138340 (Remove some unsized tuple impls now that we don't support unsizing tuples anymore) - #141219 (Change `{Box,Arc,Rc,Weak}::into_raw` to only work with `A = Global`) - #142212 (bootstrap: validate `rust.codegen-backends` & `target.<triple>.codegen-backends`) - #142237 (Detect more cases of unused_parens around types) - #142964 (Attribute rework: a parser for single attributes without arguments) - #143070 (Rewrite `macro_rules!` parser to not use the MBE engine itself) - #143235 (Assemble const bounds via normal item bounds in old solver too) - #143261 (Feed `explicit_predicates_of` instead of `predicates_of`) - #143276 (loop match: handle opaque patterns) - #143306 (Add `track_caller` attributes to trace origin of Clippy lints) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-apple try-job: x86_64-msvc-1 try-job: x86_64-gnu try-job: dist-i586-gnu-i586-i686-musl try-job: test-various
Rollup merge of #143235 - compiler-errors:const-item-bound, r=oli-obk Assemble const bounds via normal item bounds in old solver too Fixes the first example in https://rust-lang.zulipchat.com/#narrow/channel/144729-t-types/topic/elaboration.20of.20const.20bounds.3F/with/526378135 The code duplication here is not that nice, but it's at least very localized. cc `@davidtwco` r? oli-obk
Fixes the first example in https://rust-lang.zulipchat.com/#narrow/channel/144729-t-types/topic/elaboration.20of.20const.20bounds.3F/with/526378135
The code duplication here is not that nice, but it's at least very localized.
cc @davidtwco
r? oli-obk