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

Detect mistyped associated consts in Instance::resolve. #70970

Merged
merged 2 commits into from
Apr 22, 2020
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4310,6 +4310,7 @@ version = "0.0.0"
dependencies = [
"log",
"rustc_data_structures",
"rustc_errors",
"rustc_hir",
"rustc_infer",
"rustc_middle",
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_llvm/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> {
def_id,
tcx.intern_substs(&[]),
)
.unwrap()
.unwrap(),
),
_ => {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_ssa/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,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(),
);
(
Expand Down
1 change: 1 addition & 0 deletions src/librustc_codegen_ssa/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 7 additions & 6 deletions src/librustc_middle/mir/interpret/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
}
}

Expand Down
17 changes: 14 additions & 3 deletions src/librustc_middle/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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) }
}
}
}
29 changes: 21 additions & 8 deletions src/librustc_middle/ty/instance.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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]");
Expand Down Expand Up @@ -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()
}
}

Expand All @@ -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(
Expand Down
11 changes: 0 additions & 11 deletions src/librustc_middle/ty/query/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
9 changes: 7 additions & 2 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,13 @@ impl<'mir, 'tcx, 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(
Expand Down
15 changes: 10 additions & 5 deletions src/librustc_mir/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -1057,6 +1060,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));
Expand Down Expand Up @@ -1112,8 +1116,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)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/monomorphize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,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;
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,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;
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir_build/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions src/librustc_trait_selection/traits/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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);

Expand Down Expand Up @@ -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)
Expand All @@ -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)
})
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_ty/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
Loading