From bc1595473bad5ab808b2734260286ba0f847546a Mon Sep 17 00:00:00 2001 From: csmoe Date: Thu, 16 Apr 2020 23:13:44 +0800 Subject: [PATCH 1/9] record await span inside GeneratorInteriorTypeCause --- src/librustc_middle/ty/context.rs | 8 +++-- .../traits/error_reporting/suggestions.rs | 30 +++++++++++-------- .../check/generator_interior.rs | 1 + 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index a49dc105498ed..e127c222dacd4 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -298,14 +298,14 @@ pub struct ResolvedOpaqueTy<'tcx> { /// /// ```ignore (pseudo-Rust) /// async move { -/// let x: T = ...; +/// let x: T = expr; /// foo.await /// ... /// } /// ``` /// -/// Here, we would store the type `T`, the span of the value `x`, and the "scope-span" for -/// the scope that contains `x`. +/// Here, we would store the type `T`, the span of the value `x`, the "scope-span" for +/// the scope that contains `x`, the expr `T` evaluated from, and the span of `foo.await`. #[derive(RustcEncodable, RustcDecodable, Clone, Debug, Eq, Hash, PartialEq, HashStable)] pub struct GeneratorInteriorTypeCause<'tcx> { /// Type of the captured binding. @@ -314,6 +314,8 @@ pub struct GeneratorInteriorTypeCause<'tcx> { pub span: Span, /// Span of the scope of the captured binding. pub scope_span: Option, + /// Span of `.await` statement. + pub await_span: Span, /// Expr which the type evaluated from. pub expr: Option, } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 254db6cb869b1..c0e14d044ecb8 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -125,6 +125,7 @@ pub trait InferCtxtExt<'tcx> { err: &mut DiagnosticBuilder<'_>, target_span: Span, scope_span: &Option, + await_span: Span, expr: Option, snippet: String, inner_generator_body: Option<&hir::Body<'_>>, @@ -1260,20 +1261,31 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ty_matches(ty) }) .map(|expr| expr.span); - let ty::GeneratorInteriorTypeCause { span, scope_span, expr, .. } = cause; - (span, source_map.span_to_snippet(*span), scope_span, expr, from_awaited_ty) + let ty::GeneratorInteriorTypeCause { span, scope_span, await_span, expr, .. } = + cause; + ( + span, + source_map.span_to_snippet(*span), + scope_span, + await_span, + expr, + from_awaited_ty, + ) }); debug!( "maybe_note_obligation_cause_for_async_await: target_ty={:?} \ - generator_interior_types={:?} target_span={:?}", - target_ty, tables.generator_interior_types, target_span + generator_interior_types={:?} target_span={:?} await_span={:?}", + target_ty, tables.generator_interior_types, target_span, await_span ); - if let Some((target_span, Ok(snippet), scope_span, expr, from_awaited_ty)) = target_span { + if let Some((target_span, Ok(snippet), scope_span, await_span, expr, from_awaited_ty)) = + target_span + { self.note_obligation_cause_for_async_await( err, *target_span, scope_span, + await_span, *expr, snippet, generator_body, @@ -1298,6 +1310,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'_>, target_span: Span, scope_span: &Option, + await_span: Span, expr: Option, snippet: String, inner_generator_body: Option<&hir::Body<'_>>, @@ -1373,12 +1386,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { if let Some(await_span) = from_awaited_ty { // The type causing this obligation is one being awaited at await_span. let mut span = MultiSpan::from_span(await_span); - - span.push_span_label( - await_span, - format!("await occurs here on type `{}`, which {}", target_ty, trait_explanation), - ); - err.span_note( span, &format!( @@ -1392,7 +1399,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { "note_obligation_cause_for_async_await generator_interior_types: {:#?}", tables.generator_interior_types ); - let await_span = tables.generator_interior_types.iter().map(|t| t.span).last().unwrap(); let mut span = MultiSpan::from_span(await_span); span.push_span_label( await_span, diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index db9c8c35c2cee..99e01a0332f49 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -96,6 +96,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { span: source_span, ty: &ty, scope_span, + await_span: yield_data.span, expr: expr.map(|e| e.hir_id), }) .or_insert(entries); From 0b83a5a8ae81f69dbfd1ed19566ab2d6eb588675 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 10 Apr 2020 05:13:20 +0300 Subject: [PATCH 2/9] ty/instance: use `ParamEnvAnd` in the `resolve_instance` query. --- src/librustc_middle/query/mod.rs | 6 ++++-- src/librustc_middle/ty/instance.rs | 9 ++++++++- src/librustc_middle/ty/query/keys.rs | 11 ----------- src/librustc_ty/instance.rs | 6 ++++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 3ddb290fc8d1e..c974ab3190e82 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -1258,8 +1258,10 @@ rustc_queries! { desc { "looking up enabled feature gates" } } - query resolve_instance(key: (ty::ParamEnv<'tcx>, DefId, SubstsRef<'tcx>)) -> Option> { - desc { "resolving instance `{:?}` `{:?}` with {:?}", key.1, key.2, key.0 } + query resolve_instance( + key: ty::ParamEnvAnd<'tcx, (DefId, SubstsRef<'tcx>)> + ) -> Option> { + desc { "resolving instance `{}`", ty::Instance::new(key.value.0, key.value.1) } } } } diff --git a/src/librustc_middle/ty/instance.rs b/src/librustc_middle/ty/instance.rs index ca76cfb14921e..0366795a20e1f 100644 --- a/src/librustc_middle/ty/instance.rs +++ b/src/librustc_middle/ty/instance.rs @@ -290,7 +290,14 @@ impl<'tcx> Instance<'tcx> { ) -> Option> { // 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(¶m_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(¶m_env.and((def_id, substs)))) } pub fn resolve_for_fn_ptr( diff --git a/src/librustc_middle/ty/query/keys.rs b/src/librustc_middle/ty/query/keys.rs index 438e7ed4331a3..a261e484a85fa 100644 --- a/src/librustc_middle/ty/query/keys.rs +++ b/src/librustc_middle/ty/query/keys.rs @@ -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) - } -} diff --git a/src/librustc_ty/instance.rs b/src/librustc_ty/instance.rs index 955e2e3615909..d50e7f39d4155 100644 --- a/src/librustc_ty/instance.rs +++ b/src/librustc_ty/instance.rs @@ -9,10 +9,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>), + key: ty::ParamEnvAnd<'tcx, (DefId, SubstsRef<'tcx>)>, ) -> Option> { + 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); From 289f46a7f5d02e27b2948eee6879f83427d0199f Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Fri, 10 Apr 2020 05:13:29 +0300 Subject: [PATCH 3/9] Detect mistyped associated consts in `Instance::resolve`. --- Cargo.lock | 1 + src/librustc_codegen_llvm/context.rs | 1 + src/librustc_codegen_ssa/base.rs | 1 + src/librustc_codegen_ssa/mir/block.rs | 1 + src/librustc_middle/mir/interpret/queries.rs | 13 ++-- src/librustc_middle/query/mod.rs | 13 +++- src/librustc_middle/ty/instance.rs | 20 ++++-- src/librustc_mir/interpret/eval_context.rs | 9 ++- src/librustc_mir/monomorphize/collector.rs | 15 +++-- src/librustc_mir/monomorphize/mod.rs | 2 +- .../transform/check_consts/validation.rs | 2 +- src/librustc_mir/transform/inline.rs | 3 +- src/librustc_mir_build/lints.rs | 2 +- .../traits/codegen/mod.rs | 7 +- src/librustc_ty/Cargo.toml | 1 + src/librustc_ty/instance.rs | 66 +++++++++++++++---- .../issue-70942-trait-vs-impl-mismatch.rs | 14 ++++ .../issue-70942-trait-vs-impl-mismatch.stderr | 12 ++++ ...issue-69602-type-err-during-codegen-ice.rs | 1 - ...e-69602-type-err-during-codegen-ice.stderr | 10 +-- 20 files changed, 142 insertions(+), 52 deletions(-) create mode 100644 src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.rs create mode 100644 src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.stderr diff --git a/Cargo.lock b/Cargo.lock index cf86f44305e71..f3d226b30f7cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4310,6 +4310,7 @@ version = "0.0.0" dependencies = [ "log", "rustc_data_structures", + "rustc_errors", "rustc_hir", "rustc_infer", "rustc_middle", diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index daa723495f6b3..d385c07307474 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -376,6 +376,7 @@ impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> { def_id, tcx.intern_substs(&[]), ) + .unwrap() .unwrap(), ), _ => { diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs index 8a9b8f11f76ba..1577dbbd3508f 100644 --- a/src/librustc_codegen_ssa/base.rs +++ b/src/librustc_codegen_ssa/base.rs @@ -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(), ); ( diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 4913138650880..112833845e5a4 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -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, diff --git a/src/librustc_middle/mir/interpret/queries.rs b/src/librustc_middle/mir/interpret/queries.rs index 46bf1d9695796..a7953f0f900fb 100644 --- a/src/librustc_middle/mir/interpret/queries.rs +++ b/src/librustc_middle/mir/interpret/queries.rs @@ -39,12 +39,13 @@ impl<'tcx> TyCtxt<'tcx> { promoted: Option, span: Option, ) -> 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)), } } diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index c974ab3190e82..6e56b36476007 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -679,7 +679,7 @@ rustc_queries! { Codegen { query codegen_fulfill_obligation( key: (ty::ParamEnv<'tcx>, ty::PolyTraitRef<'tcx>) - ) -> Option> { + ) -> Result, ErrorReported> { cache_on_disk_if { true } desc { |tcx| "checking if `{}` fulfills its obligations", @@ -1258,9 +1258,18 @@ rustc_queries! { desc { "looking up enabled feature gates" } } + /// 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>)> - ) -> Option> { + ) -> Result>, ErrorReported> { desc { "resolving instance `{}`", ty::Instance::new(key.value.0, key.value.1) } } } diff --git a/src/librustc_middle/ty/instance.rs b/src/librustc_middle/ty/instance.rs index 0366795a20e1f..d7b59d5fa132c 100644 --- a/src/librustc_middle/ty/instance.rs +++ b/src/librustc_middle/ty/instance.rs @@ -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,26 +269,31 @@ 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: 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> { + ) -> Result>, ErrorReported> { // All regions in the result of this query are erased, so it's // fine to erase all of the input regions. @@ -307,7 +313,7 @@ impl<'tcx> Instance<'tcx> { substs: SubstsRef<'tcx>, ) -> Option> { 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]"); @@ -339,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() } } @@ -360,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( diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index fcbb253579728..18efc736d2a32 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -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( diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 32d28bb0d1362..efb5e24bf96b2 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -674,9 +674,12 @@ fn visit_fn_use<'tcx>( output: &mut Vec>, ) { 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); } } @@ -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)); @@ -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) diff --git a/src/librustc_mir/monomorphize/mod.rs b/src/librustc_mir/monomorphize/mod.rs index bbb8e063a4573..98a3d9584f587 100644 --- a/src/librustc_mir/monomorphize/mod.rs +++ b/src/librustc_mir/monomorphize/mod.rs @@ -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 => { diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 4cc42c0408f69..6754831a6fe15 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -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; diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index bfa13abb871c2..f071eb0a952d0 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -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; diff --git a/src/librustc_mir_build/lints.rs b/src/librustc_mir_build/lints.rs index 948f1ae0b4277..66d56858527ae 100644 --- a/src/librustc_mir_build/lints.rs +++ b/src/librustc_mir_build/lints.rs @@ -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) diff --git a/src/librustc_trait_selection/traits/codegen/mod.rs b/src/librustc_trait_selection/traits/codegen/mod.rs index e75432a0b72e8..b2e46bc6da116 100644 --- a/src/librustc_trait_selection/traits/codegen/mod.rs +++ b/src/librustc_trait_selection/traits/codegen/mod.rs @@ -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> { +) -> Result, 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) }) } diff --git a/src/librustc_ty/Cargo.toml b/src/librustc_ty/Cargo.toml index 37d1ed38d79cc..b6db75e44f971 100644 --- a/src/librustc_ty/Cargo.toml +++ b/src/librustc_ty/Cargo.toml @@ -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" } diff --git a/src/librustc_ty/instance.rs b/src/librustc_ty/instance.rs index d50e7f39d4155..2a99bb1aed954 100644 --- a/src/librustc_ty/instance.rs +++ b/src/librustc_ty/instance.rs @@ -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; @@ -12,7 +13,7 @@ use log::debug; fn resolve_instance<'tcx>( tcx: TyCtxt<'tcx>, key: ty::ParamEnvAnd<'tcx, (DefId, SubstsRef<'tcx>)>, -) -> Option> { +) -> Result>, ErrorReported> { let (param_env, (def_id, substs)) = key.into_parts(); debug!("resolve(def_id={:?}, substs={:?})", def_id, substs); @@ -40,7 +41,7 @@ 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"); @@ -55,7 +56,7 @@ 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 @@ -67,7 +68,7 @@ fn resolve_associated_item<'tcx>( param_env: ty::ParamEnv<'tcx>, trait_id: DefId, rcvr_substs: SubstsRef<'tcx>, -) -> Option> { +) -> Result>, ErrorReported> { let def_id = trait_item.def_id; debug!( "resolve_associated_item(trait_item={:?}, \ @@ -82,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: {:?}, {:?}, {:?}, {:?}", @@ -94,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(); @@ -135,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), @@ -157,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 { @@ -178,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 { @@ -197,7 +237,7 @@ fn resolve_associated_item<'tcx>( } } traits::VtableAutoImpl(..) | traits::VtableParam(..) | traits::VtableTraitAlias(..) => None, - } + }) } pub fn provide(providers: &mut ty::query::Providers<'_>) { diff --git a/src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.rs b/src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.rs new file mode 100644 index 0000000000000..b65f5345034e5 --- /dev/null +++ b/src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.rs @@ -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] = []; +} diff --git a/src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.stderr b/src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.stderr new file mode 100644 index 0000000000000..19d9ff7166784 --- /dev/null +++ b/src/test/ui/consts/issue-70942-trait-vs-impl-mismatch.stderr @@ -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`. diff --git a/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.rs b/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.rs index 6ac3eb53cb319..2c5257ce063cb 100644 --- a/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.rs +++ b/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.rs @@ -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 } diff --git a/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.stderr b/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.stderr index 175e6b0eaa0dd..8ae0f8b804c93 100644 --- a/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.stderr +++ b/src/test/ui/issues/issue-69602-type-err-during-codegen-ice.stderr @@ -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`. From 2add9d8fc2c0fc9f77d46c5fe43ed2c33790e366 Mon Sep 17 00:00:00 2001 From: Val Markovic Date: Sun, 19 Apr 2020 16:40:53 -0700 Subject: [PATCH 4/9] Moving all rustdoc-ui tests to check-pass These were all build-pass before and don't seem to need it. Helps with #62277 --- src/test/rustdoc-ui/cfg-test.rs | 2 +- src/test/rustdoc-ui/coverage/basic.rs | 2 +- src/test/rustdoc-ui/coverage/empty.rs | 2 +- src/test/rustdoc-ui/coverage/enums.rs | 2 +- src/test/rustdoc-ui/coverage/exotic.rs | 2 +- src/test/rustdoc-ui/coverage/json.rs | 2 +- src/test/rustdoc-ui/coverage/private.rs | 2 +- src/test/rustdoc-ui/coverage/statics-consts.rs | 2 +- src/test/rustdoc-ui/coverage/traits.rs | 2 +- src/test/rustdoc-ui/deprecated-attrs.rs | 2 +- src/test/rustdoc-ui/doc-test-doctest-feature.rs | 2 +- src/test/rustdoc-ui/doc-test-rustdoc-feature.rs | 2 +- src/test/rustdoc-ui/intra-links-warning-crlf.rs | 2 +- src/test/rustdoc-ui/intra-links-warning.rs | 2 +- src/test/rustdoc-ui/invalid-syntax.rs | 2 +- src/test/rustdoc-ui/issue-58473-2.rs | 2 +- src/test/rustdoc-ui/issue-58473.rs | 2 +- src/test/rustdoc-ui/test-no_std.rs | 2 +- src/test/rustdoc-ui/unused.rs | 2 +- 19 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/test/rustdoc-ui/cfg-test.rs b/src/test/rustdoc-ui/cfg-test.rs index 587fe21f8fa73..597c86a1f19ca 100644 --- a/src/test/rustdoc-ui/cfg-test.rs +++ b/src/test/rustdoc-ui/cfg-test.rs @@ -1,4 +1,4 @@ -// build-pass (FIXME(62277): could be check-pass?) +// check-pass // compile-flags:--test --test-args --test-threads=1 // normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR" diff --git a/src/test/rustdoc-ui/coverage/basic.rs b/src/test/rustdoc-ui/coverage/basic.rs index d25ac633d7bf2..98507f99e8d4b 100644 --- a/src/test/rustdoc-ui/coverage/basic.rs +++ b/src/test/rustdoc-ui/coverage/basic.rs @@ -1,5 +1,5 @@ // compile-flags:-Z unstable-options --show-coverage -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![feature(extern_types)] diff --git a/src/test/rustdoc-ui/coverage/empty.rs b/src/test/rustdoc-ui/coverage/empty.rs index 27bcf6f39383a..55a87e9d97b31 100644 --- a/src/test/rustdoc-ui/coverage/empty.rs +++ b/src/test/rustdoc-ui/coverage/empty.rs @@ -1,4 +1,4 @@ // compile-flags:-Z unstable-options --show-coverage -// build-pass (FIXME(62277): could be check-pass?) +// check-pass // an empty crate still has one item to document: the crate root diff --git a/src/test/rustdoc-ui/coverage/enums.rs b/src/test/rustdoc-ui/coverage/enums.rs index e4171d7cfb250..a4ae36d6837af 100644 --- a/src/test/rustdoc-ui/coverage/enums.rs +++ b/src/test/rustdoc-ui/coverage/enums.rs @@ -1,5 +1,5 @@ // compile-flags:-Z unstable-options --show-coverage -// build-pass (FIXME(62277): could be check-pass?) +// check-pass //! (remember the crate root is still a module) diff --git a/src/test/rustdoc-ui/coverage/exotic.rs b/src/test/rustdoc-ui/coverage/exotic.rs index 414d6f8405816..281ce571aa03f 100644 --- a/src/test/rustdoc-ui/coverage/exotic.rs +++ b/src/test/rustdoc-ui/coverage/exotic.rs @@ -1,5 +1,5 @@ // compile-flags:-Z unstable-options --show-coverage -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![feature(doc_keyword)] diff --git a/src/test/rustdoc-ui/coverage/json.rs b/src/test/rustdoc-ui/coverage/json.rs index b1220b32e9194..2bd6a312ab583 100644 --- a/src/test/rustdoc-ui/coverage/json.rs +++ b/src/test/rustdoc-ui/coverage/json.rs @@ -1,4 +1,4 @@ -// build-pass +// check-pass // compile-flags:-Z unstable-options --output-format json --show-coverage pub mod foo { diff --git a/src/test/rustdoc-ui/coverage/private.rs b/src/test/rustdoc-ui/coverage/private.rs index 6ff1bfa7275de..2a0271727f26e 100644 --- a/src/test/rustdoc-ui/coverage/private.rs +++ b/src/test/rustdoc-ui/coverage/private.rs @@ -1,5 +1,5 @@ // compile-flags:-Z unstable-options --show-coverage --document-private-items -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![allow(unused)] diff --git a/src/test/rustdoc-ui/coverage/statics-consts.rs b/src/test/rustdoc-ui/coverage/statics-consts.rs index b7d2b1dc10c62..5a35260fa3580 100644 --- a/src/test/rustdoc-ui/coverage/statics-consts.rs +++ b/src/test/rustdoc-ui/coverage/statics-consts.rs @@ -1,5 +1,5 @@ // compile-flags:-Z unstable-options --show-coverage -// build-pass (FIXME(62277): could be check-pass?) +// check-pass //! gotta make sure we can count statics and consts correctly, too diff --git a/src/test/rustdoc-ui/coverage/traits.rs b/src/test/rustdoc-ui/coverage/traits.rs index 97f73b4e1f224..7d5cf049e7fdd 100644 --- a/src/test/rustdoc-ui/coverage/traits.rs +++ b/src/test/rustdoc-ui/coverage/traits.rs @@ -1,5 +1,5 @@ // compile-flags:-Z unstable-options --show-coverage -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![feature(trait_alias)] diff --git a/src/test/rustdoc-ui/deprecated-attrs.rs b/src/test/rustdoc-ui/deprecated-attrs.rs index 21169eeb8c83e..0d5dfa733fab6 100644 --- a/src/test/rustdoc-ui/deprecated-attrs.rs +++ b/src/test/rustdoc-ui/deprecated-attrs.rs @@ -1,4 +1,4 @@ -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![doc(no_default_passes, passes = "collapse-docs unindent-comments")] diff --git a/src/test/rustdoc-ui/doc-test-doctest-feature.rs b/src/test/rustdoc-ui/doc-test-doctest-feature.rs index 984d49b43efd0..9a79fb8838351 100644 --- a/src/test/rustdoc-ui/doc-test-doctest-feature.rs +++ b/src/test/rustdoc-ui/doc-test-doctest-feature.rs @@ -1,4 +1,4 @@ -// build-pass +// check-pass // compile-flags:--test // normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR" diff --git a/src/test/rustdoc-ui/doc-test-rustdoc-feature.rs b/src/test/rustdoc-ui/doc-test-rustdoc-feature.rs index 62fd3da9233fa..2af5782453e6d 100644 --- a/src/test/rustdoc-ui/doc-test-rustdoc-feature.rs +++ b/src/test/rustdoc-ui/doc-test-rustdoc-feature.rs @@ -1,4 +1,4 @@ -// build-pass +// check-pass // compile-flags:--test // normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR" diff --git a/src/test/rustdoc-ui/intra-links-warning-crlf.rs b/src/test/rustdoc-ui/intra-links-warning-crlf.rs index ccd2841ff0be3..18c9837b0bb45 100644 --- a/src/test/rustdoc-ui/intra-links-warning-crlf.rs +++ b/src/test/rustdoc-ui/intra-links-warning-crlf.rs @@ -1,5 +1,5 @@ // ignore-tidy-cr -// build-pass +// check-pass // This file checks the spans of intra-link warnings in a file with CRLF line endings. The // .gitattributes file in this directory should enforce it. diff --git a/src/test/rustdoc-ui/intra-links-warning.rs b/src/test/rustdoc-ui/intra-links-warning.rs index b0c637521f9ae..623dcc320bb8d 100644 --- a/src/test/rustdoc-ui/intra-links-warning.rs +++ b/src/test/rustdoc-ui/intra-links-warning.rs @@ -1,4 +1,4 @@ -// build-pass +// check-pass //! Test with [Foo::baz], [Bar::foo], ... //~^ WARNING `[Foo::baz]` cannot be resolved diff --git a/src/test/rustdoc-ui/invalid-syntax.rs b/src/test/rustdoc-ui/invalid-syntax.rs index 72037dd74be35..c395a8ef3d41a 100644 --- a/src/test/rustdoc-ui/invalid-syntax.rs +++ b/src/test/rustdoc-ui/invalid-syntax.rs @@ -1,4 +1,4 @@ -// build-pass +// check-pass /// ``` /// \__________pkt->size___________/ \_result->size_/ \__pkt->size__/ diff --git a/src/test/rustdoc-ui/issue-58473-2.rs b/src/test/rustdoc-ui/issue-58473-2.rs index 1bb19353ba2f7..e5f3b4daf5729 100644 --- a/src/test/rustdoc-ui/issue-58473-2.rs +++ b/src/test/rustdoc-ui/issue-58473-2.rs @@ -1,4 +1,4 @@ -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![deny(private_doc_tests)] diff --git a/src/test/rustdoc-ui/issue-58473.rs b/src/test/rustdoc-ui/issue-58473.rs index 6756d3b5a6051..44e1f58d0a0fb 100644 --- a/src/test/rustdoc-ui/issue-58473.rs +++ b/src/test/rustdoc-ui/issue-58473.rs @@ -1,4 +1,4 @@ -// build-pass (FIXME(62277): could be check-pass?) +// check-pass pub trait Foo { /** diff --git a/src/test/rustdoc-ui/test-no_std.rs b/src/test/rustdoc-ui/test-no_std.rs index 166a87382cb65..af4843ad32405 100644 --- a/src/test/rustdoc-ui/test-no_std.rs +++ b/src/test/rustdoc-ui/test-no_std.rs @@ -1,6 +1,6 @@ // compile-flags:--test // normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR" -// build-pass +// check-pass #![no_std] diff --git a/src/test/rustdoc-ui/unused.rs b/src/test/rustdoc-ui/unused.rs index ffa421d4f7f2d..702b24c36c56c 100644 --- a/src/test/rustdoc-ui/unused.rs +++ b/src/test/rustdoc-ui/unused.rs @@ -1,4 +1,4 @@ -// build-pass (FIXME(62277): could be check-pass?) +// check-pass // This test purpose is to check that unused_imports lint isn't fired // by rustdoc. Why would it? Because when rustdoc is running, it uses From 00d12ef9016f2eab194488ac7d51567175eddebc Mon Sep 17 00:00:00 2001 From: csmoe Date: Thu, 16 Apr 2020 23:14:11 +0800 Subject: [PATCH 5/9] add test for correct await span --- src/librustc_middle/ty/context.rs | 4 ++-- .../traits/error_reporting/suggestions.rs | 24 ++++++++++++------- .../check/generator_interior.rs | 2 +- src/test/ui/async-await/issue-71137.rs | 21 ++++++++++++++++ src/test/ui/async-await/issue-71137.stderr | 23 ++++++++++++++++++ 5 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/async-await/issue-71137.rs create mode 100644 src/test/ui/async-await/issue-71137.stderr diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index e127c222dacd4..c6228a913cfe2 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -314,8 +314,8 @@ pub struct GeneratorInteriorTypeCause<'tcx> { pub span: Span, /// Span of the scope of the captured binding. pub scope_span: Option, - /// Span of `.await` statement. - pub await_span: Span, + /// Span of `.await` or `yield` expression. + pub yield_span: Span, /// Expr which the type evaluated from. pub expr: Option, } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index c0e14d044ecb8..51ac552c31239 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -1261,13 +1261,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { ty_matches(ty) }) .map(|expr| expr.span); - let ty::GeneratorInteriorTypeCause { span, scope_span, await_span, expr, .. } = + let ty::GeneratorInteriorTypeCause { span, scope_span, yield_span, expr, .. } = cause; ( span, source_map.span_to_snippet(*span), scope_span, - await_span, + yield_span, expr, from_awaited_ty, ) @@ -1275,17 +1275,17 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { debug!( "maybe_note_obligation_cause_for_async_await: target_ty={:?} \ - generator_interior_types={:?} target_span={:?} await_span={:?}", - target_ty, tables.generator_interior_types, target_span, await_span + generator_interior_types={:?} target_span={:?}", + target_ty, tables.generator_interior_types, target_span ); - if let Some((target_span, Ok(snippet), scope_span, await_span, expr, from_awaited_ty)) = + if let Some((target_span, Ok(snippet), scope_span, yield_span, expr, from_awaited_ty)) = target_span { self.note_obligation_cause_for_async_await( err, *target_span, scope_span, - await_span, + *yield_span, *expr, snippet, generator_body, @@ -1310,7 +1310,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'_>, target_span: Span, scope_span: &Option, - await_span: Span, + yield_span: Span, expr: Option, snippet: String, inner_generator_body: Option<&hir::Body<'_>>, @@ -1386,6 +1386,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { if let Some(await_span) = from_awaited_ty { // The type causing this obligation is one being awaited at await_span. let mut span = MultiSpan::from_span(await_span); + + span.push_span_label( + await_span, + format!("await occurs here on type `{}`, which {}", target_ty, trait_explanation), + ); + err.span_note( span, &format!( @@ -1399,9 +1405,9 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { "note_obligation_cause_for_async_await generator_interior_types: {:#?}", tables.generator_interior_types ); - let mut span = MultiSpan::from_span(await_span); + let mut span = MultiSpan::from_span(yield_span); span.push_span_label( - await_span, + yield_span, format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet), ); diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index 99e01a0332f49..716d23b4d173c 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -96,7 +96,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> { span: source_span, ty: &ty, scope_span, - await_span: yield_data.span, + yield_span: yield_data.span, expr: expr.map(|e| e.hir_id), }) .or_insert(entries); diff --git a/src/test/ui/async-await/issue-71137.rs b/src/test/ui/async-await/issue-71137.rs new file mode 100644 index 0000000000000..ebb392a45308e --- /dev/null +++ b/src/test/ui/async-await/issue-71137.rs @@ -0,0 +1,21 @@ +// edition:2018 + +use std::future::Future; +use std::sync::Mutex; + +fn fake_spawn(f: F) { } + +async fn wrong_mutex() { + let m = Mutex::new(1); + { + let mut guard = m.lock().unwrap(); + (async { "right"; }).await; + *guard += 1; + } + + (async { "wrong"; }).await; +} + +fn main() { + fake_spawn(wrong_mutex()); //~ Error future cannot be sent between threads safely +} diff --git a/src/test/ui/async-await/issue-71137.stderr b/src/test/ui/async-await/issue-71137.stderr new file mode 100644 index 0000000000000..788a9bc2c7e47 --- /dev/null +++ b/src/test/ui/async-await/issue-71137.stderr @@ -0,0 +1,23 @@ +error: future cannot be sent between threads safely + --> $DIR/issue-71137.rs:20:3 + | +LL | fn fake_spawn(f: F) { } + | ---- required by this bound in `fake_spawn` +... +LL | fake_spawn(wrong_mutex()); + | ^^^^^^^^^^ future returned by `wrong_mutex` is not `Send` + | + = help: within `impl std::future::Future`, the trait `std::marker::Send` is not implemented for `std::sync::MutexGuard<'_, i32>` +note: future is not `Send` as this value is used across an await + --> $DIR/issue-71137.rs:12:5 + | +LL | let mut guard = m.lock().unwrap(); + | --------- has type `std::sync::MutexGuard<'_, i32>` which is not `Send` +LL | (async { "right"; }).await; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `mut guard` maybe used later +LL | *guard += 1; +LL | } + | - `mut guard` is later dropped here + +error: aborting due to previous error + From 3a8097f2b44f8221bfbdbf47b66558eca44953fd Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Tue, 21 Apr 2020 21:09:24 -0700 Subject: [PATCH 6/9] Clarify unused_doc_comments note on macro invocations --- src/librustc_lint/context.rs | 2 +- src/test/ui/useless-comment.stderr | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_lint/context.rs b/src/librustc_lint/context.rs index 1747a78d36a4c..e5d3227d5afd4 100644 --- a/src/librustc_lint/context.rs +++ b/src/librustc_lint/context.rs @@ -566,7 +566,7 @@ pub trait LintContext: Sized { stability::deprecation_suggestion(&mut db, suggestion, span) } BuiltinLintDiagnostics::UnusedDocComment(span) => { - db.span_label(span, "rustdoc does not generate documentation for macros"); + db.span_label(span, "rustdoc does not generate documentation for macro invocations"); db.help("to document an item produced by a macro, \ the macro must produce the documentation as part of its expansion"); } diff --git a/src/test/ui/useless-comment.stderr b/src/test/ui/useless-comment.stderr index 92817321a88f0..5a0af8db7c504 100644 --- a/src/test/ui/useless-comment.stderr +++ b/src/test/ui/useless-comment.stderr @@ -2,7 +2,7 @@ error: unused doc comment --> $DIR/useless-comment.rs:9:1 | LL | /// foo - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rustdoc does not generate documentation for macros + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rustdoc does not generate documentation for macro invocations | note: the lint level is defined here --> $DIR/useless-comment.rs:3:9 @@ -15,7 +15,7 @@ error: unused doc comment --> $DIR/useless-comment.rs:32:5 | LL | /// bar - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rustdoc does not generate documentation for macros + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rustdoc does not generate documentation for macro invocations | = help: to document an item produced by a macro, the macro must produce the documentation as part of its expansion From 23b9f46fff4453fb90611c2fb554e004f1cec096 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 22 Apr 2020 07:47:31 +0200 Subject: [PATCH 7/9] More diagnostic items for Clippy usage This adds a couple of more diagnostic items to be used in Clippy. I chose these particular ones because they were the types which we seem to check for the most in Clippy. I'm not sure if the `cfg_attr(not(test))` is needed, but it was also used for `Vec` and a few other types. --- src/liballoc/collections/vec_deque.rs | 1 + src/liballoc/string.rs | 1 + src/libstd/collections/hash/map.rs | 1 + src/libstd/collections/hash/set.rs | 1 + src/libstd/sync/mutex.rs | 1 + 5 files changed, 5 insertions(+) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 091b068b0b245..a3b61f1f4a589 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -50,6 +50,7 @@ const MAXIMUM_ZST_CAPACITY: usize = 1 << (64 - 1); // Largest possible power of /// [`pop_front`]: #method.pop_front /// [`extend`]: #method.extend /// [`append`]: #method.append +#[cfg_attr(not(test), rustc_diagnostic_item = "vecdeque_type")] #[stable(feature = "rust1", since = "1.0.0")] pub struct VecDeque { // tail and head are pointers into the buffer. Tail always points diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs index 80fa813991567..f3fe1adebb141 100644 --- a/src/liballoc/string.rs +++ b/src/liballoc/string.rs @@ -278,6 +278,7 @@ use crate::vec::Vec; /// [`Deref`]: ../../std/ops/trait.Deref.html /// [`as_str()`]: struct.String.html#method.as_str #[derive(PartialOrd, Eq, Ord)] +#[cfg_attr(not(test), rustc_diagnostic_item = "string_type")] #[stable(feature = "rust1", since = "1.0.0")] pub struct String { vec: Vec, diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs index 706b388f78323..e6da7426eb4af 100644 --- a/src/libstd/collections/hash/map.rs +++ b/src/libstd/collections/hash/map.rs @@ -198,6 +198,7 @@ use crate::sys; /// ``` #[derive(Clone)] +#[cfg_attr(not(test), rustc_diagnostic_item = "hashmap_type")] #[stable(feature = "rust1", since = "1.0.0")] pub struct HashMap { base: base::HashMap, diff --git a/src/libstd/collections/hash/set.rs b/src/libstd/collections/hash/set.rs index 1ad99f03703dd..c1a57f2ce6129 100644 --- a/src/libstd/collections/hash/set.rs +++ b/src/libstd/collections/hash/set.rs @@ -105,6 +105,7 @@ use super::map::{self, HashMap, Keys, RandomState}; /// [`PartialEq`]: ../../std/cmp/trait.PartialEq.html /// [`RefCell`]: ../../std/cell/struct.RefCell.html #[derive(Clone)] +#[cfg_attr(not(test), rustc_diagnostic_item = "hashset_type")] #[stable(feature = "rust1", since = "1.0.0")] pub struct HashSet { map: HashMap, diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index 0cb16b19d7326..797b22fdd1279 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -108,6 +108,7 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// *guard += 1; /// ``` #[stable(feature = "rust1", since = "1.0.0")] +#[cfg_attr(not(test), rustc_diagnostic_item = "mutex_type")] pub struct Mutex { // Note that this mutex is in a *box*, not inlined into the struct itself. // Once a native mutex has been used once, its address can never change (it From 3390ff97b22e082bb553cc0f175ae5ca18bd5e60 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 22 Apr 2020 11:08:50 +0200 Subject: [PATCH 8/9] Add error code to inner doc comment attribute error --- src/librustc_error_codes/error_codes.rs | 1 + src/librustc_error_codes/error_codes/E0753.md | 31 +++++++++++++++++++ src/librustc_parse/parser/attr.rs | 12 +++++-- 3 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 src/librustc_error_codes/error_codes/E0753.md diff --git a/src/librustc_error_codes/error_codes.rs b/src/librustc_error_codes/error_codes.rs index bc04809eaa1df..9f4b5fd85fd4d 100644 --- a/src/librustc_error_codes/error_codes.rs +++ b/src/librustc_error_codes/error_codes.rs @@ -432,6 +432,7 @@ E0749: include_str!("./error_codes/E0749.md"), E0750: include_str!("./error_codes/E0750.md"), E0751: include_str!("./error_codes/E0751.md"), E0752: include_str!("./error_codes/E0752.md"), +E0753: include_str!("./error_codes/E0753.md"), ; // E0006, // merged with E0005 // E0008, // cannot bind by-move into a pattern guard diff --git a/src/librustc_error_codes/error_codes/E0753.md b/src/librustc_error_codes/error_codes/E0753.md new file mode 100644 index 0000000000000..a69da964aee39 --- /dev/null +++ b/src/librustc_error_codes/error_codes/E0753.md @@ -0,0 +1,31 @@ +An inner doc comment was used in an invalid context. + +Erroneous code example: + +```compile_fail,E0753 +fn foo() {} +//! foo +// ^ error! +fn main() {} +``` + +Inner document can only be used before items. For example: + +``` +//! A working comment applied to the module! +fn foo() { + //! Another working comment! +} +fn main() {} +``` + +In case you want to document the item following the doc comment, you might want +to use outer doc comment: + +``` +/// I am an outer doc comment +#[doc = "I am also an outer doc comment!"] +fn foo() { + // ... +} +``` diff --git a/src/librustc_parse/parser/attr.rs b/src/librustc_parse/parser/attr.rs index b56dd30739dae..803f14a2a228a 100644 --- a/src/librustc_parse/parser/attr.rs +++ b/src/librustc_parse/parser/attr.rs @@ -4,7 +4,7 @@ use rustc_ast::attr; use rustc_ast::token::{self, Nonterminal}; use rustc_ast::util::comments; use rustc_ast_pretty::pprust; -use rustc_errors::PResult; +use rustc_errors::{error_code, PResult}; use rustc_span::{Span, Symbol}; use log::debug; @@ -50,10 +50,16 @@ impl<'a> Parser<'a> { } else if let token::DocComment(s) = self.token.kind { let attr = self.mk_doc_comment(s); if attr.style != ast::AttrStyle::Outer { - self.struct_span_err(self.token.span, "expected outer doc comment") + self.sess + .span_diagnostic + .struct_span_err_with_code( + self.token.span, + "expected outer doc comment", + error_code!(E0753), + ) .note( "inner doc comments like this (starting with \ - `//!` or `/*!`) can only appear before items", + `//!` or `/*!`) can only appear before items", ) .emit(); } From 038f5b74336f310f495af0c15e4a2b4d0750cfee Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 22 Apr 2020 11:09:57 +0200 Subject: [PATCH 9/9] Update UI tests --- src/test/ui/parser/doc-comment-in-if-statement.stderr | 3 ++- src/test/ui/parser/issue-30318.stderr | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/ui/parser/doc-comment-in-if-statement.stderr b/src/test/ui/parser/doc-comment-in-if-statement.stderr index af21b78733f90..be52a0afd46b7 100644 --- a/src/test/ui/parser/doc-comment-in-if-statement.stderr +++ b/src/test/ui/parser/doc-comment-in-if-statement.stderr @@ -1,4 +1,4 @@ -error: expected outer doc comment +error[E0753]: expected outer doc comment --> $DIR/doc-comment-in-if-statement.rs:2:13 | LL | if true /*!*/ {} @@ -17,3 +17,4 @@ LL | if true /*!*/ {} error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0753`. diff --git a/src/test/ui/parser/issue-30318.stderr b/src/test/ui/parser/issue-30318.stderr index 489451bb5dc8d..b3a27f1985171 100644 --- a/src/test/ui/parser/issue-30318.stderr +++ b/src/test/ui/parser/issue-30318.stderr @@ -1,4 +1,4 @@ -error: expected outer doc comment +error[E0753]: expected outer doc comment --> $DIR/issue-30318.rs:3:1 | LL | //! Misplaced comment... @@ -8,3 +8,4 @@ LL | //! Misplaced comment... error: aborting due to previous error +For more information about this error, try `rustc --explain E0753`.