Rollup merge of #70970 - eddyb:trait-vs-impl-mismatch, r=oli-obk

Detect mistyped associated consts in `Instance::resolve`.

*Based on #71049 to prevent redundant/misleading downstream errors.*

Fixes #70942 by refusing to resolve an associated `const` if it doesn't have the same type in the `impl` that it does in the `trait` (which we assume had errored, and `delay_span_bug` guards against bugs).
This commit is contained in:
Dylan DPC 2020-04-22 12:18:31 +02:00 committed by GitHub
commit 707004c552
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 157 additions and 67 deletions

View File

@ -4295,6 +4295,7 @@ version = "0.0.0"
dependencies = [
"log",
"rustc_data_structures",
"rustc_errors",
"rustc_hir",
"rustc_infer",
"rustc_middle",

View File

@ -376,6 +376,7 @@ impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> {
def_id,
tcx.intern_substs(&[]),
)
.unwrap()
.unwrap(),
),
_ => {

View File

@ -465,6 +465,7 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
start_def_id,
cx.tcx().intern_substs(&[main_ret_ty.into()]),
)
.unwrap()
.unwrap(),
);
(

View File

@ -537,6 +537,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
ty::FnDef(def_id, substs) => (
Some(
ty::Instance::resolve(bx.tcx(), ty::ParamEnv::reveal_all(), def_id, substs)
.unwrap()
.unwrap(),
),
None,

View File

@ -39,12 +39,13 @@ impl<'tcx> TyCtxt<'tcx> {
promoted: Option<mir::Promoted>,
span: Option<Span>,
) -> ConstEvalResult<'tcx> {
let instance = ty::Instance::resolve(self, param_env, def_id, substs);
if let Some(instance) = instance {
let cid = GlobalId { instance, promoted };
self.const_eval_global_id(param_env, cid, span)
} else {
Err(ErrorHandled::TooGeneric)
match ty::Instance::resolve(self, param_env, def_id, substs) {
Ok(Some(instance)) => {
let cid = GlobalId { instance, promoted };
self.const_eval_global_id(param_env, cid, span)
}
Ok(None) => Err(ErrorHandled::TooGeneric),
Err(error_reported) => Err(ErrorHandled::Reported(error_reported)),
}
}

View File

@ -679,7 +679,7 @@ rustc_queries! {
Codegen {
query codegen_fulfill_obligation(
key: (ty::ParamEnv<'tcx>, ty::PolyTraitRef<'tcx>)
) -> Option<Vtable<'tcx, ()>> {
) -> Result<Vtable<'tcx, ()>, ErrorReported> {
cache_on_disk_if { true }
desc { |tcx|
"checking if `{}` fulfills its obligations",
@ -1258,8 +1258,19 @@ rustc_queries! {
desc { "looking up enabled feature gates" }
}
query resolve_instance(key: (ty::ParamEnv<'tcx>, DefId, SubstsRef<'tcx>)) -> Option<ty::Instance<'tcx>> {
desc { "resolving instance `{:?}` `{:?}` with {:?}", key.1, key.2, key.0 }
/// Attempt to resolve the given `DefId` to an `Instance`, for the
/// given generics args (`SubstsRef`), returning one of:
/// * `Ok(Some(instance))` on success
/// * `Ok(None)` when the `SubstsRef` are still too generic,
/// and therefore don't allow finding the final `Instance`
/// * `Err(ErrorReported)` when the `Instance` resolution process
/// couldn't complete due to errors elsewhere - this is distinct
/// from `Ok(None)` to avoid misleading diagnostics when an error
/// has already been/will be emitted, for the original cause
query resolve_instance(
key: ty::ParamEnvAnd<'tcx, (DefId, SubstsRef<'tcx>)>
) -> Result<Option<ty::Instance<'tcx>>, ErrorReported> {
desc { "resolving instance `{}`", ty::Instance::new(key.value.0, key.value.1) }
}
}
}

View File

@ -1,6 +1,7 @@
use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use crate::ty::print::{FmtPrinter, Printer};
use crate::ty::{self, SubstsRef, Ty, TyCtxt, TypeFoldable};
use rustc_errors::ErrorReported;
use rustc_hir::def::Namespace;
use rustc_hir::def_id::{CrateNum, DefId};
use rustc_hir::lang_items::DropInPlaceFnLangItem;
@ -268,29 +269,41 @@ impl<'tcx> Instance<'tcx> {
/// this is used to find the precise code that will run for a trait method invocation,
/// if known.
///
/// Returns `None` if we cannot resolve `Instance` to a specific instance.
/// Returns `Ok(None)` if we cannot resolve `Instance` to a specific instance.
/// For example, in a context like this,
///
/// ```
/// fn foo<T: Debug>(t: T) { ... }
/// ```
///
/// trying to resolve `Debug::fmt` applied to `T` will yield `None`, because we do not
/// trying to resolve `Debug::fmt` applied to `T` will yield `Ok(None)`, because we do not
/// know what code ought to run. (Note that this setting is also affected by the
/// `RevealMode` in the parameter environment.)
///
/// Presuming that coherence and type-check have succeeded, if this method is invoked
/// in a monomorphic context (i.e., like during codegen), then it is guaranteed to return
/// `Some`.
/// `Ok(Some(instance))`.
///
/// Returns `Err(ErrorReported)` when the `Instance` resolution process
/// couldn't complete due to errors elsewhere - this is distinct
/// from `Ok(None)` to avoid misleading diagnostics when an error
/// has already been/will be emitted, for the original cause
pub fn resolve(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
def_id: DefId,
substs: SubstsRef<'tcx>,
) -> Option<Instance<'tcx>> {
) -> Result<Option<Instance<'tcx>>, ErrorReported> {
// All regions in the result of this query are erased, so it's
// fine to erase all of the input regions.
tcx.resolve_instance((tcx.erase_regions(&param_env), def_id, tcx.erase_regions(&substs)))
// HACK(eddyb) erase regions in `substs` first, so that `param_env.and(...)`
// below is more likely to ignore the bounds in scope (e.g. if the only
// generic parameters mentioned by `substs` were lifetime ones).
let substs = tcx.erase_regions(&substs);
// FIXME(eddyb) should this always use `param_env.with_reveal_all()`?
tcx.resolve_instance(tcx.erase_regions(&param_env.and((def_id, substs))))
}
pub fn resolve_for_fn_ptr(
@ -300,7 +313,7 @@ impl<'tcx> Instance<'tcx> {
substs: SubstsRef<'tcx>,
) -> Option<Instance<'tcx>> {
debug!("resolve(def_id={:?}, substs={:?})", def_id, substs);
Instance::resolve(tcx, param_env, def_id, substs).map(|mut resolved| {
Instance::resolve(tcx, param_env, def_id, substs).ok().flatten().map(|mut resolved| {
match resolved.def {
InstanceDef::Item(def_id) if resolved.def.requires_caller_location(tcx) => {
debug!(" => fn pointer created for function with #[track_caller]");
@ -332,7 +345,7 @@ impl<'tcx> Instance<'tcx> {
debug!(" => associated item with unsizeable self: Self");
Some(Instance { def: InstanceDef::VtableShim(def_id), substs })
} else {
Instance::resolve(tcx, param_env, def_id, substs)
Instance::resolve(tcx, param_env, def_id, substs).ok().flatten()
}
}
@ -353,7 +366,7 @@ impl<'tcx> Instance<'tcx> {
pub fn resolve_drop_in_place(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> ty::Instance<'tcx> {
let def_id = tcx.require_lang_item(DropInPlaceFnLangItem, None);
let substs = tcx.intern_substs(&[ty.into()]);
Instance::resolve(tcx, ty::ParamEnv::reveal_all(), def_id, substs).unwrap()
Instance::resolve(tcx, ty::ParamEnv::reveal_all(), def_id, substs).unwrap().unwrap()
}
pub fn fn_once_adapter_instance(

View File

@ -296,14 +296,3 @@ impl Key for (Symbol, u32, u32) {
DUMMY_SP
}
}
impl<'tcx> Key for (ty::ParamEnv<'tcx>, DefId, SubstsRef<'tcx>) {
type CacheSelector = DefaultCacheSelector;
fn query_crate(&self) -> CrateNum {
self.1.krate
}
fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
tcx.def_span(self.1)
}
}

View File

@ -461,8 +461,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
trace!("resolve: {:?}, {:#?}", def_id, substs);
trace!("param_env: {:#?}", self.param_env);
trace!("substs: {:#?}", substs);
ty::Instance::resolve(*self.tcx, self.param_env, def_id, substs)
.ok_or_else(|| err_inval!(TooGeneric).into())
match ty::Instance::resolve(*self.tcx, self.param_env, def_id, substs) {
Ok(Some(instance)) => Ok(instance),
Ok(None) => throw_inval!(TooGeneric),
// FIXME(eddyb) this could be a bit more specific than `TypeckError`.
Err(error_reported) => throw_inval!(TypeckError(error_reported)),
}
}
pub fn layout_of_local(

View File

@ -674,9 +674,12 @@ fn visit_fn_use<'tcx>(
output: &mut Vec<MonoItem<'tcx>>,
) {
if let ty::FnDef(def_id, substs) = ty.kind {
let resolver =
if is_direct_call { ty::Instance::resolve } else { ty::Instance::resolve_for_fn_ptr };
let instance = resolver(tcx, ty::ParamEnv::reveal_all(), def_id, substs).unwrap();
let instance = if is_direct_call {
ty::Instance::resolve(tcx, ty::ParamEnv::reveal_all(), def_id, substs).unwrap().unwrap()
} else {
ty::Instance::resolve_for_fn_ptr(tcx, ty::ParamEnv::reveal_all(), def_id, substs)
.unwrap()
};
visit_instance_use(tcx, instance, is_direct_call, output);
}
}
@ -1056,6 +1059,7 @@ impl RootCollector<'_, 'v> {
start_def_id,
self.tcx.intern_substs(&[main_ret_ty.into()]),
)
.unwrap()
.unwrap();
self.output.push(create_fn_mono_item(start_instance));
@ -1111,8 +1115,9 @@ fn create_mono_items_for_default_impls<'tcx>(
trait_ref.substs[param.index as usize]
}
});
let instance =
ty::Instance::resolve(tcx, param_env, method.def_id, substs).unwrap();
let instance = ty::Instance::resolve(tcx, param_env, method.def_id, substs)
.unwrap()
.unwrap();
let mono_item = create_fn_mono_item(instance);
if mono_item.is_instantiable(tcx) && should_monomorphize_locally(tcx, &instance)

View File

@ -18,7 +18,7 @@ pub fn custom_coerce_unsize_info<'tcx>(
});
match tcx.codegen_fulfill_obligation((ty::ParamEnv::reveal_all(), trait_ref)) {
Some(traits::VtableImpl(traits::VtableImplData { impl_def_id, .. })) => {
Ok(traits::VtableImpl(traits::VtableImplData { impl_def_id, .. })) => {
tcx.coerce_unsized_info(impl_def_id).custom_kind.unwrap()
}
vtable => {

View File

@ -522,7 +522,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
if self.tcx.features().const_trait_impl {
let instance = Instance::resolve(self.tcx, self.param_env, def_id, substs);
debug!("Resolving ({:?}) -> {:?}", def_id, instance);
if let Some(func) = instance {
if let Ok(Some(func)) = instance {
if let InstanceDef::Item(def_id) = func.def {
if is_const_fn(self.tcx, def_id) {
return;

View File

@ -176,7 +176,8 @@ impl Inliner<'tcx> {
let terminator = bb_data.terminator();
if let TerminatorKind::Call { func: ref op, .. } = terminator.kind {
if let ty::FnDef(callee_def_id, substs) = op.ty(caller_body, self.tcx).kind {
let instance = Instance::resolve(self.tcx, param_env, callee_def_id, substs)?;
let instance =
Instance::resolve(self.tcx, param_env, callee_def_id, substs).ok().flatten()?;
if let InstanceDef::Virtual(..) = instance.def {
return None;

View File

@ -72,7 +72,7 @@ impl<'mir, 'tcx> Search<'mir, 'tcx> {
let func_ty = func.ty(body, tcx);
if let ty::FnDef(fn_def_id, substs) = func_ty.kind {
let (call_fn_id, call_substs) =
if let Some(instance) = Instance::resolve(tcx, param_env, fn_def_id, substs) {
if let Ok(Some(instance)) = Instance::resolve(tcx, param_env, fn_def_id, substs) {
(instance.def_id(), instance.substs)
} else {
(fn_def_id, substs)

View File

@ -7,6 +7,7 @@ use crate::infer::{InferCtxt, TyCtxtInferExt};
use crate::traits::{
FulfillmentContext, Obligation, ObligationCause, SelectionContext, TraitEngine, Vtable,
};
use rustc_errors::ErrorReported;
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::{self, TyCtxt};
@ -19,7 +20,7 @@ use rustc_middle::ty::{self, TyCtxt};
pub fn codegen_fulfill_obligation<'tcx>(
ty: TyCtxt<'tcx>,
(param_env, trait_ref): (ty::ParamEnv<'tcx>, ty::PolyTraitRef<'tcx>),
) -> Option<Vtable<'tcx, ()>> {
) -> Result<Vtable<'tcx, ()>, ErrorReported> {
// Remove any references to regions; this helps improve caching.
let trait_ref = ty.erase_regions(&trait_ref);
@ -55,7 +56,7 @@ pub fn codegen_fulfill_obligation<'tcx>(
trait_ref
),
);
return None;
return Err(ErrorReported);
}
Err(e) => {
bug!("Encountered error `{:?}` selecting `{:?}` during codegen", e, trait_ref)
@ -75,7 +76,7 @@ pub fn codegen_fulfill_obligation<'tcx>(
let vtable = drain_fulfillment_cx_or_panic(&infcx, &mut fulfill_cx, &vtable);
info!("Cache miss: {:?} => {:?}", trait_ref, vtable);
Some(vtable)
Ok(vtable)
})
}

View File

@ -12,6 +12,7 @@ path = "lib.rs"
log = "0.4"
rustc_middle = { path = "../librustc_middle" }
rustc_data_structures = { path = "../librustc_data_structures" }
rustc_errors = { path = "../librustc_errors" }
rustc_hir = { path = "../librustc_hir" }
rustc_infer = { path = "../librustc_infer" }
rustc_span = { path = "../librustc_span" }

View File

@ -1,3 +1,4 @@
use rustc_errors::ErrorReported;
use rustc_hir::def_id::DefId;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::ty::subst::SubstsRef;
@ -9,10 +10,12 @@ use traits::{translate_substs, Reveal};
use log::debug;
pub fn resolve_instance<'tcx>(
fn resolve_instance<'tcx>(
tcx: TyCtxt<'tcx>,
(param_env, def_id, substs): (ty::ParamEnv<'tcx>, DefId, SubstsRef<'tcx>),
) -> Option<Instance<'tcx>> {
key: ty::ParamEnvAnd<'tcx, (DefId, SubstsRef<'tcx>)>,
) -> Result<Option<Instance<'tcx>>, ErrorReported> {
let (param_env, (def_id, substs)) = key.into_parts();
debug!("resolve(def_id={:?}, substs={:?})", def_id, substs);
let result = if let Some(trait_def_id) = tcx.trait_of_item(def_id) {
debug!(" => associated item, attempting to find impl in param_env {:#?}", param_env);
@ -38,7 +41,7 @@ pub fn resolve_instance<'tcx>(
if ty.needs_drop(tcx, param_env) {
// `DropGlue` requires a monomorphic aka concrete type.
if ty.needs_subst() {
return None;
return Ok(None);
}
debug!(" => nontrivial drop glue");
@ -53,7 +56,7 @@ pub fn resolve_instance<'tcx>(
ty::InstanceDef::Item(def_id)
}
};
Some(Instance { def, substs })
Ok(Some(Instance { def, substs }))
};
debug!("resolve(def_id={:?}, substs={:?}) = {:?}", def_id, substs, result);
result
@ -65,7 +68,7 @@ fn resolve_associated_item<'tcx>(
param_env: ty::ParamEnv<'tcx>,
trait_id: DefId,
rcvr_substs: SubstsRef<'tcx>,
) -> Option<Instance<'tcx>> {
) -> Result<Option<Instance<'tcx>>, ErrorReported> {
let def_id = trait_item.def_id;
debug!(
"resolve_associated_item(trait_item={:?}, \
@ -80,7 +83,7 @@ fn resolve_associated_item<'tcx>(
// Now that we know which impl is being used, we can dispatch to
// the actual function:
match vtbl {
Ok(match vtbl {
traits::VtableImpl(impl_data) => {
debug!(
"resolving VtableImpl: {:?}, {:?}, {:?}, {:?}",
@ -92,13 +95,11 @@ fn resolve_associated_item<'tcx>(
let trait_def_id = tcx.trait_id_of_impl(impl_data.impl_def_id).unwrap();
let trait_def = tcx.trait_def(trait_def_id);
let leaf_def = trait_def
.ancestors(tcx, impl_data.impl_def_id)
.ok()?
.ancestors(tcx, impl_data.impl_def_id)?
.leaf_def(tcx, trait_item.ident, trait_item.kind)
.unwrap_or_else(|| {
bug!("{:?} not found in {:?}", trait_item, impl_data.impl_def_id);
});
let def_id = leaf_def.item.def_id;
let substs = tcx.infer_ctxt().enter(|infcx| {
let param_env = param_env.with_reveal_all();
@ -133,11 +134,52 @@ fn resolve_associated_item<'tcx>(
};
if !eligible {
return None;
return Ok(None);
}
let substs = tcx.erase_regions(&substs);
Some(ty::Instance::new(def_id, substs))
// Check if we just resolved an associated `const` declaration from
// a `trait` to an associated `const` definition in an `impl`, where
// the definition in the `impl` has the wrong type (for which an
// error has already been/will be emitted elsewhere).
//
// NB: this may be expensive, we try to skip it in all the cases where
// we know the error would've been caught (e.g. in an upstream crate).
//
// A better approach might be to just introduce a query (returning
// `Result<(), ErrorReported>`) for the check that `rustc_typeck`
// performs (i.e. that the definition's type in the `impl` matches
// the declaration in the `trait`), so that we can cheaply check
// here if it failed, instead of approximating it.
if trait_item.kind == ty::AssocKind::Const
&& trait_item.def_id != leaf_def.item.def_id
&& leaf_def.item.def_id.is_local()
{
let normalized_type_of = |def_id, substs| {
tcx.subst_and_normalize_erasing_regions(substs, param_env, &tcx.type_of(def_id))
};
let original_ty = normalized_type_of(trait_item.def_id, rcvr_substs);
let resolved_ty = normalized_type_of(leaf_def.item.def_id, substs);
if original_ty != resolved_ty {
let msg = format!(
"Instance::resolve: inconsistent associated `const` type: \
was `{}: {}` but resolved to `{}: {}`",
tcx.def_path_str_with_substs(trait_item.def_id, rcvr_substs),
original_ty,
tcx.def_path_str_with_substs(leaf_def.item.def_id, substs),
resolved_ty,
);
let span = tcx.def_span(leaf_def.item.def_id);
tcx.sess.delay_span_bug(span, &msg);
return Err(ErrorReported);
}
}
Some(ty::Instance::new(leaf_def.item.def_id, substs))
}
traits::VtableGenerator(generator_data) => Some(Instance {
def: ty::InstanceDef::Item(generator_data.generator_def_id),
@ -155,7 +197,7 @@ fn resolve_associated_item<'tcx>(
traits::VtableFnPointer(ref data) => {
// `FnPtrShim` requires a monomorphic aka concrete type.
if data.fn_ty.needs_subst() {
return None;
return Ok(None);
}
Some(Instance {
@ -176,7 +218,7 @@ fn resolve_associated_item<'tcx>(
// `CloneShim` requires a monomorphic aka concrete type.
if self_ty.needs_subst() {
return None;
return Ok(None);
}
Some(Instance {
@ -195,7 +237,7 @@ fn resolve_associated_item<'tcx>(
}
}
traits::VtableAutoImpl(..) | traits::VtableParam(..) | traits::VtableTraitAlias(..) => None,
}
})
}
pub fn provide(providers: &mut ty::query::Providers<'_>) {

View File

@ -0,0 +1,14 @@
trait Nat {
const VALUE: usize;
}
struct Zero;
impl Nat for Zero {
const VALUE: i32 = 0;
//~^ ERROR implemented const `VALUE` has an incompatible type for trait
}
fn main() {
let _: [i32; Zero::VALUE] = [];
}

View File

@ -0,0 +1,12 @@
error[E0326]: implemented const `VALUE` has an incompatible type for trait
--> $DIR/issue-70942-trait-vs-impl-mismatch.rs:8:18
|
LL | const VALUE: usize;
| ----- type in trait
...
LL | const VALUE: i32 = 0;
| ^^^ expected `usize`, found `i32`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0326`.

View File

@ -19,5 +19,4 @@ impl TraitB for B { //~ ERROR not all trait items implemented, missing: `MyA`
fn main() {
let _ = [0; B::VALUE];
//~^ ERROR constant expression depends on a generic parameter
}

View File

@ -13,15 +13,7 @@ LL | type MyA: TraitA;
LL | impl TraitB for B {
| ^^^^^^^^^^^^^^^^^ missing `MyA` in implementation
error: constant expression depends on a generic parameter
--> $DIR/issue-69602-type-err-during-codegen-ice.rs:21:17
|
LL | let _ = [0; B::VALUE];
| ^^^^^^^^
|
= note: this may fail depending on what value the parameter takes
error: aborting due to 3 previous errors
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0046, E0437.
For more information about an error, try `rustc --explain E0046`.