Skip to content

Commit

Permalink
Rollup merge of #94689 - compiler-errors:on-unimplemented-substs, r=p…
Browse files Browse the repository at this point in the history
…etrochenkov

Use impl substs in `#[rustc_on_unimplemented]`

We were using the trait-ref substs instead of impl substs in `rustc_on_unimplemented`, even when computing the `rustc_on_unimplemented` attached to an impl block. Let's not do that.

This PR also untangles impl and trait def-ids in the logic in `on_unimplemented` a bit.

Fixes #94675
  • Loading branch information
Dylan-DPC authored Mar 8, 2022
2 parents 64187b8 + b726bfb commit 568736b
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 42 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,6 @@ impl<'tcx> TraitRef<'tcx> {
substs: SubstsRef<'tcx>,
) -> ty::TraitRef<'tcx> {
let defs = tcx.generics_of(trait_id);

ty::TraitRef { def_id: trait_id, substs: tcx.intern_substs(&substs[..defs.params.len()]) }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::{
use crate::infer::InferCtxt;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::subst::{Subst, SubstsRef};
use rustc_middle::ty::{self, GenericParamDefKind};
use rustc_span::symbol::sym;
use std::iter;
Expand All @@ -17,7 +17,7 @@ crate trait InferCtxtExt<'tcx> {
&self,
trait_ref: ty::PolyTraitRef<'tcx>,
obligation: &PredicateObligation<'tcx>,
) -> Option<DefId>;
) -> Option<(DefId, SubstsRef<'tcx>)>;

/*private*/
fn describe_enclosure(&self, hir_id: hir::HirId) -> Option<&'static str>;
Expand All @@ -34,7 +34,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
&self,
trait_ref: ty::PolyTraitRef<'tcx>,
obligation: &PredicateObligation<'tcx>,
) -> Option<DefId> {
) -> Option<(DefId, SubstsRef<'tcx>)> {
let tcx = self.tcx;
let param_env = obligation.param_env;
let trait_ref = tcx.erase_late_bound_regions(trait_ref);
Expand All @@ -50,28 +50,29 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
let impl_self_ty = impl_trait_ref.self_ty();

if let Ok(..) = self.can_eq(param_env, trait_self_ty, impl_self_ty) {
self_match_impls.push(def_id);
self_match_impls.push((def_id, impl_substs));

if iter::zip(
trait_ref.substs.types().skip(1),
impl_trait_ref.substs.types().skip(1),
)
.all(|(u, v)| self.fuzzy_match_tys(u, v, false).is_some())
{
fuzzy_match_impls.push(def_id);
fuzzy_match_impls.push((def_id, impl_substs));
}
}
});

let impl_def_id = if self_match_impls.len() == 1 {
let impl_def_id_and_substs = if self_match_impls.len() == 1 {
self_match_impls[0]
} else if fuzzy_match_impls.len() == 1 {
fuzzy_match_impls[0]
} else {
return None;
};

tcx.has_attr(impl_def_id, sym::rustc_on_unimplemented).then_some(impl_def_id)
tcx.has_attr(impl_def_id_and_substs.0, sym::rustc_on_unimplemented)
.then_some(impl_def_id_and_substs)
}

/// Used to set on_unimplemented's `ItemContext`
Expand Down Expand Up @@ -120,8 +121,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
trait_ref: ty::PolyTraitRef<'tcx>,
obligation: &PredicateObligation<'tcx>,
) -> OnUnimplementedNote {
let def_id =
self.impl_similar_to(trait_ref, obligation).unwrap_or_else(|| trait_ref.def_id());
let (def_id, substs) = self
.impl_similar_to(trait_ref, obligation)
.unwrap_or_else(|| (trait_ref.def_id(), trait_ref.skip_binder().substs));
let trait_ref = trait_ref.skip_binder();

let mut flags = vec![(
Expand Down Expand Up @@ -176,15 +178,15 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
for param in generics.params.iter() {
let value = match param.kind {
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
trait_ref.substs[param.index as usize].to_string()
substs[param.index as usize].to_string()
}
GenericParamDefKind::Lifetime => continue,
};
let name = param.name;
flags.push((name, Some(value)));

if let GenericParamDefKind::Type { .. } = param.kind {
let param_ty = trait_ref.substs[param.index as usize].expect_ty();
let param_ty = substs[param.index as usize].expect_ty();
if let Some(def) = param_ty.ty_adt_def() {
// We also want to be able to select the parameter's
// original signature with no type arguments resolved
Expand Down Expand Up @@ -229,9 +231,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
});

if let Ok(Some(command)) =
OnUnimplementedDirective::of_item(self.tcx, trait_ref.def_id, def_id)
{
if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) {
command.evaluate(self.tcx, trait_ref, &flags)
} else {
OnUnimplementedNote::default()
Expand Down
46 changes: 26 additions & 20 deletions compiler/rustc_trait_selection/src/traits/on_unimplemented.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn parse_error(
impl<'tcx> OnUnimplementedDirective {
fn parse(
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
item_def_id: DefId,
items: &[NestedMetaItem],
span: Span,
is_root: bool,
Expand All @@ -63,7 +63,7 @@ impl<'tcx> OnUnimplementedDirective {
let mut item_iter = items.iter();

let parse_value = |value_str| {
OnUnimplementedFormatString::try_parse(tcx, trait_def_id, value_str, span).map(Some)
OnUnimplementedFormatString::try_parse(tcx, item_def_id, value_str, span).map(Some)
};

let condition = if is_root {
Expand Down Expand Up @@ -135,7 +135,7 @@ impl<'tcx> OnUnimplementedDirective {
{
if let Some(items) = item.meta_item_list() {
if let Ok(subcommand) =
Self::parse(tcx, trait_def_id, &items, item.span(), false)
Self::parse(tcx, item_def_id, &items, item.span(), false)
{
subcommands.push(subcommand);
} else {
Expand Down Expand Up @@ -178,27 +178,23 @@ impl<'tcx> OnUnimplementedDirective {
}
}

pub fn of_item(
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
impl_def_id: DefId,
) -> Result<Option<Self>, ErrorGuaranteed> {
let attrs = tcx.get_attrs(impl_def_id);
pub fn of_item(tcx: TyCtxt<'tcx>, item_def_id: DefId) -> Result<Option<Self>, ErrorGuaranteed> {
let attrs = tcx.get_attrs(item_def_id);

let Some(attr) = tcx.sess.find_by_name(&attrs, sym::rustc_on_unimplemented) else {
return Ok(None);
};

let result = if let Some(items) = attr.meta_item_list() {
Self::parse(tcx, trait_def_id, &items, attr.span, true).map(Some)
Self::parse(tcx, item_def_id, &items, attr.span, true).map(Some)
} else if let Some(value) = attr.value_str() {
Ok(Some(OnUnimplementedDirective {
condition: None,
message: None,
subcommands: vec![],
label: Some(OnUnimplementedFormatString::try_parse(
tcx,
trait_def_id,
item_def_id,
value,
attr.span,
)?),
Expand All @@ -209,7 +205,7 @@ impl<'tcx> OnUnimplementedDirective {
} else {
return Err(ErrorGuaranteed);
};
debug!("of_item({:?}/{:?}) = {:?}", trait_def_id, impl_def_id, result);
debug!("of_item({:?}) = {:?}", item_def_id, result);
result
}

Expand Down Expand Up @@ -280,23 +276,29 @@ impl<'tcx> OnUnimplementedDirective {
impl<'tcx> OnUnimplementedFormatString {
fn try_parse(
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
item_def_id: DefId,
from: Symbol,
err_sp: Span,
) -> Result<Self, ErrorGuaranteed> {
let result = OnUnimplementedFormatString(from);
result.verify(tcx, trait_def_id, err_sp)?;
result.verify(tcx, item_def_id, err_sp)?;
Ok(result)
}

fn verify(
&self,
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
item_def_id: DefId,
span: Span,
) -> Result<(), ErrorGuaranteed> {
let name = tcx.item_name(trait_def_id);
let generics = tcx.generics_of(trait_def_id);
let trait_def_id = if tcx.is_trait(item_def_id) {
item_def_id
} else {
tcx.trait_id_of_impl(item_def_id)
.expect("expected `on_unimplemented` to correspond to a trait")
};
let trait_name = tcx.item_name(trait_def_id);
let generics = tcx.generics_of(item_def_id);
let s = self.0.as_str();
let parser = Parser::new(s, None, None, false, ParseMode::Format);
let mut result = Ok(());
Expand All @@ -307,7 +309,7 @@ impl<'tcx> OnUnimplementedFormatString {
// `{Self}` is allowed
Position::ArgumentNamed(s, _) if s == kw::SelfUpper => (),
// `{ThisTraitsName}` is allowed
Position::ArgumentNamed(s, _) if s == name => (),
Position::ArgumentNamed(s, _) if s == trait_name => (),
// `{from_method}` is allowed
Position::ArgumentNamed(s, _) if s == sym::from_method => (),
// `{from_desugaring}` is allowed
Expand All @@ -329,9 +331,13 @@ impl<'tcx> OnUnimplementedFormatString {
tcx.sess,
span,
E0230,
"there is no parameter `{}` on trait `{}`",
"there is no parameter `{}` on {}",
s,
name
if trait_def_id == item_def_id {
format!("trait `{}`", trait_name)
} else {
"impl".to_string()
}
)
.emit();
result = Err(ErrorGuaranteed);
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_typeck/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,12 +742,11 @@ pub fn check_item_type<'tcx>(tcx: TyCtxt<'tcx>, it: &'tcx hir::Item<'tcx>) {
impl_trait_ref,
&impl_.items,
);
let trait_def_id = impl_trait_ref.def_id;
check_on_unimplemented(tcx, trait_def_id, it);
check_on_unimplemented(tcx, it);
}
}
hir::ItemKind::Trait(_, _, _, _, ref items) => {
check_on_unimplemented(tcx, it.def_id.to_def_id(), it);
check_on_unimplemented(tcx, it);

for item in items.iter() {
let item = tcx.hir().trait_item(item.id);
Expand Down Expand Up @@ -857,9 +856,9 @@ pub fn check_item_type<'tcx>(tcx: TyCtxt<'tcx>, it: &'tcx hir::Item<'tcx>) {
}
}

pub(super) fn check_on_unimplemented(tcx: TyCtxt<'_>, trait_def_id: DefId, item: &hir::Item<'_>) {
pub(super) fn check_on_unimplemented(tcx: TyCtxt<'_>, item: &hir::Item<'_>) {
// an error would be reported if this fails.
let _ = traits::OnUnimplementedDirective::of_item(tcx, trait_def_id, item.def_id.to_def_id());
let _ = traits::OnUnimplementedDirective::of_item(tcx, item.def_id.to_def_id());
}

pub(super) fn check_specialization_validity<'tcx>(
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/consts/issue-94675.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![feature(const_trait_impl, const_mut_refs)]

struct Foo<'a> {
bar: &'a mut Vec<usize>,
}

impl<'a> Foo<'a> {
const fn spam(&mut self, baz: &mut Vec<u32>) {
self.bar[0] = baz.len();
//~^ ERROR cannot call non-const fn `Vec::<u32>::len` in constant functions
//~| ERROR the trait bound `Vec<usize>: ~const IndexMut<usize>` is not satisfied
//~| ERROR cannot call non-const operator in constant functions
}
}

fn main() {}
38 changes: 38 additions & 0 deletions src/test/ui/consts/issue-94675.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
error[E0015]: cannot call non-const fn `Vec::<u32>::len` in constant functions
--> $DIR/issue-94675.rs:9:27
|
LL | self.bar[0] = baz.len();
| ^^^^^
|
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants

error[E0277]: the trait bound `Vec<usize>: ~const IndexMut<usize>` is not satisfied
--> $DIR/issue-94675.rs:9:9
|
LL | self.bar[0] = baz.len();
| ^^^^^^^^^^^ vector indices are of type `usize` or ranges of `usize`
|
= help: the trait `~const IndexMut<usize>` is not implemented for `Vec<usize>`
note: the trait `IndexMut<usize>` is implemented for `Vec<usize>`, but that implementation is not `const`
--> $DIR/issue-94675.rs:9:9
|
LL | self.bar[0] = baz.len();
| ^^^^^^^^^^^

error[E0015]: cannot call non-const operator in constant functions
--> $DIR/issue-94675.rs:9:9
|
LL | self.bar[0] = baz.len();
| ^^^^^^^^^^^
|
note: impl defined here, but it is not `const`
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
|
LL | impl<T, I: SliceIndex<[T]>, A: Allocator> IndexMut<I> for Vec<T, A> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0015, E0277.
For more information about an error, try `rustc --explain E0015`.
1 change: 1 addition & 0 deletions src/test/ui/issues/issue-87707.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// test for #87707
// edition:2018
// run-fail
// exec-env:RUST_BACKTRACE=0
// check-run-results

use std::sync::Once;
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/issues/issue-87707.run.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
thread 'main' panicked at 'Here Once instance is poisoned.', $DIR/issue-87707.rs:12:24
thread 'main' panicked at 'Here Once instance is poisoned.', $DIR/issue-87707.rs:13:24
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at 'Once instance has previously been poisoned', $DIR/issue-87707.rs:14:7
thread 'main' panicked at 'Once instance has previously been poisoned', $DIR/issue-87707.rs:15:7
15 changes: 15 additions & 0 deletions src/test/ui/on-unimplemented/impl-substs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![feature(rustc_attrs)]

trait Foo<A> {
fn foo(self);
}

#[rustc_on_unimplemented = "an impl did not match: {A} {B} {C}"]
impl<A, B, C> Foo<A> for (A, B, C) {
fn foo(self) {}
}

fn main() {
Foo::<usize>::foo((1i32, 1i32, 1i32));
//~^ ERROR the trait bound `(i32, i32, i32): Foo<usize>` is not satisfied
}
13 changes: 13 additions & 0 deletions src/test/ui/on-unimplemented/impl-substs.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error[E0277]: the trait bound `(i32, i32, i32): Foo<usize>` is not satisfied
--> $DIR/impl-substs.rs:13:23
|
LL | Foo::<usize>::foo((1i32, 1i32, 1i32));
| ----------------- ^^^^^^^^^^^^^^^^^^ an impl did not match: usize _ _
| |
| required by a bound introduced by this call
|
= help: the trait `Foo<usize>` is not implemented for `(i32, i32, i32)`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.

0 comments on commit 568736b

Please sign in to comment.