Skip to content
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

Fix erroneous span for borrowck error #98022

Merged
merged 1 commit into from
Jun 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,20 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
.add_element(live_region_vid, location);
});

// HACK(compiler-errors): Constants that are gathered into Body.required_consts
// have their locations erased...
let locations = if location != Location::START {
location.to_locations()
} else {
Locations::All(constant.span)
};
Comment on lines +360 to +366
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like you've identified the right course of action (modifying Body.required_consts instead of here).

I'd be ok with landing this as a temporary hack, but I'd rather getting sign off from somebody immediately related to the const generics effort. I'd punt to @oli-obk, but he'll be somewhat unavailable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should just unconditionally use the constant's span, it seems like that span will always be better than what we can guess from the location (or we'll just end up fetching the constant's span at the location anyway). At which point it begs the question of whether we should be passing a location to visit_constant at all.

A quick skim of all visit_constant and visit_const suggest that we're only really interested in the span, not the actual mir location.

anyway. Leaving the hack comment and fixing this in a follow up PR seems the quickest way forward.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I looked into that refactoring, but we also use Location (and not Locations) as a key to some different HashMaps in different MIR stages? I'll see if I can give the refactoring another go, though.


if let Some(annotation_index) = constant.user_ty {
if let Err(terr) = self.cx.relate_type_and_user_type(
constant.literal.ty(),
ty::Variance::Invariant,
&UserTypeProjection { base: annotation_index, projs: vec![] },
location.to_locations(),
locations,
ConstraintCategory::Boring,
) {
let annotation = &self.cx.user_type_annotations[annotation_index];
Expand Down Expand Up @@ -390,12 +398,9 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
promoted: &Body<'tcx>,
ty,
san_ty| {
if let Err(terr) = verifier.cx.eq_types(
ty,
san_ty,
location.to_locations(),
ConstraintCategory::Boring,
) {
if let Err(terr) =
verifier.cx.eq_types(ty, san_ty, locations, ConstraintCategory::Boring)
{
span_mirbug!(
verifier,
promoted,
Expand All @@ -416,7 +421,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
}
} else {
if let Err(terr) = self.cx.fully_perform_op(
location.to_locations(),
locations,
ConstraintCategory::Boring,
self.cx.param_env.and(type_op::ascribe_user_type::AscribeUserType::new(
constant.literal.ty(),
Expand All @@ -435,7 +440,6 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
}
} else if let Some(static_def_id) = constant.check_static_ptr(tcx) {
let unnormalized_ty = tcx.type_of(static_def_id);
let locations = location.to_locations();
let normalized_ty = self.cx.normalize(unnormalized_ty, locations);
let literal_ty = constant.literal.ty().builtin_deref(true).unwrap().ty;

Expand All @@ -454,7 +458,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
self.cx.normalize_and_prove_instantiated_predicates(
def_id,
instantiated_predicates,
location.to_locations(),
locations,
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/hrtb/hrtb-just-for-static.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: implementation of `Foo` is not general enough
--> $DIR/hrtb-just-for-static.rs:24:5
|
LL | want_hrtb::<StaticInt>()
| ^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
| ^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
|
= note: `StaticInt` must implement `Foo<&'0 isize>`, for any lifetime `'0`...
= note: ...but it actually implements `Foo<&'static isize>`
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/nll/issue-97997.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
trait Foo {
const ASSOC: bool = true;
}
impl<T> Foo for fn(T) {}

fn foo(_x: i32) {}

fn impls_foo<T: Foo>(_x: T) {}

fn main() {
impls_foo(foo as fn(i32));

<fn(&u8) as Foo>::ASSOC;
//~^ ERROR implementation of `Foo` is not general enough
//~| ERROR implementation of `Foo` is not general enough
}
20 changes: 20 additions & 0 deletions src/test/ui/nll/issue-97997.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: implementation of `Foo` is not general enough
--> $DIR/issue-97997.rs:13:5
|
LL | <fn(&u8) as Foo>::ASSOC;
| ^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
|
= note: `Foo` would have to be implemented for the type `for<'r> fn(&'r u8)`
= note: ...but `Foo` is actually implemented for the type `fn(&'0 u8)`, for some specific lifetime `'0`

error: implementation of `Foo` is not general enough
--> $DIR/issue-97997.rs:13:5
|
LL | <fn(&u8) as Foo>::ASSOC;
| ^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
|
= note: `Foo` would have to be implemented for the type `for<'r> fn(&'r u8)`
= note: ...but `Foo` is actually implemented for the type `fn(&'0 u8)`, for some specific lifetime `'0`

error: aborting due to 2 previous errors