From b97487bad8608afe05f34f07016aa6276c1a291d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 30 May 2020 15:36:42 +0200 Subject: [PATCH 01/24] Add check for doc alias attribute format --- src/librustdoc/clean/types.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 381238165274d..4f99476ebfa55 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -481,6 +481,33 @@ impl Attributes { }) } + /// Enforce the format of attributes inside `#[doc(...)]`. + pub fn check_doc_attributes( + diagnostic: &::rustc_errors::Handler, + mi: &ast::MetaItem, + ) -> Option<(String, String)> { + mi.meta_item_list().and_then(|list| { + for meta in list { + if meta.check_name(sym::alias) { + if !meta.is_value_str() + || meta + .value_str() + .map(|s| s.to_string()) + .unwrap_or_else(String::new) + .is_empty() + { + diagnostic.span_err( + meta.span(), + "doc alias attribute expects a string: #[doc(alias = \"0\")]", + ); + } + } + } + + None + }) + } + pub fn has_doc_flag(&self, flag: Symbol) -> bool { for attr in &self.other_attrs { if !attr.check_name(sym::doc) { @@ -524,6 +551,7 @@ impl Attributes { } else { if attr.check_name(sym::doc) { if let Some(mi) = attr.meta() { + Attributes::check_doc_attributes(&diagnostic, &mi); if let Some(cfg_mi) = Attributes::extract_cfg(&mi) { // Extracted #[doc(cfg(...))] match Cfg::parse(cfg_mi) { From 2d6267a7a8ae0399f2d363b6ac667fee0b53a1a0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 30 May 2020 15:36:57 +0200 Subject: [PATCH 02/24] Add test for doc alias attribute validation --- src/test/rustdoc-ui/check-doc-alias-attr.rs | 9 +++++++++ .../rustdoc-ui/check-doc-alias-attr.stderr | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 src/test/rustdoc-ui/check-doc-alias-attr.rs create mode 100644 src/test/rustdoc-ui/check-doc-alias-attr.stderr diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.rs b/src/test/rustdoc-ui/check-doc-alias-attr.rs new file mode 100644 index 0000000000000..2f01099107d9e --- /dev/null +++ b/src/test/rustdoc-ui/check-doc-alias-attr.rs @@ -0,0 +1,9 @@ +#![feature(doc_alias)] + +#[doc(alias = "foo")] // ok! +pub struct Bar; + +#[doc(alias)] //~ ERROR +#[doc(alias = 0)] //~ ERROR +#[doc(alias("bar"))] //~ ERROR +pub struct Foo; diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.stderr b/src/test/rustdoc-ui/check-doc-alias-attr.stderr new file mode 100644 index 0000000000000..480acc821aaa8 --- /dev/null +++ b/src/test/rustdoc-ui/check-doc-alias-attr.stderr @@ -0,0 +1,20 @@ +error: doc alias attribute expects a string: #[doc(alias = "0")] + --> $DIR/check-doc-alias-attr.rs:6:7 + | +LL | #[doc(alias)] + | ^^^^^ + +error: doc alias attribute expects a string: #[doc(alias = "0")] + --> $DIR/check-doc-alias-attr.rs:7:7 + | +LL | #[doc(alias = 0)] + | ^^^^^^^^^ + +error: doc alias attribute expects a string: #[doc(alias = "0")] + --> $DIR/check-doc-alias-attr.rs:8:7 + | +LL | #[doc(alias("bar"))] + | ^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + From 0c5c644c91edf6ed949cfa5ffc524f43369df604 Mon Sep 17 00:00:00 2001 From: TrolledWoods Date: Mon, 1 Jun 2020 08:18:34 +0200 Subject: [PATCH 03/24] Mention that BTreeMap::new() doesn't allocate I think it would be nice to mention this, so you don't have to dig through the src to look at the definition of new(). --- src/liballoc/collections/btree/map.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index fa1c09d9ece87..688d660a02afd 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -490,6 +490,8 @@ struct MergeIter> { impl BTreeMap { /// Makes a new empty BTreeMap with a reasonable choice for B. /// + /// Does not allocate anything on its own. + /// /// # Examples /// /// Basic usage: From 8c7c84b4e8923779ff56a518e4cd39c1c600c7d0 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Thu, 18 Jun 2020 13:29:43 -0700 Subject: [PATCH 04/24] code coverage foundation for hash and num_counters Replaced dummy values for hash and num_counters with computed values, and refactored InstrumentCoverage pass to simplify injecting more counters per function in upcoming versions. Improved usage documentation and error messaging. --- src/librustc_codegen_llvm/intrinsic.rs | 37 ++-- src/librustc_codegen_ssa/mir/block.rs | 2 +- src/librustc_codegen_ssa/mir/mod.rs | 4 +- src/librustc_codegen_ssa/traits/intrinsic.rs | 6 +- src/librustc_metadata/locator.rs | 2 + src/librustc_middle/ich/hcx.rs | 33 ++- src/librustc_middle/mir/mod.rs | 23 +- src/librustc_middle/ty/context.rs | 7 + .../transform/instrument_coverage.rs | 196 +++++++++++++----- src/librustc_session/options.rs | 5 +- src/librustc_span/symbol.rs | 1 + 11 files changed, 230 insertions(+), 86 deletions(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 95465939070a0..8c0ccde6b16bb 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -16,6 +16,7 @@ use rustc_codegen_ssa::common::TypeKind; use rustc_codegen_ssa::glue; use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue}; use rustc_codegen_ssa::mir::place::PlaceRef; +use rustc_codegen_ssa::mir::FunctionCx; use rustc_codegen_ssa::traits::*; use rustc_codegen_ssa::MemFlags; use rustc_hir as hir; @@ -23,7 +24,6 @@ use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt}; use rustc_middle::ty::{self, Ty}; use rustc_middle::{bug, span_bug}; use rustc_span::Span; -use rustc_span::Symbol; use rustc_target::abi::{self, HasDataLayout, LayoutOf, Primitive}; use rustc_target::spec::PanicStrategy; @@ -82,14 +82,14 @@ fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: &str) -> Option<&'ll Valu } impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { - fn codegen_intrinsic_call( + fn codegen_intrinsic_call<'b, Bx: BuilderMethods<'b, 'tcx>>( &mut self, + fx: &FunctionCx<'b, 'tcx, Bx>, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, args: &[OperandRef<'tcx, &'ll Value>], llresult: &'ll Value, span: Span, - caller_instance: ty::Instance<'tcx>, ) { let tcx = self.tcx; let callee_ty = instance.monomorphic_ty(tcx); @@ -141,26 +141,17 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { - if let ty::InstanceDef::Item(fn_def_id) = caller_instance.def { - let caller_fn_path = tcx.def_path_str(fn_def_id); - debug!( - "count_code_region to llvm.instrprof.increment(fn_name={})", - caller_fn_path - ); - - // FIXME(richkadel): (1) Replace raw function name with mangled function name; - // (2) Replace hardcoded `1234` in `hash` with a computed hash (as discussed in) - // the MCP (compiler-team/issues/278); and replace the hardcoded `1` for - // `num_counters` with the actual number of counters per function (when the - // changes are made to inject more than one counter per function). - let (fn_name, _len_val) = self.const_str(Symbol::intern(&caller_fn_path)); - let index = args[0].immediate(); - let hash = self.const_u64(1234); - let num_counters = self.const_u32(1); - self.instrprof_increment(fn_name, hash, num_counters, index) - } else { - bug!("intrinsic count_code_region: no src.instance"); - } + let coverage_data = fx.mir.coverage_data.as_ref().unwrap(); + let mangled_fn = tcx.symbol_name(fx.instance); + let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); + let hash = self.const_u64(coverage_data.hash); + let index = args[0].immediate(); + let num_counters = self.const_u32(coverage_data.num_counters as u32); + debug!( + "count_code_region to LLVM intrinsic instrprof.increment(fn_name={}, hash={:?}, num_counters={:?}, index={:?})", + mangled_fn.name, hash, index, num_counters + ); + self.instrprof_increment(mangled_fn_name, hash, num_counters, index) } "va_start" => self.va_start(args[0].immediate()), "va_end" => self.va_end(args[0].immediate()), diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index d56c816811b3c..945c3d4843471 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -688,12 +688,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { .collect(); bx.codegen_intrinsic_call( + self, *instance.as_ref().unwrap(), &fn_abi, &args, dest, terminator.source_info.span, - self.instance, ); if let ReturnDest::IndirectOperand(dst, _) = ret_dest { diff --git a/src/librustc_codegen_ssa/mir/mod.rs b/src/librustc_codegen_ssa/mir/mod.rs index 00b4bf96afa59..fd2e779f681be 100644 --- a/src/librustc_codegen_ssa/mir/mod.rs +++ b/src/librustc_codegen_ssa/mir/mod.rs @@ -21,9 +21,9 @@ use self::operand::{OperandRef, OperandValue}; /// Master context for codegenning from MIR. pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { - instance: Instance<'tcx>, + pub instance: Instance<'tcx>, - mir: &'tcx mir::Body<'tcx>, + pub mir: &'tcx mir::Body<'tcx>, debug_context: Option>, diff --git a/src/librustc_codegen_ssa/traits/intrinsic.rs b/src/librustc_codegen_ssa/traits/intrinsic.rs index f62019498511c..0036eadf4db81 100644 --- a/src/librustc_codegen_ssa/traits/intrinsic.rs +++ b/src/librustc_codegen_ssa/traits/intrinsic.rs @@ -1,5 +1,7 @@ use super::BackendTypes; use crate::mir::operand::OperandRef; +use crate::mir::FunctionCx; +use crate::traits::BuilderMethods; use rustc_middle::ty::{self, Ty}; use rustc_span::Span; use rustc_target::abi::call::FnAbi; @@ -8,14 +10,14 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes { /// Remember to add all intrinsics here, in librustc_typeck/check/mod.rs, /// and in libcore/intrinsics.rs; if you need access to any llvm intrinsics, /// add them to librustc_codegen_llvm/context.rs - fn codegen_intrinsic_call( + fn codegen_intrinsic_call<'a, Bx: BuilderMethods<'a, 'tcx>>( &mut self, + fx: &FunctionCx<'a, 'tcx, Bx>, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, args: &[OperandRef<'tcx, Self::Value>], llresult: Self::Value, span: Span, - caller_instance: ty::Instance<'tcx>, ); fn abort(&mut self); diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index 5a4862d452183..662794f0aa11f 100644 --- a/src/librustc_metadata/locator.rs +++ b/src/librustc_metadata/locator.rs @@ -488,6 +488,8 @@ impl<'a> CrateLocator<'a> { && self.triple != TargetTriple::from_triple(config::host_triple()) { err.note(&format!("the `{}` target may not be installed", self.triple)); + } else if self.crate_name == sym::profiler_builtins { + err.note(&"the compiler may have been built without `profiler = true`"); } err.span_label(self.span, "can't find crate"); err diff --git a/src/librustc_middle/ich/hcx.rs b/src/librustc_middle/ich/hcx.rs index 69b4adb3a0e1d..f5b0b73c49de1 100644 --- a/src/librustc_middle/ich/hcx.rs +++ b/src/librustc_middle/ich/hcx.rs @@ -67,13 +67,15 @@ impl<'a> StableHashingContext<'a> { /// Don't use it for anything else or you'll run the risk of /// leaking data out of the tracking system. #[inline] - pub fn new( + fn new_with_or_without_spans( sess: &'a Session, krate: &'a hir::Crate<'a>, definitions: &'a Definitions, cstore: &'a dyn CrateStore, + always_ignore_spans: bool, ) -> Self { - let hash_spans_initial = !sess.opts.debugging_opts.incremental_ignore_spans; + let hash_spans_initial = + !always_ignore_spans && !sess.opts.debugging_opts.incremental_ignore_spans; StableHashingContext { sess, @@ -88,6 +90,33 @@ impl<'a> StableHashingContext<'a> { } } + #[inline] + pub fn new( + sess: &'a Session, + krate: &'a hir::Crate<'a>, + definitions: &'a Definitions, + cstore: &'a dyn CrateStore, + ) -> Self { + Self::new_with_or_without_spans( + sess, + krate, + definitions, + cstore, + /*always_ignore_spans=*/ false, + ) + } + + #[inline] + pub fn ignore_spans( + sess: &'a Session, + krate: &'a hir::Crate<'a>, + definitions: &'a Definitions, + cstore: &'a dyn CrateStore, + ) -> Self { + let always_ignore_spans = true; + Self::new_with_or_without_spans(sess, krate, definitions, cstore, always_ignore_spans) + } + #[inline] pub fn sess(&self) -> &'a Session { self.sess diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index 3381b95c2a38e..a89a5ef3f8218 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -88,6 +88,19 @@ impl MirPhase { } } +/// Coverage data computed by the `InstrumentCoverage` MIR pass, when compiling with +/// `-Zinstrument_coverage`. +#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable, TypeFoldable)] +pub struct CoverageData { + /// A hash value that can be used by the consumer of the coverage profile data to detect + /// changes to the instrumented source of the associated MIR body (typically, for an + /// individual function). + pub hash: u64, + + /// The total number of coverage region counters added to this MIR Body. + pub num_counters: usize, +} + /// The lowered representation of a single function. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable, TypeFoldable)] pub struct Body<'tcx> { @@ -164,13 +177,17 @@ pub struct Body<'tcx> { /// The user may be writing e.g. `&[(SOME_CELL, 42)][i].1` and this would get promoted, because /// we'd statically know that no thing with interior mutability will ever be available to the /// user without some serious unsafe code. Now this means that our promoted is actually - /// `&[(SOME_CELL, 42)]` and the MIR using it will do the `&promoted[i].1` projection because the - /// index may be a runtime value. Such a promoted value is illegal because it has reachable + /// `&[(SOME_CELL, 42)]` and the MIR using it will do the `&promoted[i].1` projection because + /// the index may be a runtime value. Such a promoted value is illegal because it has reachable /// interior mutability. This flag just makes this situation very obvious where the previous /// implementation without the flag hid this situation silently. /// FIXME(oli-obk): rewrite the promoted during promotion to eliminate the cell components. pub ignore_interior_mut_in_const_validation: bool, + /// If compiling with `-Zinstrument_coverage`, the `InstrumentCoverage` pass stores summary + /// information associated with the MIR, used in code generation of the coverage counters. + pub coverage_data: Option, + predecessor_cache: PredecessorCache, } @@ -211,6 +228,7 @@ impl<'tcx> Body<'tcx> { required_consts: Vec::new(), ignore_interior_mut_in_const_validation: false, control_flow_destroyed, + coverage_data: None, predecessor_cache: PredecessorCache::new(), } } @@ -238,6 +256,7 @@ impl<'tcx> Body<'tcx> { generator_kind: None, var_debug_info: Vec::new(), ignore_interior_mut_in_const_validation: false, + coverage_data: None, predecessor_cache: PredecessorCache::new(), } } diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index 62d6de2d71e6d..0696cae4810ec 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -1284,6 +1284,13 @@ impl<'tcx> TyCtxt<'tcx> { StableHashingContext::new(self.sess, krate, self.definitions, &*self.cstore) } + #[inline(always)] + pub fn create_no_span_stable_hashing_context(self) -> StableHashingContext<'tcx> { + let krate = self.gcx.untracked_crate; + + StableHashingContext::ignore_spans(self.sess, krate, self.definitions, &*self.cstore) + } + // This method makes sure that we have a DepNode and a Fingerprint for // every upstream crate. It needs to be called once right after the tcx is // created. diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index c36614938e10f..793ccbb081bed 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -1,8 +1,15 @@ use crate::transform::{MirPass, MirSource}; use crate::util::patch::MirPatch; +use rustc_data_structures::fingerprint::Fingerprint; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::lang_items; +use rustc_hir::*; +use rustc_middle::ich::StableHashingContext; use rustc_middle::mir::interpret::Scalar; -use rustc_middle::mir::*; +use rustc_middle::mir::{ + self, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, StatementKind, + Terminator, TerminatorKind, START_BLOCK, +}; use rustc_middle::ty; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; @@ -12,64 +19,104 @@ use rustc_span::Span; /// the intrinsic llvm.instrprof.increment. pub struct InstrumentCoverage; +struct Instrumentor<'tcx> { + tcx: TyCtxt<'tcx>, + num_counters: usize, +} + impl<'tcx> MirPass<'tcx> for InstrumentCoverage { - fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) { + fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, mir_body: &mut mir::Body<'tcx>) { if tcx.sess.opts.debugging_opts.instrument_coverage { - debug!("instrumenting {:?}", src.def_id()); - instrument_coverage(tcx, body); + // If the InstrumentCoverage pass is called on promoted MIRs, skip them. + // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 + if src.promoted.is_none() { + assert!(mir_body.coverage_data.is_none()); + + let hash = hash_mir_source(tcx, &src); + + debug!( + "instrumenting {:?}, hash: {}, span: {}", + src.def_id(), + hash, + tcx.sess.source_map().span_to_string(mir_body.span) + ); + + let num_counters = Instrumentor::new(tcx).inject_counters(mir_body); + + mir_body.coverage_data = Some(CoverageData { hash, num_counters }); + } } } } -// The first counter (start of the function) is index zero. -const INIT_FUNCTION_COUNTER: u32 = 0; - -/// Injects calls to placeholder function `count_code_region()`. -// FIXME(richkadel): As a first step, counters are only injected at the top of each function. -// The complete solution will inject counters at each conditional code branch. -pub fn instrument_coverage<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - let span = body.span.shrink_to_lo(); - - let count_code_region_fn = function_handle( - tcx, - tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None), - span, - ); - let counter_index = Operand::const_from_scalar( - tcx, - tcx.types.u32, - Scalar::from_u32(INIT_FUNCTION_COUNTER), - span, - ); - - let mut patch = MirPatch::new(body); - - let new_block = patch.new_block(placeholder_block(SourceInfo::outermost(body.span))); - let next_block = START_BLOCK; - - let temp = patch.new_temp(tcx.mk_unit(), body.span); - patch.patch_terminator( - new_block, - TerminatorKind::Call { - func: count_code_region_fn, - args: vec![counter_index], - // new_block will swapped with the next_block, after applying patch - destination: Some((Place::from(temp), new_block)), - cleanup: None, - from_hir_call: false, - fn_span: span, - }, - ); +impl<'tcx> Instrumentor<'tcx> { + fn new(tcx: TyCtxt<'tcx>) -> Self { + Self { tcx, num_counters: 0 } + } + + fn next_counter(&mut self) -> u32 { + let next = self.num_counters as u32; + self.num_counters += 1; + next + } + + fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) -> usize { + // FIXME(richkadel): As a first step, counters are only injected at the top of each + // function. The complete solution will inject counters at each conditional code branch. + let top_of_function = START_BLOCK; + let entire_function = mir_body.span; + + self.inject_counter(mir_body, top_of_function, entire_function); + + self.num_counters + } + + fn inject_counter( + &mut self, + mir_body: &mut mir::Body<'tcx>, + next_block: BasicBlock, + code_region: Span, + ) { + let injection_point = code_region.shrink_to_lo(); - patch.add_statement(new_block.start_location(), StatementKind::StorageLive(temp)); - patch.add_statement(next_block.start_location(), StatementKind::StorageDead(temp)); + let count_code_region_fn = function_handle( + self.tcx, + self.tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None), + injection_point, + ); + let counter_index = Operand::const_from_scalar( + self.tcx, + self.tcx.types.u32, + Scalar::from_u32(self.next_counter()), + injection_point, + ); - patch.apply(body); + let mut patch = MirPatch::new(mir_body); - // To insert the `new_block` in front of the first block in the counted branch (for example, - // the START_BLOCK, at the top of the function), just swap the indexes, leaving the rest of the - // graph unchanged. - body.basic_blocks_mut().swap(next_block, new_block); + let temp = patch.new_temp(self.tcx.mk_unit(), code_region); + let new_block = patch.new_block(placeholder_block(code_region)); + patch.patch_terminator( + new_block, + TerminatorKind::Call { + func: count_code_region_fn, + args: vec![counter_index], + // new_block will swapped with the next_block, after applying patch + destination: Some((Place::from(temp), new_block)), + cleanup: None, + from_hir_call: false, + fn_span: injection_point, + }, + ); + + patch.add_statement(new_block.start_location(), StatementKind::StorageLive(temp)); + patch.add_statement(next_block.start_location(), StatementKind::StorageDead(temp)); + + patch.apply(mir_body); + + // To insert the `new_block` in front of the first block in the counted branch (the + // `next_block`), just swap the indexes, leaving the rest of the graph unchanged. + mir_body.basic_blocks_mut().swap(next_block, new_block); + } } fn function_handle<'tcx>(tcx: TyCtxt<'tcx>, fn_def_id: DefId, span: Span) -> Operand<'tcx> { @@ -79,14 +126,59 @@ fn function_handle<'tcx>(tcx: TyCtxt<'tcx>, fn_def_id: DefId, span: Span) -> Ope Operand::function_handle(tcx, fn_def_id, substs, span) } -fn placeholder_block<'tcx>(source_info: SourceInfo) -> BasicBlockData<'tcx> { +fn placeholder_block(span: Span) -> BasicBlockData<'tcx> { BasicBlockData { statements: vec![], terminator: Some(Terminator { - source_info, + source_info: SourceInfo::outermost(span), // this gets overwritten by the counter Call kind: TerminatorKind::Unreachable, }), is_cleanup: false, } } + +fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, src: &MirSource<'tcx>) -> u64 { + let fn_body_id = match tcx.hir().get_if_local(src.def_id()) { + Some(node) => match associated_body(node) { + Some(body_id) => body_id, + _ => bug!("instrumented MirSource does not include a function body: {:?}", node), + }, + None => bug!("instrumented MirSource is not local: {:?}", src), + }; + let hir_body = tcx.hir().body(fn_body_id); + let mut hcx = tcx.create_no_span_stable_hashing_context(); + hash(&mut hcx, &hir_body.value).to_smaller_hash() +} + +fn hash( + hcx: &mut StableHashingContext<'tcx>, + node: &impl HashStable>, +) -> Fingerprint { + let mut stable_hasher = StableHasher::new(); + node.hash_stable(hcx, &mut stable_hasher); + stable_hasher.finish() +} + +fn associated_body<'hir>(node: Node<'hir>) -> Option { + match node { + Node::Item(Item { + kind: ItemKind::Const(_, body) | ItemKind::Static(.., body) | ItemKind::Fn(.., body), + .. + }) + | Node::TraitItem(TraitItem { + kind: + TraitItemKind::Const(_, Some(body)) | TraitItemKind::Fn(_, TraitFn::Provided(body)), + .. + }) + | Node::ImplItem(ImplItem { + kind: ImplItemKind::Const(_, body) | ImplItemKind::Fn(_, body), + .. + }) + | Node::Expr(Expr { kind: ExprKind::Closure(.., body, _, _), .. }) => Some(*body), + + Node::AnonConst(constant) => Some(constant.body), + + _ => None, + } +} diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index 2d231359057fd..c2d056ad26d0b 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -877,8 +877,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, (such as entering an empty infinite loop) by inserting llvm.sideeffect \ (default: no)"), instrument_coverage: bool = (false, parse_bool, [TRACKED], - "instrument the generated code with LLVM code region counters to \ - (in the future) generate coverage reports (experimental; default: no)"), + "instrument the generated code with LLVM code region counters to (in the \ + future) generate coverage reports (default: no; note, the compiler build \ + config must include `profiler = true`)"), instrument_mcount: bool = (false, parse_bool, [TRACKED], "insert function instrument code for mcount-based tracing (default: no)"), keep_hygiene_data: bool = (false, parse_bool, [UNTRACKED], diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 970a26325926c..6efe9b20d0d8f 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -587,6 +587,7 @@ symbols! { proc_macro_mod, proc_macro_non_items, proc_macro_path_invoc, + profiler_builtins, profiler_runtime, ptr_offset_from, pub_restricted, From 8fc2eeb1c81aef42855257adc11791dcffe803cb Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 10 Jun 2020 14:24:28 -0700 Subject: [PATCH 05/24] Use newtype to map from `Local` to `GeneratorSavedLocal` --- src/librustc_mir/transform/generator.rs | 108 ++++++++++++++---------- 1 file changed, 64 insertions(+), 44 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index c8702eeae1d5b..1445315c6b18d 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -72,7 +72,7 @@ use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt}; use rustc_target::abi::VariantIdx; use rustc_target::spec::PanicStrategy; use std::borrow::Cow; -use std::iter; +use std::{iter, ops}; pub struct StateTransform; @@ -417,11 +417,7 @@ fn replace_local<'tcx>( struct LivenessInfo { /// Which locals are live across any suspension point. - /// - /// GeneratorSavedLocal is indexed in terms of the elements in this set; - /// i.e. GeneratorSavedLocal::new(1) corresponds to the second local - /// included in this set. - live_locals: BitSet, + saved_locals: GeneratorSavedLocals, /// The set of saved locals live at each suspension point. live_locals_at_suspension_points: Vec>, @@ -524,49 +520,75 @@ fn locals_live_across_suspend_points( live_locals_at_suspension_points.push(live_locals); } } + debug!("live_locals_anywhere = {:?}", live_locals_at_any_suspension_point); + let saved_locals = GeneratorSavedLocals(live_locals_at_any_suspension_point); // Renumber our liveness_map bitsets to include only the locals we are // saving. let live_locals_at_suspension_points = live_locals_at_suspension_points .iter() - .map(|live_here| renumber_bitset(&live_here, &live_locals_at_any_suspension_point)) + .map(|live_here| saved_locals.renumber_bitset(&live_here)) .collect(); let storage_conflicts = compute_storage_conflicts( body_ref, - &live_locals_at_any_suspension_point, + &saved_locals, always_live_locals.clone(), requires_storage_results, ); LivenessInfo { - live_locals: live_locals_at_any_suspension_point, + saved_locals, live_locals_at_suspension_points, storage_conflicts, storage_liveness: storage_liveness_map, } } -/// Renumbers the items present in `stored_locals` and applies the renumbering -/// to 'input`. +/// The set of `Local`s that must be saved across yield points. /// -/// For example, if `stored_locals = [1, 3, 5]`, this would be renumbered to -/// `[0, 1, 2]`. Thus, if `input = [3, 5]` we would return `[1, 2]`. -fn renumber_bitset( - input: &BitSet, - stored_locals: &BitSet, -) -> BitSet { - assert!(stored_locals.superset(&input), "{:?} not a superset of {:?}", stored_locals, input); - let mut out = BitSet::new_empty(stored_locals.count()); - for (idx, local) in stored_locals.iter().enumerate() { - let saved_local = GeneratorSavedLocal::from(idx); - if input.contains(local) { - out.insert(saved_local); +/// `GeneratorSavedLocal` is indexed in terms of the elements in this set; +/// i.e. `GeneratorSavedLocal::new(1)` corresponds to the second local +/// included in this set. +struct GeneratorSavedLocals(BitSet); + +impl GeneratorSavedLocals { + /// Returns an iterator over each `GeneratorSavedLocal` along with the `Local` it corresponds + /// to. + fn iter_enumerated(&self) -> impl '_ + Iterator { + self.iter().enumerate().map(|(i, l)| (GeneratorSavedLocal::from(i), l)) + } + + /// Transforms a `BitSet` that contains only locals saved across yield points to the + /// equivalent `BitSet`. + fn renumber_bitset(&self, input: &BitSet) -> BitSet { + assert!(self.superset(&input), "{:?} not a superset of {:?}", self.0, input); + let mut out = BitSet::new_empty(self.count()); + for (saved_local, local) in self.iter_enumerated() { + if input.contains(local) { + out.insert(saved_local); + } + } + out + } + + fn get(&self, local: Local) -> Option { + if !self.contains(local) { + return None; } + + let idx = self.iter().take_while(|&l| l < local).count(); + Some(GeneratorSavedLocal::new(idx)) + } +} + +impl ops::Deref for GeneratorSavedLocals { + type Target = BitSet; + + fn deref(&self) -> &Self::Target { + &self.0 } - debug!("renumber_bitset({:?}, {:?}) => {:?}", input, stored_locals, out); - out } /// For every saved local, looks for which locals are StorageLive at the same @@ -575,11 +597,11 @@ fn renumber_bitset( /// computation; see `GeneratorLayout` for more. fn compute_storage_conflicts( body: &'mir Body<'tcx>, - stored_locals: &BitSet, + saved_locals: &GeneratorSavedLocals, always_live_locals: storage::AlwaysLiveLocals, requires_storage: dataflow::Results<'tcx, MaybeRequiresStorage<'mir, 'tcx>>, ) -> BitMatrix { - assert_eq!(body.local_decls.len(), stored_locals.domain_size()); + assert_eq!(body.local_decls.len(), saved_locals.domain_size()); debug!("compute_storage_conflicts({:?})", body.span); debug!("always_live = {:?}", always_live_locals); @@ -587,12 +609,12 @@ fn compute_storage_conflicts( // Locals that are always live or ones that need to be stored across // suspension points are not eligible for overlap. let mut ineligible_locals = always_live_locals.into_inner(); - ineligible_locals.intersect(stored_locals); + ineligible_locals.intersect(saved_locals); // Compute the storage conflicts for all eligible locals. let mut visitor = StorageConflictVisitor { body, - stored_locals: &stored_locals, + saved_locals: &saved_locals, local_conflicts: BitMatrix::from_row_n(&ineligible_locals, body.local_decls.len()), }; @@ -609,16 +631,14 @@ fn compute_storage_conflicts( // However, in practice these bitsets are not usually large. The layout code // also needs to keep track of how many conflicts each local has, so it's // simpler to keep it this way for now. - let mut storage_conflicts = BitMatrix::new(stored_locals.count(), stored_locals.count()); - for (idx_a, local_a) in stored_locals.iter().enumerate() { - let saved_local_a = GeneratorSavedLocal::new(idx_a); + let mut storage_conflicts = BitMatrix::new(saved_locals.count(), saved_locals.count()); + for (saved_local_a, local_a) in saved_locals.iter_enumerated() { if ineligible_locals.contains(local_a) { // Conflicts with everything. storage_conflicts.insert_all_into_row(saved_local_a); } else { // Keep overlap information only for stored locals. - for (idx_b, local_b) in stored_locals.iter().enumerate() { - let saved_local_b = GeneratorSavedLocal::new(idx_b); + for (saved_local_b, local_b) in saved_locals.iter_enumerated() { if local_conflicts.contains(local_a, local_b) { storage_conflicts.insert(saved_local_a, saved_local_b); } @@ -630,7 +650,7 @@ fn compute_storage_conflicts( struct StorageConflictVisitor<'mir, 'tcx, 's> { body: &'mir Body<'tcx>, - stored_locals: &'s BitSet, + saved_locals: &'s GeneratorSavedLocals, // FIXME(tmandry): Consider using sparse bitsets here once we have good // benchmarks for generators. local_conflicts: BitMatrix, @@ -666,7 +686,7 @@ impl<'body, 'tcx, 's> StorageConflictVisitor<'body, 'tcx, 's> { } let mut eligible_storage_live = flow_state.clone(); - eligible_storage_live.intersect(&self.stored_locals); + eligible_storage_live.intersect(&self.saved_locals); for local in eligible_storage_live.iter() { self.local_conflicts.union_row_with(&eligible_storage_live, local); @@ -678,7 +698,7 @@ impl<'body, 'tcx, 's> StorageConflictVisitor<'body, 'tcx, 's> { } } -/// Validates the typeck view of the generator against the actual set of types retained between +/// Validates the typeck view of the generator against the actual set of types saved between /// yield points. fn sanitize_witness<'tcx>( tcx: TyCtxt<'tcx>, @@ -686,7 +706,7 @@ fn sanitize_witness<'tcx>( did: DefId, witness: Ty<'tcx>, upvars: &Vec>, - retained: &BitSet, + saved_locals: &GeneratorSavedLocals, ) { let allowed_upvars = tcx.erase_regions(upvars); let allowed = match witness.kind { @@ -703,8 +723,8 @@ fn sanitize_witness<'tcx>( let param_env = tcx.param_env(did); for (local, decl) in body.local_decls.iter_enumerated() { - // Ignore locals which are internal or not retained between yields. - if !retained.contains(local) || decl.internal { + // Ignore locals which are internal or not saved between yields. + if !saved_locals.contains(local) || decl.internal { continue; } let decl_ty = tcx.normalize_erasing_regions(param_env, decl.ty); @@ -738,21 +758,21 @@ fn compute_layout<'tcx>( ) { // Use a liveness analysis to compute locals which are live across a suspension point let LivenessInfo { - live_locals, + saved_locals, live_locals_at_suspension_points, storage_conflicts, storage_liveness, } = locals_live_across_suspend_points(tcx, body, source, always_live_locals, movable); - sanitize_witness(tcx, body, source.def_id(), interior, upvars, &live_locals); + sanitize_witness(tcx, body, source.def_id(), interior, upvars, &saved_locals); // Gather live local types and their indices. let mut locals = IndexVec::::new(); let mut tys = IndexVec::::new(); - for (idx, local) in live_locals.iter().enumerate() { + for (saved_local, local) in saved_locals.iter_enumerated() { locals.push(local); tys.push(body.local_decls[local].ty); - debug!("generator saved local {:?} => {:?}", GeneratorSavedLocal::from(idx), local); + debug!("generator saved local {:?} => {:?}", saved_local, local); } // Leave empty variants for the UNRESUMED, RETURNED, and POISONED states. From c178e6436a4c3f34b8790a908de044bba9401284 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Wed, 10 Jun 2020 14:00:59 -0700 Subject: [PATCH 06/24] Look for stores between non-conflicting generator saved locals This is to prevent the miscompilation in #73137 from reappearing. Only runs with `-Zvalidate-mir`. --- src/librustc_mir/transform/generator.rs | 160 ++++++++++++++++++++++-- 1 file changed, 147 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 1445315c6b18d..9a15fba2bc8a0 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -64,7 +64,7 @@ use rustc_hir::def_id::DefId; use rustc_hir::lang_items::{GeneratorStateLangItem, PinTypeLangItem}; use rustc_index::bit_set::{BitMatrix, BitSet}; use rustc_index::vec::{Idx, IndexVec}; -use rustc_middle::mir::visit::{MutVisitor, PlaceContext}; +use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; use rustc_middle::mir::*; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::GeneratorSubsts; @@ -744,27 +744,19 @@ fn sanitize_witness<'tcx>( } fn compute_layout<'tcx>( - tcx: TyCtxt<'tcx>, - source: MirSource<'tcx>, - upvars: &Vec>, - interior: Ty<'tcx>, - always_live_locals: &storage::AlwaysLiveLocals, - movable: bool, + liveness: LivenessInfo, body: &mut Body<'tcx>, ) -> ( FxHashMap, VariantIdx, usize)>, GeneratorLayout<'tcx>, IndexVec>>, ) { - // Use a liveness analysis to compute locals which are live across a suspension point let LivenessInfo { saved_locals, live_locals_at_suspension_points, storage_conflicts, storage_liveness, - } = locals_live_across_suspend_points(tcx, body, source, always_live_locals, movable); - - sanitize_witness(tcx, body, source.def_id(), interior, upvars, &saved_locals); + } = liveness; // Gather live local types and their indices. let mut locals = IndexVec::::new(); @@ -1280,11 +1272,25 @@ impl<'tcx> MirPass<'tcx> for StateTransform { let always_live_locals = storage::AlwaysLiveLocals::new(&body); + let liveness_info = + locals_live_across_suspend_points(tcx, body, source, &always_live_locals, movable); + + sanitize_witness(tcx, body, def_id, interior, &upvars, &liveness_info.saved_locals); + + if tcx.sess.opts.debugging_opts.validate_mir { + let mut vis = EnsureGeneratorFieldAssignmentsNeverAlias { + assigned_local: None, + saved_locals: &liveness_info.saved_locals, + storage_conflicts: &liveness_info.storage_conflicts, + }; + + vis.visit_body(body); + } + // Extract locals which are live across suspension point into `layout` // `remap` gives a mapping from local indices onto generator struct indices // `storage_liveness` tells us which locals have live storage at suspension points - let (remap, layout, storage_liveness) = - compute_layout(tcx, source, &upvars, interior, &always_live_locals, movable, body); + let (remap, layout, storage_liveness) = compute_layout(liveness_info, body); let can_return = can_return(tcx, body); @@ -1335,3 +1341,131 @@ impl<'tcx> MirPass<'tcx> for StateTransform { create_generator_resume_function(tcx, transform, source, body, can_return); } } + +/// Looks for any assignments between locals (e.g., `_4 = _5`) that will both be converted to fields +/// in the generator state machine but whose storage does not conflict. +/// +/// Validation needs to happen immediately *before* `TransformVisitor` is invoked, not after. +/// +/// This condition would arise when the assignment is the last use of `_5` but the initial +/// definition of `_4` if we weren't extra careful to mark all locals used inside a statement as +/// conflicting. Non-conflicting generator saved locals may be stored at the same location within +/// the generator state machine, which would result in ill-formed MIR: the left-hand and right-hand +/// sides of an assignment may not alias. This caused a miscompilation in [#73137]. +/// +/// [#73137]: https://github.com/rust-lang/rust/issues/73137 +struct EnsureGeneratorFieldAssignmentsNeverAlias<'a> { + saved_locals: &'a GeneratorSavedLocals, + storage_conflicts: &'a BitMatrix, + assigned_local: Option, +} + +impl EnsureGeneratorFieldAssignmentsNeverAlias<'_> { + fn saved_local_for_direct_place(&self, place: Place<'_>) -> Option { + if place.is_indirect() { + return None; + } + + self.saved_locals.get(place.local) + } + + fn check_assigned_place(&mut self, place: Place<'tcx>, f: impl FnOnce(&mut Self)) { + if let Some(assigned_local) = self.saved_local_for_direct_place(place) { + assert!(self.assigned_local.is_none(), "`check_assigned_local` must not recurse"); + + self.assigned_local = Some(assigned_local); + f(self); + self.assigned_local = None; + } + } +} + +impl Visitor<'tcx> for EnsureGeneratorFieldAssignmentsNeverAlias<'_> { + fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) { + let lhs = match self.assigned_local { + Some(l) => l, + None => { + // We should be visiting all places used in the MIR. + assert!(!context.is_use()); + return; + } + }; + + let rhs = match self.saved_local_for_direct_place(*place) { + Some(l) => l, + None => return, + }; + + if !self.storage_conflicts.contains(lhs, rhs) { + bug!( + "Assignment between generator saved locals whose storage does not conflict: \ + {:?}: {:?} = {:?}", + location, + lhs, + rhs, + ); + } + } + + fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { + match &statement.kind { + StatementKind::Assign(box (lhs, rhs)) => { + self.check_assigned_place(*lhs, |this| this.visit_rvalue(rhs, location)); + } + + // FIXME: Does `llvm_asm!` have any aliasing requirements? + StatementKind::LlvmInlineAsm(_) => {} + + StatementKind::FakeRead(..) + | StatementKind::SetDiscriminant { .. } + | StatementKind::StorageLive(_) + | StatementKind::StorageDead(_) + | StatementKind::Retag(..) + | StatementKind::AscribeUserType(..) + | StatementKind::Nop => {} + } + } + + fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) { + // Checking for aliasing in terminators is probably overkill, but until we have actual + // semantics, we should be conservative here. + match &terminator.kind { + TerminatorKind::Call { + func, + args, + destination: Some((dest, _)), + cleanup: _, + from_hir_call: _, + fn_span: _, + } => { + self.check_assigned_place(*dest, |this| { + this.visit_operand(func, location); + for arg in args { + this.visit_operand(arg, location); + } + }); + } + + TerminatorKind::Yield { value, resume: _, resume_arg, drop: _ } => { + self.check_assigned_place(*resume_arg, |this| this.visit_operand(value, location)); + } + + // FIXME: Does `asm!` have any aliasing requirements? + TerminatorKind::InlineAsm { .. } => {} + + TerminatorKind::Call { .. } + | TerminatorKind::Goto { .. } + | TerminatorKind::SwitchInt { .. } + | TerminatorKind::Resume + | TerminatorKind::Abort + | TerminatorKind::Return + | TerminatorKind::Unreachable + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::Assert { .. } + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseEdge { .. } + | TerminatorKind::FalseUnwind { .. } => {} + } + } +} From b2ec645d16ce9a3345b2f9cb527ca52e86e54324 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Fri, 19 Jun 2020 10:27:10 -0700 Subject: [PATCH 07/24] Incorporate review suggestions Co-authored-by: Tyler Mandry --- src/librustc_mir/transform/generator.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 9a15fba2bc8a0..59be8dc224dee 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -1343,7 +1343,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform { } /// Looks for any assignments between locals (e.g., `_4 = _5`) that will both be converted to fields -/// in the generator state machine but whose storage does not conflict. +/// in the generator state machine but whose storage is not marked as conflicting /// /// Validation needs to happen immediately *before* `TransformVisitor` is invoked, not after. /// @@ -1371,7 +1371,7 @@ impl EnsureGeneratorFieldAssignmentsNeverAlias<'_> { fn check_assigned_place(&mut self, place: Place<'tcx>, f: impl FnOnce(&mut Self)) { if let Some(assigned_local) = self.saved_local_for_direct_place(place) { - assert!(self.assigned_local.is_none(), "`check_assigned_local` must not recurse"); + assert!(self.assigned_local.is_none(), "`check_assigned_place` must not recurse"); self.assigned_local = Some(assigned_local); f(self); @@ -1385,7 +1385,10 @@ impl Visitor<'tcx> for EnsureGeneratorFieldAssignmentsNeverAlias<'_> { let lhs = match self.assigned_local { Some(l) => l, None => { - // We should be visiting all places used in the MIR. + // This visitor only invokes `visit_place` for the right-hand side of an assignment + // and only after setting `self.assigned_local`. However, the default impl of + // `Visitor::super_body` may call `visit_place` with a `NonUseContext` for places + // with debuginfo. Ignore them here. assert!(!context.is_use()); return; } @@ -1398,8 +1401,8 @@ impl Visitor<'tcx> for EnsureGeneratorFieldAssignmentsNeverAlias<'_> { if !self.storage_conflicts.contains(lhs, rhs) { bug!( - "Assignment between generator saved locals whose storage does not conflict: \ - {:?}: {:?} = {:?}", + "Assignment between generator saved locals whose storage is not \ + marked as conflicting: {:?}: {:?} = {:?}", location, lhs, rhs, From 95f8daa82b52e95081b66d58953c2329a9f0458e Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 19 Jun 2020 20:06:49 -0400 Subject: [PATCH 08/24] Fix -Z unpretty=everybody_loops It turns out that this has not been working for who knows how long. Previously: ``` pub fn h() { 1 + 2; } ``` After this change: ``` pub fn h() { loop {} } ``` This only affected the pass when run with the command line pretty-printing option, so rustdoc was still replacing bodies with `loop {}`. --- src/librustc_driver/lib.rs | 1 + src/librustc_interface/interface.rs | 1 + src/librustc_interface/passes.rs | 3 +++ src/librustc_interface/queries.rs | 1 + src/librustc_session/config.rs | 7 +++++-- 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index ccea041699ee1..b45ab0f80ffac 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -307,6 +307,7 @@ pub fn run_compiler( compiler.output_file().as_ref().map(|p| &**p), ); } + trace!("finished pretty-printing"); return early_exit(); } diff --git a/src/librustc_interface/interface.rs b/src/librustc_interface/interface.rs index 5aad64f84cee3..920cc6021e687 100644 --- a/src/librustc_interface/interface.rs +++ b/src/librustc_interface/interface.rs @@ -202,6 +202,7 @@ pub fn run_compiler_in_existing_thread_pool( } pub fn run_compiler(mut config: Config, f: impl FnOnce(&Compiler) -> R + Send) -> R { + log::trace!("run_compiler"); let stderr = config.stderr.take(); util::spawn_thread_pool( config.opts.edition, diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 1ed9bc3f1f509..c0a67f20a1e8c 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -101,6 +101,7 @@ pub fn configure_and_expand( krate: ast::Crate, crate_name: &str, ) -> Result<(ast::Crate, BoxedResolver)> { + log::trace!("configure_and_expand"); // Currently, we ignore the name resolution data structures for the purposes of dependency // tracking. Instead we will run name resolution and include its output in the hash of each // item, much like we do for macro expansion. In other words, the hash reflects not just @@ -230,6 +231,7 @@ fn configure_and_expand_inner<'a>( resolver_arenas: &'a ResolverArenas<'a>, metadata_loader: &'a MetadataLoaderDyn, ) -> Result<(ast::Crate, Resolver<'a>)> { + log::trace!("configure_and_expand_inner"); pre_expansion_lint(sess, lint_store, &krate); let mut resolver = Resolver::new(sess, &krate, crate_name, metadata_loader, &resolver_arenas); @@ -357,6 +359,7 @@ fn configure_and_expand_inner<'a>( should_loop |= true; } if should_loop { + log::debug!("replacing bodies with loop {{}}"); util::ReplaceBodyWithLoop::new(&mut resolver).visit_crate(&mut krate); } diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 283be165c192c..4a41c3e97cafc 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -169,6 +169,7 @@ impl<'tcx> Queries<'tcx> { pub fn expansion( &self, ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { + log::trace!("expansion"); self.expansion.compute(|| { let crate_name = self.crate_name()?.peek().clone(); let (krate, lint_store) = self.register_plugins()?.take(); diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index 411a6eecbba15..ff94a43c4f1a2 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -1826,6 +1826,7 @@ fn parse_pretty( } } }; + log::debug!("got unpretty option: {:?}", first); first } } @@ -1954,9 +1955,11 @@ impl PpMode { use PpMode::*; use PpSourceMode::*; match *self { - PpmSource(PpmNormal | PpmEveryBodyLoops | PpmIdentified) => false, + PpmSource(PpmNormal | PpmIdentified) => false, - PpmSource(PpmExpanded | PpmExpandedIdentified | PpmExpandedHygiene) + PpmSource( + PpmExpanded | PpmEveryBodyLoops | PpmExpandedIdentified | PpmExpandedHygiene, + ) | PpmHir(_) | PpmHirTree(_) | PpmMir From f60513ec8d97414a346cedb755a4cac3abd55ed1 Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sat, 20 Jun 2020 19:59:29 +0100 Subject: [PATCH 09/24] Move remaining `NodeId` APIs from `Definitions` to `Resolver` --- Cargo.lock | 1 + src/librustc_ast_lowering/item.rs | 17 +-- src/librustc_ast_lowering/lib.rs | 50 ++++++--- src/librustc_hir/definitions.rs | 105 ++---------------- src/librustc_metadata/creader.rs | 4 +- src/librustc_resolve/Cargo.toml | 1 + src/librustc_resolve/build_reduced_graph.rs | 67 +++++------ src/librustc_resolve/check_unused.rs | 5 +- src/librustc_resolve/def_collector.rs | 27 +++-- src/librustc_resolve/diagnostics.rs | 8 +- src/librustc_resolve/imports.rs | 3 +- src/librustc_resolve/late.rs | 17 +-- src/librustc_resolve/late/diagnostics.rs | 6 +- src/librustc_resolve/lib.rs | 117 ++++++++++++++++---- src/librustc_resolve/macros.rs | 7 +- 15 files changed, 219 insertions(+), 216 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5bb6cda64cba8..d5bc99e12651f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4242,6 +4242,7 @@ dependencies = [ "rustc_expand", "rustc_feature", "rustc_hir", + "rustc_index", "rustc_metadata", "rustc_middle", "rustc_session", diff --git a/src/librustc_ast_lowering/item.rs b/src/librustc_ast_lowering/item.rs index 8cfbd408e22b3..00665c4cafb6b 100644 --- a/src/librustc_ast_lowering/item.rs +++ b/src/librustc_ast_lowering/item.rs @@ -253,7 +253,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ItemKind::Const(ty, body_id) } ItemKind::Fn(_, FnSig { ref decl, header }, ref generics, ref body) => { - let fn_def_id = self.resolver.definitions().local_def_id(id); + let fn_def_id = self.resolver.local_def_id(id); self.with_new_scopes(|this| { this.current_item = Some(ident.span); @@ -342,7 +342,7 @@ impl<'hir> LoweringContext<'_, 'hir> { self_ty: ref ty, items: ref impl_items, } => { - let def_id = self.resolver.definitions().local_def_id(id); + let def_id = self.resolver.local_def_id(id); // Lower the "impl header" first. This ordering is important // for in-band lifetimes! Consider `'a` here: @@ -646,7 +646,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } fn lower_foreign_item(&mut self, i: &ForeignItem) -> hir::ForeignItem<'hir> { - let def_id = self.resolver.definitions().local_def_id(i.id); + let def_id = self.resolver.local_def_id(i.id); hir::ForeignItem { hir_id: self.lower_node_id(i.id), ident: i.ident, @@ -747,7 +747,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } fn lower_trait_item(&mut self, i: &AssocItem) -> hir::TraitItem<'hir> { - let trait_item_def_id = self.resolver.definitions().local_def_id(i.id); + let trait_item_def_id = self.resolver.local_def_id(i.id); let (generics, kind) = match i.kind { AssocItemKind::Const(_, ref ty, ref default) => { @@ -812,7 +812,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } fn lower_impl_item(&mut self, i: &AssocItem) -> hir::ImplItem<'hir> { - let impl_item_def_id = self.resolver.definitions().local_def_id(i.id); + let impl_item_def_id = self.resolver.local_def_id(i.id); let (generics, kind) = match &i.kind { AssocItemKind::Const(_, ty, expr) => { @@ -1320,12 +1320,7 @@ impl<'hir> LoweringContext<'_, 'hir> { if let Some(def_id) = def_id.as_local() { for param in &generics.params { if let GenericParamKind::Type { .. } = param.kind { - if def_id - == self - .resolver - .definitions() - .local_def_id(param.id) - { + if def_id == self.resolver.local_def_id(param.id) { add_bounds .entry(param.id) .or_default() diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 335cc3e61040d..ac8a229da43db 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -54,7 +54,7 @@ use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId, CRATE_DEF_INDEX}; use rustc_hir::definitions::{DefKey, DefPathData, Definitions}; use rustc_hir::intravisit; use rustc_hir::{ConstArg, GenericArg, ParamName}; -use rustc_index::vec::IndexVec; +use rustc_index::vec::{Idx, IndexVec}; use rustc_session::config::nightly_options; use rustc_session::lint::{builtin::BARE_TRAIT_OBJECTS, BuiltinLintDiagnostics, LintBuffer}; use rustc_session::parse::ParseSess; @@ -205,6 +205,19 @@ pub trait Resolver { fn next_node_id(&mut self) -> NodeId; fn trait_map(&self) -> &NodeMap>; + + fn opt_local_def_id(&self, node: NodeId) -> Option; + + fn local_def_id(&self, node: NodeId) -> LocalDefId; + + fn create_def_with_parent( + &mut self, + parent: LocalDefId, + node_id: ast::NodeId, + data: DefPathData, + expn_id: ExpnId, + span: Span, + ) -> LocalDefId; } type NtToTokenstream = fn(&Nonterminal, &ParseSess, Span) -> TokenStream; @@ -436,7 +449,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { match tree.kind { UseTreeKind::Simple(_, id1, id2) => { for &id in &[id1, id2] { - self.lctx.resolver.definitions().create_def_with_parent( + self.lctx.resolver.create_def_with_parent( owner, id, DefPathData::Misc, @@ -488,7 +501,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { | ItemKind::Enum(_, ref generics) | ItemKind::TyAlias(_, ref generics, ..) | ItemKind::Trait(_, _, ref generics, ..) => { - let def_id = self.lctx.resolver.definitions().local_def_id(item.id); + let def_id = self.lctx.resolver.local_def_id(item.id); let count = generics .params .iter() @@ -564,7 +577,18 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { .map(|(&k, v)| (self.node_id_to_hir_id[k].unwrap(), v.clone())) .collect(); - self.resolver.definitions().init_node_id_to_hir_id_mapping(self.node_id_to_hir_id); + let mut def_id_to_hir_id = IndexVec::default(); + + for (node_id, hir_id) in self.node_id_to_hir_id.into_iter_enumerated() { + if let Some(def_id) = self.resolver.opt_local_def_id(node_id) { + if def_id_to_hir_id.len() <= def_id.index() { + def_id_to_hir_id.resize(def_id.index() + 1, None); + } + def_id_to_hir_id[def_id] = hir_id; + } + } + + self.resolver.definitions().init_def_id_to_hir_id_mapping(def_id_to_hir_id); hir::Crate { item: hir::CrateItem { module, attrs, span: c.span }, @@ -628,7 +652,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { .item_local_id_counters .insert(owner, HIR_ID_COUNTER_LOCKED) .unwrap_or_else(|| panic!("no `item_local_id_counters` entry for {:?}", owner)); - let def_id = self.resolver.definitions().local_def_id(owner); + let def_id = self.resolver.local_def_id(owner); self.current_hir_id_owner.push((def_id, counter)); let ret = f(self); let (new_def_id, new_counter) = self.current_hir_id_owner.pop().unwrap(); @@ -671,7 +695,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { debug_assert!(local_id != HIR_ID_COUNTER_LOCKED); *local_id_counter += 1; - let owner = this.resolver.definitions().opt_local_def_id(owner).expect( + let owner = this.resolver.opt_local_def_id(owner).expect( "you forgot to call `create_def_with_parent` or are lowering node-IDs \ that do not belong to the current owner", ); @@ -800,7 +824,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { }; // Add a definition for the in-band lifetime def. - self.resolver.definitions().create_def_with_parent( + self.resolver.create_def_with_parent( parent_def_id, node_id, DefPathData::LifetimeNs(str_name), @@ -1088,7 +1112,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let impl_trait_node_id = self.resolver.next_node_id(); let parent_def_id = self.current_hir_id_owner.last().unwrap().0; - self.resolver.definitions().create_def_with_parent( + self.resolver.create_def_with_parent( parent_def_id, impl_trait_node_id, DefPathData::ImplTrait, @@ -1154,7 +1178,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let node_id = self.resolver.next_node_id(); // Add a definition for the in-band const def. - self.resolver.definitions().create_def_with_parent( + self.resolver.create_def_with_parent( parent_def_id, node_id, DefPathData::AnonConst, @@ -1339,7 +1363,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } ImplTraitContext::Universal(in_band_ty_params) => { // Add a definition for the in-band `Param`. - let def_id = self.resolver.definitions().local_def_id(def_node_id); + let def_id = self.resolver.local_def_id(def_node_id); let hir_bounds = self.lower_param_bounds( bounds, @@ -1428,7 +1452,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // frequently opened issues show. let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None); - let opaque_ty_def_id = self.resolver.definitions().local_def_id(opaque_ty_node_id); + let opaque_ty_def_id = self.resolver.local_def_id(opaque_ty_node_id); self.allocate_hir_id_counter(opaque_ty_node_id); @@ -1620,7 +1644,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let def_node_id = self.context.resolver.next_node_id(); let hir_id = self.context.lower_node_id_with_owner(def_node_id, self.opaque_ty_id); - self.context.resolver.definitions().create_def_with_parent( + self.context.resolver.create_def_with_parent( self.parent, def_node_id, DefPathData::LifetimeNs(name.ident().name), @@ -1870,7 +1894,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::Async, span, None); - let opaque_ty_def_id = self.resolver.definitions().local_def_id(opaque_ty_node_id); + let opaque_ty_def_id = self.resolver.local_def_id(opaque_ty_node_id); self.allocate_hir_id_counter(opaque_ty_node_id); diff --git a/src/librustc_hir/definitions.rs b/src/librustc_hir/definitions.rs index 5755a3db92ac1..c34607f8a94c6 100644 --- a/src/librustc_hir/definitions.rs +++ b/src/librustc_hir/definitions.rs @@ -8,14 +8,12 @@ pub use crate::def_id::DefPathHash; use crate::def_id::{CrateNum, DefId, DefIndex, LocalDefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use crate::hir; -use rustc_ast::ast; use rustc_ast::crate_disambiguator::CrateDisambiguator; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::StableHasher; use rustc_index::vec::IndexVec; use rustc_span::hygiene::ExpnId; use rustc_span::symbol::{sym, Symbol}; -use rustc_span::Span; use log::debug; use std::fmt::Write; @@ -79,11 +77,6 @@ impl DefPathTable { pub struct Definitions { table: DefPathTable, - def_id_to_span: IndexVec, - - node_id_to_def_id: FxHashMap, - def_id_to_node_id: IndexVec, - // FIXME(eddyb) ideally all `LocalDefId`s would be HIR owners. pub(super) def_id_to_hir_id: IndexVec>, /// The reverse mapping of `def_id_to_hir_id`. @@ -95,11 +88,6 @@ pub struct Definitions { /// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`. expansions_that_defined: FxHashMap, next_disambiguator: FxHashMap<(LocalDefId, DefPathData), u32>, - /// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId` - /// we know what parent node that fragment should be attached to thanks to this table. - invocation_parents: FxHashMap, - /// Indices of unnamed struct or variant fields with unresolved attributes. - placeholder_field_indices: FxHashMap, } /// A unique identifier that we can use to lookup a definition @@ -319,16 +307,6 @@ impl Definitions { }) } - #[inline] - pub fn opt_local_def_id(&self, node: ast::NodeId) -> Option { - self.node_id_to_def_id.get(&node).copied() - } - - #[inline] - pub fn local_def_id(&self, node: ast::NodeId) -> LocalDefId { - self.opt_local_def_id(node).unwrap_or_else(|| panic!("no entry for node id: `{:?}`", node)) - } - #[inline] pub fn as_local_hir_id(&self, def_id: LocalDefId) -> hir::HirId { self.local_def_id_to_hir_id(def_id) @@ -349,12 +327,6 @@ impl Definitions { self.hir_id_to_def_id.get(&hir_id).copied() } - /// Retrieves the span of the given `DefId` if `DefId` is in the local crate. - #[inline] - pub fn opt_span(&self, def_id: DefId) -> Option { - if let Some(def_id) = def_id.as_local() { Some(self.def_id_to_span[def_id]) } else { None } - } - /// Adds a root definition (no parent) and a few other reserved definitions. pub fn create_root_def( &mut self, @@ -376,12 +348,6 @@ impl Definitions { let root = LocalDefId { local_def_index: self.table.allocate(key, def_path_hash) }; assert_eq!(root.local_def_index, CRATE_DEF_INDEX); - assert_eq!(self.def_id_to_node_id.push(ast::CRATE_NODE_ID), root); - assert_eq!(self.def_id_to_span.push(rustc_span::DUMMY_SP), root); - - self.node_id_to_def_id.insert(ast::CRATE_NODE_ID, root); - self.set_invocation_parent(ExpnId::root(), root); - root } @@ -389,22 +355,12 @@ impl Definitions { pub fn create_def_with_parent( &mut self, parent: LocalDefId, - node_id: ast::NodeId, data: DefPathData, expn_id: ExpnId, - span: Span, ) -> LocalDefId { debug!( - "create_def_with_parent(parent={:?}, node_id={:?}, data={:?})", - parent, node_id, data - ); - - assert!( - !self.node_id_to_def_id.contains_key(&node_id), - "adding a def'n for node-id {:?} and data {:?} but a previous def'n exists: {:?}", - node_id, - data, - self.table.def_key(self.node_id_to_def_id[&node_id].local_def_index), + "create_def_with_parent(parent={:?}, data={:?}, expn_id={:?})", + parent, data, expn_id ); // The root node must be created with `create_root_def()`. @@ -431,17 +387,6 @@ impl Definitions { // Create the definition. let def_id = LocalDefId { local_def_index: self.table.allocate(key, def_path_hash) }; - assert_eq!(self.def_id_to_node_id.push(node_id), def_id); - assert_eq!(self.def_id_to_span.push(span), def_id); - - // Some things for which we allocate `LocalDefId`s don't correspond to - // anything in the AST, so they don't have a `NodeId`. For these cases - // we don't need a mapping from `NodeId` to `LocalDefId`. - if node_id != ast::DUMMY_NODE_ID { - debug!("create_def_with_parent: def_id_to_node_id[{:?}] <-> {:?}", def_id, node_id); - self.node_id_to_def_id.insert(node_id, def_id); - } - if expn_id != ExpnId::root() { self.expansions_that_defined.insert(def_id, expn_id); } @@ -449,32 +394,24 @@ impl Definitions { def_id } - /// Initializes the `ast::NodeId` to `HirId` mapping once it has been generated during + /// Initializes the `LocalDefId` to `HirId` mapping once it has been generated during /// AST to HIR lowering. - pub fn init_node_id_to_hir_id_mapping( + pub fn init_def_id_to_hir_id_mapping( &mut self, - mapping: IndexVec>, + mapping: IndexVec>, ) { assert!( self.def_id_to_hir_id.is_empty(), "trying to initialize `LocalDefId` <-> `HirId` mappings twice" ); - self.def_id_to_hir_id = self - .def_id_to_node_id - .iter() - .map(|&node_id| mapping.get(node_id).and_then(|&hir_id| hir_id)) - .collect(); - // Build the reverse mapping of `def_id_to_hir_id`. self.hir_id_to_def_id = mapping - .into_iter_enumerated() - .filter_map(|(node_id, hir_id)| { - hir_id.and_then(|hir_id| { - self.node_id_to_def_id.get(&node_id).map(|&def_id| (hir_id, def_id)) - }) - }) + .iter_enumerated() + .filter_map(|(def_id, hir_id)| hir_id.map(|hir_id| (hir_id, def_id))) .collect(); + + self.def_id_to_hir_id = mapping; } pub fn expansion_that_defined(&self, id: LocalDefId) -> ExpnId { @@ -488,30 +425,6 @@ impl Definitions { pub fn add_parent_module_of_macro_def(&mut self, expn_id: ExpnId, module: DefId) { self.parent_modules_of_macro_defs.insert(expn_id, module); } - - pub fn invocation_parent(&self, invoc_id: ExpnId) -> LocalDefId { - self.invocation_parents[&invoc_id] - } - - pub fn set_invocation_parent(&mut self, invoc_id: ExpnId, parent: LocalDefId) { - let old_parent = self.invocation_parents.insert(invoc_id, parent); - assert!(old_parent.is_none(), "parent `LocalDefId` is reset for an invocation"); - } - - pub fn placeholder_field_index(&self, node_id: ast::NodeId) -> usize { - self.placeholder_field_indices[&node_id] - } - - pub fn set_placeholder_field_index(&mut self, node_id: ast::NodeId, index: usize) { - let old_index = self.placeholder_field_indices.insert(node_id, index); - assert!(old_index.is_none(), "placeholder field index is reset for a node ID"); - } - - pub fn lint_node_id(&mut self, expn_id: ExpnId) -> ast::NodeId { - self.invocation_parents - .get(&expn_id) - .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[*id]) - } } impl DefPathData { diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 0dc007bbfd72f..2c80c846681a6 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -10,7 +10,7 @@ use rustc_data_structures::svh::Svh; use rustc_data_structures::sync::Lrc; use rustc_errors::struct_span_err; use rustc_expand::base::SyntaxExtension; -use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, LocalDefId, LOCAL_CRATE}; use rustc_hir::definitions::Definitions; use rustc_index::vec::IndexVec; use rustc_middle::middle::cstore::DepKind; @@ -896,6 +896,7 @@ impl<'a> CrateLoader<'a> { &mut self, item: &ast::Item, definitions: &Definitions, + def_id: LocalDefId, ) -> CrateNum { match item.kind { ast::ItemKind::ExternCrate(orig_name) => { @@ -918,7 +919,6 @@ impl<'a> CrateLoader<'a> { let cnum = self.resolve_crate(name, item.span, dep_kind, None); - let def_id = definitions.opt_local_def_id(item.id).unwrap(); let path_len = definitions.def_path(def_id).data.len(); self.update_extern_crate( cnum, diff --git a/src/librustc_resolve/Cargo.toml b/src/librustc_resolve/Cargo.toml index fa5c557b5d9c6..6f6104c3d6932 100644 --- a/src/librustc_resolve/Cargo.toml +++ b/src/librustc_resolve/Cargo.toml @@ -24,6 +24,7 @@ rustc_errors = { path = "../librustc_errors" } rustc_expand = { path = "../librustc_expand" } rustc_feature = { path = "../librustc_feature" } rustc_hir = { path = "../librustc_hir" } +rustc_index = { path = "../librustc_index" } rustc_metadata = { path = "../librustc_metadata" } rustc_session = { path = "../librustc_session" } rustc_span = { path = "../librustc_span" } diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 8432e34a5271c..ef43f597eab47 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -19,6 +19,7 @@ use rustc_ast::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind, use rustc_ast::ast::{AssocItem, AssocItemKind, MetaItemKind, StmtKind}; use rustc_ast::token::{self, Token}; use rustc_ast::visit::{self, AssocCtxt, Visitor}; +use rustc_ast_lowering::Resolver as ResolverAstLowering; use rustc_attr as attr; use rustc_data_structures::sync::Lrc; use rustc_errors::{struct_span_err, Applicability}; @@ -171,7 +172,7 @@ impl<'a> Resolver<'a> { fragment: &AstFragment, parent_scope: ParentScope<'a>, ) -> MacroRulesScope<'a> { - collect_definitions(&mut self.definitions, fragment, parent_scope.expansion); + collect_definitions(self, fragment, parent_scope.expansion); let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope }; fragment.visit_with(&mut visitor); visitor.parent_scope.macro_rules @@ -647,9 +648,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } else if orig_name == Some(kw::SelfLower) { self.r.graph_root } else { - let def_id = self.r.definitions.local_def_id(item.id); + let def_id = self.r.local_def_id(item.id); let crate_id = - self.r.crate_loader.process_extern_crate(item, &self.r.definitions); + self.r.crate_loader.process_extern_crate(item, &self.r.definitions, def_id); self.r.extern_crate_map.insert(def_id, crate_id); self.r.get_module(DefId { krate: crate_id, index: CRATE_DEF_INDEX }) }; @@ -704,7 +705,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { ItemKind::Mod(..) if ident.name == kw::Invalid => {} // Crate root ItemKind::Mod(..) => { - let def_id = self.r.definitions.local_def_id(item.id); + let def_id = self.r.local_def_id(item.id); let module_kind = ModuleKind::Def(DefKind::Mod, def_id.to_def_id(), ident.name); let module = self.r.arenas.alloc_module(ModuleData { no_implicit_prelude: parent.no_implicit_prelude || { @@ -727,18 +728,15 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // These items live in the value namespace. ItemKind::Static(..) => { - let res = - Res::Def(DefKind::Static, self.r.definitions.local_def_id(item.id).to_def_id()); + let res = Res::Def(DefKind::Static, self.r.local_def_id(item.id).to_def_id()); self.r.define(parent, ident, ValueNS, (res, vis, sp, expansion)); } ItemKind::Const(..) => { - let res = - Res::Def(DefKind::Const, self.r.definitions.local_def_id(item.id).to_def_id()); + let res = Res::Def(DefKind::Const, self.r.local_def_id(item.id).to_def_id()); self.r.define(parent, ident, ValueNS, (res, vis, sp, expansion)); } ItemKind::Fn(..) => { - let res = - Res::Def(DefKind::Fn, self.r.definitions.local_def_id(item.id).to_def_id()); + let res = Res::Def(DefKind::Fn, self.r.local_def_id(item.id).to_def_id()); self.r.define(parent, ident, ValueNS, (res, vis, sp, expansion)); // Functions introducing procedural macros reserve a slot @@ -748,15 +746,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // These items live in the type namespace. ItemKind::TyAlias(..) => { - let res = Res::Def( - DefKind::TyAlias, - self.r.definitions.local_def_id(item.id).to_def_id(), - ); + let res = Res::Def(DefKind::TyAlias, self.r.local_def_id(item.id).to_def_id()); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); } ItemKind::Enum(_, _) => { - let def_id = self.r.definitions.local_def_id(item.id).to_def_id(); + let def_id = self.r.local_def_id(item.id).to_def_id(); self.r.variant_vis.insert(def_id, vis); let module_kind = ModuleKind::Def(DefKind::Enum, def_id, ident.name); let module = self.r.new_module( @@ -771,17 +766,14 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } ItemKind::TraitAlias(..) => { - let res = Res::Def( - DefKind::TraitAlias, - self.r.definitions.local_def_id(item.id).to_def_id(), - ); + let res = Res::Def(DefKind::TraitAlias, self.r.local_def_id(item.id).to_def_id()); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); } // These items live in both the type and value namespaces. ItemKind::Struct(ref vdata, _) => { // Define a name in the type namespace. - let def_id = self.r.definitions.local_def_id(item.id).to_def_id(); + let def_id = self.r.local_def_id(item.id).to_def_id(); let res = Res::Def(DefKind::Struct, def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); @@ -814,7 +806,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } let ctor_res = Res::Def( DefKind::Ctor(CtorOf::Struct, CtorKind::from_ast(vdata)), - self.r.definitions.local_def_id(ctor_node_id).to_def_id(), + self.r.local_def_id(ctor_node_id).to_def_id(), ); self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, sp, expansion)); self.r.struct_constructors.insert(def_id, (ctor_res, ctor_vis)); @@ -822,7 +814,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } ItemKind::Union(ref vdata, _) => { - let def_id = self.r.definitions.local_def_id(item.id).to_def_id(); + let def_id = self.r.local_def_id(item.id).to_def_id(); let res = Res::Def(DefKind::Union, def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); @@ -831,7 +823,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } ItemKind::Trait(..) => { - let def_id = self.r.definitions.local_def_id(item.id).to_def_id(); + let def_id = self.r.local_def_id(item.id).to_def_id(); // Add all the items within to a new module. let module_kind = ModuleKind::Def(DefKind::Trait, def_id, ident.name); @@ -856,18 +848,15 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { /// Constructs the reduced graph for one foreign item. fn build_reduced_graph_for_foreign_item(&mut self, item: &ForeignItem) { let (res, ns) = match item.kind { - ForeignItemKind::Fn(..) => ( - Res::Def(DefKind::Fn, self.r.definitions.local_def_id(item.id).to_def_id()), - ValueNS, - ), - ForeignItemKind::Static(..) => ( - Res::Def(DefKind::Static, self.r.definitions.local_def_id(item.id).to_def_id()), - ValueNS, - ), - ForeignItemKind::TyAlias(..) => ( - Res::Def(DefKind::ForeignTy, self.r.definitions.local_def_id(item.id).to_def_id()), - TypeNS, - ), + ForeignItemKind::Fn(..) => { + (Res::Def(DefKind::Fn, self.r.local_def_id(item.id).to_def_id()), ValueNS) + } + ForeignItemKind::Static(..) => { + (Res::Def(DefKind::Static, self.r.local_def_id(item.id).to_def_id()), ValueNS) + } + ForeignItemKind::TyAlias(..) => { + (Res::Def(DefKind::ForeignTy, self.r.local_def_id(item.id).to_def_id()), TypeNS) + } ForeignItemKind::MacCall(_) => unreachable!(), }; let parent = self.parent_scope.module; @@ -1170,7 +1159,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { fn define_macro(&mut self, item: &ast::Item) -> MacroRulesScope<'a> { let parent_scope = self.parent_scope; let expansion = parent_scope.expansion; - let def_id = self.r.definitions.local_def_id(item.id); + let def_id = self.r.local_def_id(item.id); let (ext, ident, span, macro_rules) = match &item.kind { ItemKind::MacroDef(def) => { let ext = Lrc::new(self.r.compile_macro(item, self.r.session.edition())); @@ -1315,7 +1304,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { } // Add the item to the trait info. - let item_def_id = self.r.definitions.local_def_id(item.id).to_def_id(); + let item_def_id = self.r.local_def_id(item.id).to_def_id(); let (res, ns) = match item.kind { AssocItemKind::Const(..) => (Res::Def(DefKind::AssocConst, item_def_id), ValueNS), AssocItemKind::Fn(_, ref sig, _, _) => { @@ -1417,7 +1406,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { let ident = variant.ident; // Define a name in the type namespace. - let def_id = self.r.definitions.local_def_id(variant.id).to_def_id(); + let def_id = self.r.local_def_id(variant.id).to_def_id(); let res = Res::Def(DefKind::Variant, def_id); self.r.define(parent, ident, TypeNS, (res, vis, variant.span, expn_id)); @@ -1435,7 +1424,7 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { // It's ok to use the variant's id as a ctor id since an // error will be reported on any use of such resolution anyway. let ctor_node_id = variant.data.ctor_id().unwrap_or(variant.id); - let ctor_def_id = self.r.definitions.local_def_id(ctor_node_id).to_def_id(); + let ctor_def_id = self.r.local_def_id(ctor_node_id).to_def_id(); let ctor_kind = CtorKind::from_ast(&variant.data); let ctor_res = Res::Def(DefKind::Ctor(CtorOf::Variant, ctor_kind), ctor_def_id); self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, variant.span, expn_id)); diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs index cc0e97aeb1430..0ca01a384e73e 100644 --- a/src/librustc_resolve/check_unused.rs +++ b/src/librustc_resolve/check_unused.rs @@ -29,6 +29,7 @@ use crate::Resolver; use rustc_ast::ast; use rustc_ast::node_id::NodeMap; use rustc_ast::visit::{self, Visitor}; +use rustc_ast_lowering::Resolver as ResolverAstLowering; use rustc_data_structures::fx::FxHashSet; use rustc_errors::pluralize; use rustc_middle::ty; @@ -64,7 +65,7 @@ impl<'a, 'b> UnusedImportCheckVisitor<'a, 'b> { fn check_import(&mut self, id: ast::NodeId) { let mut used = false; self.r.per_ns(|this, ns| used |= this.used_imports.contains(&(id, ns))); - let def_id = self.r.definitions.local_def_id(id); + let def_id = self.r.local_def_id(id); if !used { if self.r.maybe_unused_trait_imports.contains(&def_id) { // Check later. @@ -246,7 +247,7 @@ impl Resolver<'_> { } } ImportKind::ExternCrate { .. } => { - let def_id = self.definitions.local_def_id(import.id); + let def_id = self.local_def_id(import.id); self.maybe_unused_extern_crates.push((def_id, import.span)); } ImportKind::MacroUse => { diff --git a/src/librustc_resolve/def_collector.rs b/src/librustc_resolve/def_collector.rs index 71cedb208fcf2..7bf039ea8a53e 100644 --- a/src/librustc_resolve/def_collector.rs +++ b/src/librustc_resolve/def_collector.rs @@ -1,8 +1,10 @@ +use crate::Resolver; use log::debug; use rustc_ast::ast::*; use rustc_ast::token::{self, Token}; use rustc_ast::visit::{self, FnKind}; use rustc_ast::walk_list; +use rustc_ast_lowering::Resolver as ResolverAstLowering; use rustc_expand::expand::AstFragment; use rustc_hir::def_id::LocalDefId; use rustc_hir::definitions::*; @@ -11,26 +13,26 @@ use rustc_span::symbol::{kw, sym}; use rustc_span::Span; crate fn collect_definitions( - definitions: &mut Definitions, + resolver: &mut Resolver<'_>, fragment: &AstFragment, expansion: ExpnId, ) { - let parent_def = definitions.invocation_parent(expansion); - fragment.visit_with(&mut DefCollector { definitions, parent_def, expansion }); + let parent_def = resolver.invocation_parents[&expansion]; + fragment.visit_with(&mut DefCollector { resolver, parent_def, expansion }); } /// Creates `DefId`s for nodes in the AST. -struct DefCollector<'a> { - definitions: &'a mut Definitions, +struct DefCollector<'a, 'b> { + resolver: &'a mut Resolver<'b>, parent_def: LocalDefId, expansion: ExpnId, } -impl<'a> DefCollector<'a> { +impl<'a, 'b> DefCollector<'a, 'b> { fn create_def(&mut self, node_id: NodeId, data: DefPathData, span: Span) -> LocalDefId { let parent_def = self.parent_def; debug!("create_def(node_id={:?}, data={:?}, parent_def={:?})", node_id, data, parent_def); - self.definitions.create_def_with_parent(parent_def, node_id, data, self.expansion, span) + self.resolver.create_def_with_parent(parent_def, node_id, data, self.expansion, span) } fn with_parent(&mut self, parent_def: LocalDefId, f: F) { @@ -43,12 +45,13 @@ impl<'a> DefCollector<'a> { let index = |this: &Self| { index.unwrap_or_else(|| { let node_id = NodeId::placeholder_from_expn_id(this.expansion); - this.definitions.placeholder_field_index(node_id) + this.resolver.placeholder_field_indices[&node_id] }) }; if field.is_placeholder { - self.definitions.set_placeholder_field_index(field.id, index(self)); + let old_index = self.resolver.placeholder_field_indices.insert(field.id, index(self)); + assert!(old_index.is_none(), "placeholder field index is reset for a node ID"); self.visit_macro_invoc(field.id); } else { let name = field.ident.map_or_else(|| sym::integer(index(self)), |ident| ident.name); @@ -58,11 +61,13 @@ impl<'a> DefCollector<'a> { } fn visit_macro_invoc(&mut self, id: NodeId) { - self.definitions.set_invocation_parent(id.placeholder_to_expn_id(), self.parent_def); + let old_parent = + self.resolver.invocation_parents.insert(id.placeholder_to_expn_id(), self.parent_def); + assert!(old_parent.is_none(), "parent `LocalDefId` is reset for an invocation"); } } -impl<'a> visit::Visitor<'a> for DefCollector<'a> { +impl<'a, 'b> visit::Visitor<'a> for DefCollector<'a, 'b> { fn visit_item(&mut self, i: &'a Item) { debug!("visit_item: {:?}", i); diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index bd2ce5a72e8d9..35d71a38bbe0d 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -107,7 +107,7 @@ impl<'a> Resolver<'a> { match outer_res { Res::SelfTy(maybe_trait_defid, maybe_impl_defid) => { if let Some(impl_span) = - maybe_impl_defid.and_then(|def_id| self.definitions.opt_span(def_id)) + maybe_impl_defid.and_then(|def_id| self.opt_span(def_id)) { err.span_label( reduce_impl_span_to_impl_keyword(sm, impl_span), @@ -126,12 +126,12 @@ impl<'a> Resolver<'a> { return err; } Res::Def(DefKind::TyParam, def_id) => { - if let Some(span) = self.definitions.opt_span(def_id) { + if let Some(span) = self.opt_span(def_id) { err.span_label(span, "type parameter from outer function"); } } Res::Def(DefKind::ConstParam, def_id) => { - if let Some(span) = self.definitions.opt_span(def_id) { + if let Some(span) = self.opt_span(def_id) { err.span_label(span, "const parameter from outer function"); } } @@ -825,7 +825,7 @@ impl<'a> Resolver<'a> { Applicability::MaybeIncorrect, ); let def_span = suggestion.res.opt_def_id().and_then(|def_id| match def_id.krate { - LOCAL_CRATE => self.definitions.opt_span(def_id), + LOCAL_CRATE => self.opt_span(def_id), _ => Some( self.session .source_map() diff --git a/src/librustc_resolve/imports.rs b/src/librustc_resolve/imports.rs index 74a8b7e2f556d..8a6541b399e38 100644 --- a/src/librustc_resolve/imports.rs +++ b/src/librustc_resolve/imports.rs @@ -12,6 +12,7 @@ use crate::{NameBinding, NameBindingKind, PathResult, PrivacyError, ToNameBindin use rustc_ast::ast::NodeId; use rustc_ast::unwrap_or; use rustc_ast::util::lev_distance::find_best_match_for_name; +use rustc_ast_lowering::Resolver as ResolverAstLowering; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::ptr_key::PtrKey; use rustc_errors::{pluralize, struct_span_err, Applicability}; @@ -1393,7 +1394,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { let is_good_import = binding.is_import() && !binding.is_ambiguity() && !ident.span.from_expansion(); if is_good_import || binding.is_macro_def() { - let res = binding.res().map_id(|id| this.definitions.local_def_id(id)); + let res = binding.res().map_id(|id| this.local_def_id(id)); if res != def::Res::Err { reexports.push(Export { ident, res, span: binding.span, vis: binding.vis }); } diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 61f20df8cc6c0..6f769c3c59cae 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -16,6 +16,7 @@ use rustc_ast::ptr::P; use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_ast::visit::{self, AssocCtxt, FnCtxt, FnKind, Visitor}; use rustc_ast::{unwrap_or, walk_list}; +use rustc_ast_lowering::Resolver as ResolverAstLowering; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::DiagnosticId; use rustc_hir::def::Namespace::{self, *}; @@ -707,7 +708,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } fn with_scope(&mut self, id: NodeId, f: impl FnOnce(&mut Self) -> T) -> T { - let id = self.r.definitions.local_def_id(id); + let id = self.r.local_def_id(id); let module = self.r.module_map.get(&id).cloned(); // clones a reference if let Some(module) = module { // Move down in the graph. @@ -759,7 +760,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { debug!("resolve_adt"); self.with_current_self_item(item, |this| { this.with_generic_param_rib(generics, ItemRibKind(HasGenericParams::Yes), |this| { - let item_def_id = this.r.definitions.local_def_id(item.id).to_def_id(); + let item_def_id = this.r.local_def_id(item.id).to_def_id(); this.with_self_rib(Res::SelfTy(None, Some(item_def_id)), |this| { visit::walk_item(this, item); }); @@ -839,7 +840,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { ItemKind::Trait(.., ref generics, ref bounds, ref trait_items) => { // Create a new rib for the trait-wide type parameters. self.with_generic_param_rib(generics, ItemRibKind(HasGenericParams::Yes), |this| { - let local_def_id = this.r.definitions.local_def_id(item.id).to_def_id(); + let local_def_id = this.r.local_def_id(item.id).to_def_id(); this.with_self_rib(Res::SelfTy(Some(local_def_id), None), |this| { this.visit_generics(generics); walk_list!(this, visit_param_bound, bounds); @@ -880,7 +881,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { ItemKind::TraitAlias(ref generics, ref bounds) => { // Create a new rib for the trait-wide type parameters. self.with_generic_param_rib(generics, ItemRibKind(HasGenericParams::Yes), |this| { - let local_def_id = this.r.definitions.local_def_id(item.id).to_def_id(); + let local_def_id = this.r.local_def_id(item.id).to_def_id(); this.with_self_rib(Res::SelfTy(Some(local_def_id), None), |this| { this.visit_generics(generics); walk_list!(this, visit_param_bound, bounds); @@ -961,7 +962,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { seen_bindings.entry(ident).or_insert(param.ident.span); // Plain insert (no renaming). - let res = Res::Def(def_kind, self.r.definitions.local_def_id(param.id).to_def_id()); + let res = Res::Def(def_kind, self.r.local_def_id(param.id).to_def_id()); match param.kind { GenericParamKind::Type { .. } => { @@ -1111,7 +1112,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { this.with_self_rib(Res::SelfTy(None, None), |this| { // Resolve the trait reference, if necessary. this.with_optional_trait_ref(opt_trait_reference.as_ref(), |this, trait_id| { - let item_def_id = this.r.definitions.local_def_id(item_id).to_def_id(); + let item_def_id = this.r.local_def_id(item_id).to_def_id(); this.with_self_rib(Res::SelfTy(trait_id, Some(item_def_id)), |this| { if let Some(trait_ref) = opt_trait_reference.as_ref() { // Resolve type arguments in the trait path. @@ -2002,7 +2003,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { if let StmtKind::Item(ref item) = stmt.kind { if let ItemKind::MacroDef(..) = item.kind { num_macro_definition_ribs += 1; - let res = self.r.definitions.local_def_id(item.id).to_def_id(); + let res = self.r.local_def_id(item.id).to_def_id(); self.ribs[ValueNS].push(Rib::new(MacroDefinition(res))); self.label_ribs.push(Rib::new(MacroDefinition(res))); } @@ -2296,7 +2297,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { ) -> SmallVec<[LocalDefId; 1]> { let mut import_ids = smallvec![]; while let NameBindingKind::Import { import, binding, .. } = kind { - let id = self.r.definitions.local_def_id(import.id); + let id = self.r.local_def_id(import.id); self.r.maybe_unused_trait_imports.insert(id); self.r.add_to_glob_map(&import, trait_name); import_ids.push(id); diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 05ef0aa0bb689..79c544ec6cc94 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -512,7 +512,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { _ => {} } if !suggested { - if let Some(span) = self.r.definitions.opt_span(def_id) { + if let Some(span) = self.r.opt_span(def_id) { err.span_label(span, &format!("`{}` defined here", path_str)); } err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", path_str)); @@ -536,7 +536,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { if nightly_options::is_nightly_build() { let msg = "you might have meant to use `#![feature(trait_alias)]` instead of a \ `type` alias"; - if let Some(span) = self.r.definitions.opt_span(def_id) { + if let Some(span) = self.r.opt_span(def_id) { err.span_help(span, msg); } else { err.help(msg); @@ -593,7 +593,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { bad_struct_syntax_suggestion(def_id); } (Res::Def(DefKind::Ctor(_, CtorKind::Fn), def_id), _) if ns == ValueNS => { - if let Some(span) = self.r.definitions.opt_span(def_id) { + if let Some(span) = self.r.opt_span(def_id) { err.span_label(span, &format!("`{}` defined here", path_str)); } err.span_label(span, format!("did you mean `{}( /* fields */ )`?", path_str)); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 91bd155614178..0aca5c4905df9 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -27,6 +27,7 @@ use rustc_ast::attr; use rustc_ast::node_id::NodeMap; use rustc_ast::unwrap_or; use rustc_ast::visit::{self, Visitor}; +use rustc_ast_lowering::Resolver as ResolverAstLowering; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; use rustc_data_structures::ptr_key::PtrKey; @@ -36,9 +37,10 @@ use rustc_expand::base::SyntaxExtension; use rustc_hir::def::Namespace::*; use rustc_hir::def::{self, CtorOf, DefKind, NonMacroAttrKind, PartialRes}; use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, CRATE_DEF_INDEX}; -use rustc_hir::definitions::{DefKey, Definitions}; +use rustc_hir::definitions::{DefKey, DefPathData, Definitions}; use rustc_hir::PrimTy::{self, Bool, Char, Float, Int, Str, Uint}; use rustc_hir::TraitCandidate; +use rustc_index::vec::IndexVec; use rustc_metadata::creader::{CStore, CrateLoader}; use rustc_middle::hir::exports::ExportMap; use rustc_middle::middle::cstore::{CrateStore, MetadataLoaderDyn}; @@ -256,31 +258,21 @@ impl<'a> From<&'a ast::PathSegment> for Segment { } } -struct UsePlacementFinder<'d> { - definitions: &'d Definitions, - target_module: LocalDefId, +struct UsePlacementFinder { + target_module: NodeId, span: Option, found_use: bool, } -impl<'d> UsePlacementFinder<'d> { - fn check( - definitions: &'d Definitions, - krate: &Crate, - target_module: DefId, - ) -> (Option, bool) { - if let Some(target_module) = target_module.as_local() { - let mut finder = - UsePlacementFinder { definitions, target_module, span: None, found_use: false }; - visit::walk_crate(&mut finder, krate); - (finder.span, finder.found_use) - } else { - (None, false) - } +impl UsePlacementFinder { + fn check(krate: &Crate, target_module: NodeId) -> (Option, bool) { + let mut finder = UsePlacementFinder { target_module, span: None, found_use: false }; + visit::walk_crate(&mut finder, krate); + (finder.span, finder.found_use) } } -impl<'tcx, 'd> Visitor<'tcx> for UsePlacementFinder<'d> { +impl<'tcx> Visitor<'tcx> for UsePlacementFinder { fn visit_mod( &mut self, module: &'tcx ast::Mod, @@ -291,7 +283,7 @@ impl<'tcx, 'd> Visitor<'tcx> for UsePlacementFinder<'d> { if self.span.is_some() { return; } - if self.definitions.local_def_id(node_id) != self.target_module { + if node_id != self.target_module { visit::walk_mod(self, module); return; } @@ -979,6 +971,17 @@ pub struct Resolver<'a> { lint_buffer: LintBuffer, next_node_id: NodeId, + + def_id_to_span: IndexVec, + + node_id_to_def_id: FxHashMap, + def_id_to_node_id: IndexVec, + + /// Indices of unnamed struct or variant fields with unresolved attributes. + placeholder_field_indices: FxHashMap, + /// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId` + /// we know what parent node that fragment should be attached to thanks to this table. + invocation_parents: FxHashMap, } /// Nothing really interesting here; it just provides memory for the rest of the crate. @@ -1042,7 +1045,7 @@ impl<'a, 'b> DefIdTree for &'a Resolver<'b> { /// This interface is used through the AST→HIR step, to embed full paths into the HIR. After that /// the resolver is no longer needed as all the relevant information is inline. -impl rustc_ast_lowering::Resolver for Resolver<'_> { +impl ResolverAstLowering for Resolver<'_> { fn def_key(&mut self, id: DefId) -> DefKey { if let Some(id) = id.as_local() { self.definitions().def_key(id) @@ -1113,6 +1116,47 @@ impl rustc_ast_lowering::Resolver for Resolver<'_> { fn trait_map(&self) -> &NodeMap> { &self.trait_map } + + fn opt_local_def_id(&self, node: NodeId) -> Option { + self.node_id_to_def_id.get(&node).copied() + } + + fn local_def_id(&self, node: NodeId) -> LocalDefId { + self.opt_local_def_id(node).unwrap_or_else(|| panic!("no entry for node id: `{:?}`", node)) + } + + /// Adds a definition with a parent definition. + fn create_def_with_parent( + &mut self, + parent: LocalDefId, + node_id: ast::NodeId, + data: DefPathData, + expn_id: ExpnId, + span: Span, + ) -> LocalDefId { + assert!( + !self.node_id_to_def_id.contains_key(&node_id), + "adding a def'n for node-id {:?} and data {:?} but a previous def'n exists: {:?}", + node_id, + data, + self.definitions.def_key(self.node_id_to_def_id[&node_id]), + ); + + let def_id = self.definitions.create_def_with_parent(parent, data, expn_id); + + assert_eq!(self.def_id_to_span.push(span), def_id); + + // Some things for which we allocate `LocalDefId`s don't correspond to + // anything in the AST, so they don't have a `NodeId`. For these cases + // we don't need a mapping from `NodeId` to `LocalDefId`. + if node_id != ast::DUMMY_NODE_ID { + debug!("create_def_with_parent: def_id_to_node_id[{:?}] <-> {:?}", def_id, node_id); + self.node_id_to_def_id.insert(node_id, def_id); + } + assert_eq!(self.def_id_to_node_id.push(node_id), def_id); + + def_id + } } impl<'a> Resolver<'a> { @@ -1144,7 +1188,17 @@ impl<'a> Resolver<'a> { module_map.insert(LocalDefId { local_def_index: CRATE_DEF_INDEX }, graph_root); let mut definitions = Definitions::default(); - definitions.create_root_def(crate_name, session.local_crate_disambiguator()); + let root = definitions.create_root_def(crate_name, session.local_crate_disambiguator()); + + let mut def_id_to_span = IndexVec::default(); + assert_eq!(def_id_to_span.push(rustc_span::DUMMY_SP), root); + let mut def_id_to_node_id = IndexVec::default(); + assert_eq!(def_id_to_node_id.push(CRATE_NODE_ID), root); + let mut node_id_to_def_id = FxHashMap::default(); + node_id_to_def_id.insert(CRATE_NODE_ID, root); + + let mut invocation_parents = FxHashMap::default(); + invocation_parents.insert(ExpnId::root(), root); let mut extern_prelude: FxHashMap> = session .opts @@ -1263,6 +1317,11 @@ impl<'a> Resolver<'a> { variant_vis: Default::default(), lint_buffer: LintBuffer::default(), next_node_id: NodeId::from_u32(1), + def_id_to_span, + node_id_to_def_id, + def_id_to_node_id, + placeholder_field_indices: Default::default(), + invocation_parents, } } @@ -1457,7 +1516,7 @@ impl<'a> Resolver<'a> { #[inline] fn add_to_glob_map(&mut self, import: &Import<'_>, ident: Ident) { if import.is_glob() { - let def_id = self.definitions.local_def_id(import.id); + let def_id = self.local_def_id(import.id); self.glob_map.entry(def_id).or_default().insert(ident.name); } } @@ -2538,7 +2597,11 @@ impl<'a> Resolver<'a> { for UseError { mut err, candidates, def_id, instead, suggestion } in self.use_injections.drain(..) { - let (span, found_use) = UsePlacementFinder::check(&self.definitions, krate, def_id); + let (span, found_use) = if let Some(def_id) = def_id.as_local() { + UsePlacementFinder::check(krate, self.def_id_to_node_id[def_id]) + } else { + (None, false) + }; if !candidates.is_empty() { diagnostics::show_candidates(&mut err, span, &candidates, instead, found_use); } else if let Some((span, msg, sugg, appl)) = suggestion { @@ -2934,6 +2997,12 @@ impl<'a> Resolver<'a> { pub fn all_macros(&self) -> &FxHashMap { &self.all_macros } + + /// Retrieves the span of the given `DefId` if `DefId` is in the local crate. + #[inline] + pub fn opt_span(&self, def_id: DefId) -> Option { + if let Some(def_id) = def_id.as_local() { Some(self.def_id_to_span[def_id]) } else { None } + } } fn names_to_string(names: &[Symbol]) -> String { diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 1b49722355e54..398b0e92d9d8c 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -7,6 +7,7 @@ use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy}; use crate::{CrateLint, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Weak}; use crate::{ModuleKind, ModuleOrUniformRoot, NameBinding, PathResult, Segment, ToNameBinding}; use rustc_ast::ast::{self, NodeId}; +use rustc_ast_lowering::Resolver as ResolverAstLowering; use rustc_ast_pretty::pprust; use rustc_attr::{self as attr, StabilityLevel}; use rustc_data_structures::fx::FxHashSet; @@ -190,7 +191,7 @@ impl<'a> base::Resolver for Resolver<'a> { ))); let parent_scope = if let Some(module_id) = parent_module_id { - let parent_def_id = self.definitions.local_def_id(module_id); + let parent_def_id = self.local_def_id(module_id); self.definitions.add_parent_module_of_macro_def(expn_id, parent_def_id.to_def_id()); self.module_map[&parent_def_id] } else { @@ -340,7 +341,9 @@ impl<'a> base::Resolver for Resolver<'a> { } fn lint_node_id(&mut self, expn_id: ExpnId) -> NodeId { - self.definitions.lint_node_id(expn_id) + self.invocation_parents + .get(&expn_id) + .map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[*id]) } fn has_derive_copy(&self, expn_id: ExpnId) -> bool { From 1d3f49f53654f12cf9f3501666c0dfd1afe5cf8b Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 21 Jun 2020 15:49:38 +0100 Subject: [PATCH 10/24] Always create a root definition when creating a new `Definitions` object. --- src/librustc_ast_lowering/lib.rs | 14 +++++----- src/librustc_hir/definitions.rs | 40 +++++++++++++++------------ src/librustc_interface/passes.rs | 5 +++- src/librustc_resolve/def_collector.rs | 2 +- src/librustc_resolve/lib.rs | 10 +++---- 5 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index ac8a229da43db..39b14ac458832 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -210,7 +210,7 @@ pub trait Resolver { fn local_def_id(&self, node: NodeId) -> LocalDefId; - fn create_def_with_parent( + fn create_def( &mut self, parent: LocalDefId, node_id: ast::NodeId, @@ -449,7 +449,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { match tree.kind { UseTreeKind::Simple(_, id1, id2) => { for &id in &[id1, id2] { - self.lctx.resolver.create_def_with_parent( + self.lctx.resolver.create_def( owner, id, DefPathData::Misc, @@ -696,7 +696,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { *local_id_counter += 1; let owner = this.resolver.opt_local_def_id(owner).expect( - "you forgot to call `create_def_with_parent` or are lowering node-IDs \ + "you forgot to call `create_def` or are lowering node-IDs \ that do not belong to the current owner", ); @@ -824,7 +824,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { }; // Add a definition for the in-band lifetime def. - self.resolver.create_def_with_parent( + self.resolver.create_def( parent_def_id, node_id, DefPathData::LifetimeNs(str_name), @@ -1112,7 +1112,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let impl_trait_node_id = self.resolver.next_node_id(); let parent_def_id = self.current_hir_id_owner.last().unwrap().0; - self.resolver.create_def_with_parent( + self.resolver.create_def( parent_def_id, impl_trait_node_id, DefPathData::ImplTrait, @@ -1178,7 +1178,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let node_id = self.resolver.next_node_id(); // Add a definition for the in-band const def. - self.resolver.create_def_with_parent( + self.resolver.create_def( parent_def_id, node_id, DefPathData::AnonConst, @@ -1644,7 +1644,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let def_node_id = self.context.resolver.next_node_id(); let hir_id = self.context.lower_node_id_with_owner(def_node_id, self.opaque_ty_id); - self.context.resolver.create_def_with_parent( + self.context.resolver.create_def( self.parent, def_node_id, DefPathData::LifetimeNs(name.ident().name), diff --git a/src/librustc_hir/definitions.rs b/src/librustc_hir/definitions.rs index c34607f8a94c6..5e072d37eaad4 100644 --- a/src/librustc_hir/definitions.rs +++ b/src/librustc_hir/definitions.rs @@ -71,9 +71,9 @@ impl DefPathTable { } /// The definition table containing node definitions. -/// It holds the `DefPathTable` for local `DefId`s/`DefPath`s and it also stores a -/// mapping from `NodeId`s to local `DefId`s. -#[derive(Clone, Default)] +/// It holds the `DefPathTable` for `LocalDefId`s/`DefPath`s. +/// It also stores mappings to convert `LocalDefId`s to/from `HirId`s. +#[derive(Clone)] pub struct Definitions { table: DefPathTable, @@ -328,11 +328,7 @@ impl Definitions { } /// Adds a root definition (no parent) and a few other reserved definitions. - pub fn create_root_def( - &mut self, - crate_name: &str, - crate_disambiguator: CrateDisambiguator, - ) -> LocalDefId { + pub fn new(crate_name: &str, crate_disambiguator: CrateDisambiguator) -> Definitions { let key = DefKey { parent: None, disambiguated_data: DisambiguatedDefPathData { @@ -344,24 +340,34 @@ impl Definitions { let parent_hash = DefKey::root_parent_stable_hash(crate_name, crate_disambiguator); let def_path_hash = key.compute_stable_hash(parent_hash); - // Create the definition. - let root = LocalDefId { local_def_index: self.table.allocate(key, def_path_hash) }; + // Create the root definition. + let mut table = DefPathTable::default(); + let root = LocalDefId { local_def_index: table.allocate(key, def_path_hash) }; assert_eq!(root.local_def_index, CRATE_DEF_INDEX); - root + Definitions { + table, + def_id_to_hir_id: Default::default(), + hir_id_to_def_id: Default::default(), + expansions_that_defined: Default::default(), + next_disambiguator: Default::default(), + parent_modules_of_macro_defs: Default::default(), + } + } + + /// Retrieves the root definition. + pub fn get_root_def(&self) -> LocalDefId { + LocalDefId { local_def_index: CRATE_DEF_INDEX } } /// Adds a definition with a parent definition. - pub fn create_def_with_parent( + pub fn create_def( &mut self, parent: LocalDefId, data: DefPathData, expn_id: ExpnId, ) -> LocalDefId { - debug!( - "create_def_with_parent(parent={:?}, data={:?}, expn_id={:?})", - parent, data, expn_id - ); + debug!("create_def(parent={:?}, data={:?}, expn_id={:?})", parent, data, expn_id); // The root node must be created with `create_root_def()`. assert!(data != DefPathData::CrateRoot); @@ -382,7 +388,7 @@ impl Definitions { let parent_hash = self.table.def_path_hash(parent.local_def_index); let def_path_hash = key.compute_stable_hash(parent_hash); - debug!("create_def_with_parent: after disambiguation, key = {:?}", key); + debug!("create_def: after disambiguation, key = {:?}", key); // Create the definition. let def_id = LocalDefId { local_def_index: self.table.allocate(key, def_path_hash) }; diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 1ed9bc3f1f509..ea3b19ab4a75d 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -734,7 +734,10 @@ pub fn create_global_ctxt<'tcx>( arena: &'tcx WorkerLocal>, ) -> QueryContext<'tcx> { let sess = &compiler.session(); - let defs: &'tcx Definitions = arena.alloc(mem::take(&mut resolver_outputs.definitions)); + let defs: &'tcx Definitions = arena.alloc(mem::replace( + &mut resolver_outputs.definitions, + Definitions::new(crate_name, sess.local_crate_disambiguator()), + )); let query_result_on_disk_cache = rustc_incremental::load_query_result_cache(sess); diff --git a/src/librustc_resolve/def_collector.rs b/src/librustc_resolve/def_collector.rs index 7bf039ea8a53e..f1063f42c91ec 100644 --- a/src/librustc_resolve/def_collector.rs +++ b/src/librustc_resolve/def_collector.rs @@ -32,7 +32,7 @@ impl<'a, 'b> DefCollector<'a, 'b> { fn create_def(&mut self, node_id: NodeId, data: DefPathData, span: Span) -> LocalDefId { let parent_def = self.parent_def; debug!("create_def(node_id={:?}, data={:?}, parent_def={:?})", node_id, data, parent_def); - self.resolver.create_def_with_parent(parent_def, node_id, data, self.expansion, span) + self.resolver.create_def(parent_def, node_id, data, self.expansion, span) } fn with_parent(&mut self, parent_def: LocalDefId, f: F) { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 0aca5c4905df9..6005f009cc3d5 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1126,7 +1126,7 @@ impl ResolverAstLowering for Resolver<'_> { } /// Adds a definition with a parent definition. - fn create_def_with_parent( + fn create_def( &mut self, parent: LocalDefId, node_id: ast::NodeId, @@ -1142,7 +1142,7 @@ impl ResolverAstLowering for Resolver<'_> { self.definitions.def_key(self.node_id_to_def_id[&node_id]), ); - let def_id = self.definitions.create_def_with_parent(parent, data, expn_id); + let def_id = self.definitions.create_def(parent, data, expn_id); assert_eq!(self.def_id_to_span.push(span), def_id); @@ -1150,7 +1150,7 @@ impl ResolverAstLowering for Resolver<'_> { // anything in the AST, so they don't have a `NodeId`. For these cases // we don't need a mapping from `NodeId` to `LocalDefId`. if node_id != ast::DUMMY_NODE_ID { - debug!("create_def_with_parent: def_id_to_node_id[{:?}] <-> {:?}", def_id, node_id); + debug!("create_def: def_id_to_node_id[{:?}] <-> {:?}", def_id, node_id); self.node_id_to_def_id.insert(node_id, def_id); } assert_eq!(self.def_id_to_node_id.push(node_id), def_id); @@ -1187,8 +1187,8 @@ impl<'a> Resolver<'a> { let mut module_map = FxHashMap::default(); module_map.insert(LocalDefId { local_def_index: CRATE_DEF_INDEX }, graph_root); - let mut definitions = Definitions::default(); - let root = definitions.create_root_def(crate_name, session.local_crate_disambiguator()); + let definitions = Definitions::new(crate_name, session.local_crate_disambiguator()); + let root = definitions.get_root_def(); let mut def_id_to_span = IndexVec::default(); assert_eq!(def_id_to_span.push(rustc_span::DUMMY_SP), root); From bd4f6f0b7d88baa9a5ecb18a2a700978ddcd58ff Mon Sep 17 00:00:00 2001 From: marmeladema Date: Sun, 21 Jun 2020 23:49:06 +0100 Subject: [PATCH 11/24] Move `next_disambiguator` to `Resolver` --- src/librustc_hir/definitions.rs | 12 ++---------- src/librustc_resolve/lib.rs | 14 +++++++++++++- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/librustc_hir/definitions.rs b/src/librustc_hir/definitions.rs index 5e072d37eaad4..79b7068273932 100644 --- a/src/librustc_hir/definitions.rs +++ b/src/librustc_hir/definitions.rs @@ -87,7 +87,6 @@ pub struct Definitions { parent_modules_of_macro_defs: FxHashMap, /// Item with a given `LocalDefId` was defined during macro expansion with ID `ExpnId`. expansions_that_defined: FxHashMap, - next_disambiguator: FxHashMap<(LocalDefId, DefPathData), u32>, } /// A unique identifier that we can use to lookup a definition @@ -350,7 +349,6 @@ impl Definitions { def_id_to_hir_id: Default::default(), hir_id_to_def_id: Default::default(), expansions_that_defined: Default::default(), - next_disambiguator: Default::default(), parent_modules_of_macro_defs: Default::default(), } } @@ -366,20 +364,14 @@ impl Definitions { parent: LocalDefId, data: DefPathData, expn_id: ExpnId, + mut next_disambiguator: impl FnMut(LocalDefId, DefPathData) -> u32, ) -> LocalDefId { debug!("create_def(parent={:?}, data={:?}, expn_id={:?})", parent, data, expn_id); // The root node must be created with `create_root_def()`. assert!(data != DefPathData::CrateRoot); - // Find the next free disambiguator for this key. - let disambiguator = { - let next_disamb = self.next_disambiguator.entry((parent, data)).or_insert(0); - let disambiguator = *next_disamb; - *next_disamb = next_disamb.checked_add(1).expect("disambiguator overflow"); - disambiguator - }; - + let disambiguator = next_disambiguator(parent, data); let key = DefKey { parent: Some(parent.local_def_index), disambiguated_data: DisambiguatedDefPathData { data, disambiguator }, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 6005f009cc3d5..ce068b8ac69a4 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -982,6 +982,8 @@ pub struct Resolver<'a> { /// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId` /// we know what parent node that fragment should be attached to thanks to this table. invocation_parents: FxHashMap, + + next_disambiguator: FxHashMap<(LocalDefId, DefPathData), u32>, } /// Nothing really interesting here; it just provides memory for the rest of the crate. @@ -1142,7 +1144,16 @@ impl ResolverAstLowering for Resolver<'_> { self.definitions.def_key(self.node_id_to_def_id[&node_id]), ); - let def_id = self.definitions.create_def(parent, data, expn_id); + // Find the next free disambiguator for this key. + let next_disambiguator = &mut self.next_disambiguator; + let next_disambiguator = |parent, data| { + let next_disamb = next_disambiguator.entry((parent, data)).or_insert(0); + let disambiguator = *next_disamb; + *next_disamb = next_disamb.checked_add(1).expect("disambiguator overflow"); + disambiguator + }; + + let def_id = self.definitions.create_def(parent, data, expn_id, next_disambiguator); assert_eq!(self.def_id_to_span.push(span), def_id); @@ -1322,6 +1333,7 @@ impl<'a> Resolver<'a> { def_id_to_node_id, placeholder_field_indices: Default::default(), invocation_parents, + next_disambiguator: Default::default(), } } From 933fe805777e46163c52371d81390ba721a37252 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Sun, 21 Jun 2020 23:48:39 -0700 Subject: [PATCH 12/24] num_counters to u32, after implementing TypeFoldable --- src/librustc_codegen_llvm/intrinsic.rs | 2 +- src/librustc_middle/mir/mod.rs | 2 +- src/librustc_middle/ty/structural_impls.rs | 1 + src/librustc_mir/transform/instrument_coverage.rs | 6 +++--- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 8c0ccde6b16bb..c6e7820a60ed4 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -146,7 +146,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); let hash = self.const_u64(coverage_data.hash); let index = args[0].immediate(); - let num_counters = self.const_u32(coverage_data.num_counters as u32); + let num_counters = self.const_u32(coverage_data.num_counters); debug!( "count_code_region to LLVM intrinsic instrprof.increment(fn_name={}, hash={:?}, num_counters={:?}, index={:?})", mangled_fn.name, hash, index, num_counters diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index a89a5ef3f8218..e88329db992f5 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -98,7 +98,7 @@ pub struct CoverageData { pub hash: u64, /// The total number of coverage region counters added to this MIR Body. - pub num_counters: usize, + pub num_counters: u32, } /// The lowered representation of a single function. diff --git a/src/librustc_middle/ty/structural_impls.rs b/src/librustc_middle/ty/structural_impls.rs index f04d31601ea5b..463e2c57d46c9 100644 --- a/src/librustc_middle/ty/structural_impls.rs +++ b/src/librustc_middle/ty/structural_impls.rs @@ -262,6 +262,7 @@ CloneTypeFoldableAndLiftImpls! { bool, usize, ::rustc_target::abi::VariantIdx, + u32, u64, String, crate::middle::region::Scope, diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 793ccbb081bed..a24d0acf4212c 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -21,7 +21,7 @@ pub struct InstrumentCoverage; struct Instrumentor<'tcx> { tcx: TyCtxt<'tcx>, - num_counters: usize, + num_counters: u32, } impl<'tcx> MirPass<'tcx> for InstrumentCoverage { @@ -55,12 +55,12 @@ impl<'tcx> Instrumentor<'tcx> { } fn next_counter(&mut self) -> u32 { - let next = self.num_counters as u32; + let next = self.num_counters; self.num_counters += 1; next } - fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) -> usize { + fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) -> u32 { // FIXME(richkadel): As a first step, counters are only injected at the top of each // function. The complete solution will inject counters at each conditional code branch. let top_of_function = START_BLOCK; From 932237b10184f0c70d045a5a8c5c1abad9d04725 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Mon, 22 Jun 2020 14:42:26 +0200 Subject: [PATCH 13/24] fix `intrinsics::needs_drop` docs --- src/libcore/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 9061145a695f8..e5bb18d21c489 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1291,7 +1291,7 @@ extern "rust-intrinsic" { /// implements `Copy`. /// /// If the actual type neither requires drop glue nor implements - /// `Copy`, then may return `true` or `false`. + /// `Copy`, then the return value of this function is unspecified. /// /// The stabilized version of this intrinsic is /// [`std::mem::needs_drop`](../../std/mem/fn.needs_drop.html). From 3ed96a6d638f8dc8aa081ca1ad82e61caa8930ca Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 21 Jun 2020 22:32:35 -0400 Subject: [PATCH 14/24] Point at the call spawn when overflow occurs during monomorphization This improves the output for issue #72577, but there's still more work to be done. Currently, an overflow error during monomorphization results in an error that points at the function we were unable to monomorphize. However, we don't point at the call that caused the monomorphization to happen. In the overflow occurs in a large recursive function, it may be difficult to determine where the issue is. This commit tracks and `Span` information during collection of `MonoItem`s, which is used when emitting an overflow error. `MonoItem` itself is unchanged, so this only affects `src/librustc_mir/monomorphize/collector.rs` --- src/librustc_mir/monomorphize/collector.rs | 137 ++++++++++-------- .../ui/infinite/infinite-instantiation.rs | 10 +- .../ui/infinite/infinite-instantiation.stderr | 11 +- src/test/ui/issues/issue-67552.rs | 2 +- src/test/ui/issues/issue-67552.stderr | 8 +- src/test/ui/issues/issue-8727.rs | 6 +- src/test/ui/issues/issue-8727.stderr | 6 + ...-38591-non-regular-dropck-recursion.stderr | 18 +++ src/test/ui/recursion/recursion.rs | 5 +- src/test/ui/recursion/recursion.stderr | 9 +- 10 files changed, 132 insertions(+), 80 deletions(-) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 36f3947d83017..84ef040741a38 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -178,7 +178,7 @@ use crate::monomorphize; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::{par_iter, MTLock, MTRef, ParallelIterator}; -use rustc_errors::ErrorReported; +use rustc_errors::{ErrorReported, FatalError}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId, LOCAL_CRATE}; use rustc_hir::itemlikevisit::ItemLikeVisitor; @@ -195,6 +195,7 @@ use rustc_middle::ty::print::obsolete::DefPathBasedNames; use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts}; use rustc_middle::ty::{self, GenericParamDefKind, Instance, Ty, TyCtxt, TypeFoldable}; use rustc_session::config::EntryFnType; +use rustc_span::source_map::{dummy_spanned, respan, Span, Spanned, DUMMY_SP}; use smallvec::SmallVec; use std::iter; @@ -294,7 +295,13 @@ pub fn collect_crate_mono_items( tcx.sess.time("monomorphization_collector_graph_walk", || { par_iter(roots).for_each(|root| { let mut recursion_depths = DefIdMap::default(); - collect_items_rec(tcx, root, visited, &mut recursion_depths, inlining_map); + collect_items_rec( + tcx, + dummy_spanned(root), + visited, + &mut recursion_depths, + inlining_map, + ); }); }); } @@ -323,29 +330,30 @@ fn collect_roots(tcx: TyCtxt<'_>, mode: MonoItemCollectionMode) -> Vec( tcx: TyCtxt<'tcx>, - starting_point: MonoItem<'tcx>, + starting_point: Spanned>, visited: MTRef<'_, MTLock>>>, recursion_depths: &mut DefIdMap, inlining_map: MTRef<'_, MTLock>>, ) { - if !visited.lock_mut().insert(starting_point) { + if !visited.lock_mut().insert(starting_point.node) { // We've been here already, no need to search again. return; } - debug!("BEGIN collect_items_rec({})", starting_point.to_string(tcx, true)); + debug!("BEGIN collect_items_rec({})", starting_point.node.to_string(tcx, true)); let mut neighbors = Vec::new(); let recursion_depth_reset; - match starting_point { + match starting_point.node { MonoItem::Static(def_id) => { let instance = Instance::mono(tcx, def_id); @@ -353,7 +361,7 @@ fn collect_items_rec<'tcx>( debug_assert!(should_monomorphize_locally(tcx, &instance)); let ty = instance.monomorphic_ty(tcx); - visit_drop_use(tcx, ty, true, &mut neighbors); + visit_drop_use(tcx, ty, true, starting_point.span, &mut neighbors); recursion_depth_reset = None; @@ -366,7 +374,8 @@ fn collect_items_rec<'tcx>( debug_assert!(should_monomorphize_locally(tcx, &instance)); // Keep track of the monomorphization recursion depth - recursion_depth_reset = Some(check_recursion_limit(tcx, instance, recursion_depths)); + recursion_depth_reset = + Some(check_recursion_limit(tcx, instance, starting_point.span, recursion_depths)); check_type_length_limit(tcx, instance); rustc_data_structures::stack::ensure_sufficient_stack(|| { @@ -378,7 +387,7 @@ fn collect_items_rec<'tcx>( } } - record_accesses(tcx, starting_point, &neighbors[..], inlining_map); + record_accesses(tcx, starting_point.node, neighbors.iter().map(|i| &i.node), inlining_map); for neighbour in neighbors { collect_items_rec(tcx, neighbour, visited, recursion_depths, inlining_map); @@ -388,13 +397,13 @@ fn collect_items_rec<'tcx>( recursion_depths.insert(def_id, depth); } - debug!("END collect_items_rec({})", starting_point.to_string(tcx, true)); + debug!("END collect_items_rec({})", starting_point.node.to_string(tcx, true)); } -fn record_accesses<'tcx>( +fn record_accesses<'a, 'tcx: 'a>( tcx: TyCtxt<'tcx>, caller: MonoItem<'tcx>, - callees: &[MonoItem<'tcx>], + callees: impl Iterator>, inlining_map: MTRef<'_, MTLock>>, ) { let is_inlining_candidate = |mono_item: &MonoItem<'tcx>| { @@ -405,7 +414,7 @@ fn record_accesses<'tcx>( // FIXME: Call `is_inlining_candidate` when pushing to `neighbors` in `collect_items_rec` // instead to avoid creating this `SmallVec`. let accesses: SmallVec<[_; 128]> = - callees.iter().map(|mono_item| (*mono_item, is_inlining_candidate(mono_item))).collect(); + callees.map(|mono_item| (*mono_item, is_inlining_candidate(mono_item))).collect(); inlining_map.lock_mut().record_accesses(caller, &accesses); } @@ -413,6 +422,7 @@ fn record_accesses<'tcx>( fn check_recursion_limit<'tcx>( tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, + span: Span, recursion_depths: &mut DefIdMap, ) -> (DefId, usize) { let def_id = instance.def_id(); @@ -432,12 +442,13 @@ fn check_recursion_limit<'tcx>( // infinite expansion. if !tcx.sess.recursion_limit().value_within_limit(adjusted_recursion_depth) { let error = format!("reached the recursion limit while instantiating `{}`", instance); - if let Some(def_id) = def_id.as_local() { - let hir_id = tcx.hir().as_local_hir_id(def_id); - tcx.sess.span_fatal(tcx.hir().span(hir_id), &error); - } else { - tcx.sess.fatal(&error); - } + let mut err = tcx.sess.struct_span_fatal(span, &error); + err.span_note( + tcx.def_span(def_id), + &format!("`{}` defined here", tcx.def_path_str(def_id)), + ); + err.emit(); + FatalError.raise(); } recursion_depths.insert(def_id, recursion_depth + 1); @@ -498,7 +509,7 @@ fn check_type_length_limit<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) { struct MirNeighborCollector<'a, 'tcx> { tcx: TyCtxt<'tcx>, body: &'a mir::Body<'tcx>, - output: &'a mut Vec>, + output: &'a mut Vec>>, instance: Instance<'tcx>, } @@ -520,6 +531,8 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) { debug!("visiting rvalue {:?}", *rvalue); + let span = self.body.source_info(location).span; + match *rvalue { // When doing an cast from a regular pointer to a fat pointer, we // have to instantiate all methods of the trait being cast to, so we @@ -542,6 +555,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { self.tcx, target_ty, source_ty, + span, self.output, ); } @@ -553,7 +567,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { ) => { let fn_ty = operand.ty(self.body, self.tcx); let fn_ty = self.monomorphize(fn_ty); - visit_fn_use(self.tcx, fn_ty, false, &mut self.output); + visit_fn_use(self.tcx, fn_ty, false, span, &mut self.output); } mir::Rvalue::Cast( mir::CastKind::Pointer(PointerCast::ClosureFnPointer(_)), @@ -571,7 +585,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { ty::ClosureKind::FnOnce, ); if should_monomorphize_locally(self.tcx, &instance) { - self.output.push(create_fn_mono_item(instance)); + self.output.push(create_fn_mono_item(instance, span)); } } _ => bug!(), @@ -583,7 +597,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { tcx.require_lang_item(ExchangeMallocFnLangItem, None); let instance = Instance::mono(tcx, exchange_malloc_fn_def_id); if should_monomorphize_locally(tcx, &instance) { - self.output.push(create_fn_mono_item(instance)); + self.output.push(create_fn_mono_item(instance, span)); } } mir::Rvalue::ThreadLocalRef(def_id) => { @@ -591,7 +605,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { let instance = Instance::mono(self.tcx, def_id); if should_monomorphize_locally(self.tcx, &instance) { trace!("collecting thread-local static {:?}", def_id); - self.output.push(MonoItem::Static(def_id)); + self.output.push(respan(span, MonoItem::Static(def_id))); } } _ => { /* not interesting */ } @@ -626,32 +640,33 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { debug!("visiting terminator {:?} @ {:?}", terminator, location); + let source = self.body.source_info(location).span; let tcx = self.tcx; match terminator.kind { mir::TerminatorKind::Call { ref func, .. } => { let callee_ty = func.ty(self.body, tcx); let callee_ty = self.monomorphize(callee_ty); - visit_fn_use(self.tcx, callee_ty, true, &mut self.output); + visit_fn_use(self.tcx, callee_ty, true, source, &mut self.output); } mir::TerminatorKind::Drop { ref place, .. } | mir::TerminatorKind::DropAndReplace { ref place, .. } => { let ty = place.ty(self.body, self.tcx).ty; let ty = self.monomorphize(ty); - visit_drop_use(self.tcx, ty, true, self.output); + visit_drop_use(self.tcx, ty, true, source, self.output); } mir::TerminatorKind::InlineAsm { ref operands, .. } => { for op in operands { match *op { mir::InlineAsmOperand::SymFn { ref value } => { let fn_ty = self.monomorphize(value.literal.ty); - visit_fn_use(self.tcx, fn_ty, false, &mut self.output); + visit_fn_use(self.tcx, fn_ty, false, source, &mut self.output); } mir::InlineAsmOperand::SymStatic { def_id } => { let instance = Instance::mono(self.tcx, def_id); if should_monomorphize_locally(self.tcx, &instance) { trace!("collecting asm sym static {:?}", def_id); - self.output.push(MonoItem::Static(def_id)); + self.output.push(respan(source, MonoItem::Static(def_id))); } } _ => {} @@ -687,17 +702,19 @@ fn visit_drop_use<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, is_direct_call: bool, - output: &mut Vec>, + source: Span, + output: &mut Vec>>, ) { let instance = Instance::resolve_drop_in_place(tcx, ty); - visit_instance_use(tcx, instance, is_direct_call, output); + visit_instance_use(tcx, instance, is_direct_call, source, output); } fn visit_fn_use<'tcx>( tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, is_direct_call: bool, - output: &mut Vec>, + source: Span, + output: &mut Vec>>, ) { if let ty::FnDef(def_id, substs) = ty.kind { let instance = if is_direct_call { @@ -706,7 +723,7 @@ fn visit_fn_use<'tcx>( ty::Instance::resolve_for_fn_ptr(tcx, ty::ParamEnv::reveal_all(), def_id, substs) .unwrap() }; - visit_instance_use(tcx, instance, is_direct_call, output); + visit_instance_use(tcx, instance, is_direct_call, source, output); } } @@ -714,7 +731,8 @@ fn visit_instance_use<'tcx>( tcx: TyCtxt<'tcx>, instance: ty::Instance<'tcx>, is_direct_call: bool, - output: &mut Vec>, + source: Span, + output: &mut Vec>>, ) { debug!("visit_item_use({:?}, is_direct_call={:?})", instance, is_direct_call); if !should_monomorphize_locally(tcx, &instance) { @@ -730,7 +748,7 @@ fn visit_instance_use<'tcx>( ty::InstanceDef::DropGlue(_, None) => { // Don't need to emit noop drop glue if we are calling directly. if !is_direct_call { - output.push(create_fn_mono_item(instance)); + output.push(create_fn_mono_item(instance, source)); } } ty::InstanceDef::DropGlue(_, Some(_)) @@ -740,7 +758,7 @@ fn visit_instance_use<'tcx>( | ty::InstanceDef::Item(..) | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::CloneShim(..) => { - output.push(create_fn_mono_item(instance)); + output.push(create_fn_mono_item(instance, source)); } } } @@ -832,7 +850,6 @@ fn find_vtable_types_for_unsizing<'tcx>( let ptr_vtable = |inner_source: Ty<'tcx>, inner_target: Ty<'tcx>| { let param_env = ty::ParamEnv::reveal_all(); let type_has_metadata = |ty: Ty<'tcx>| -> bool { - use rustc_span::DUMMY_SP; if ty.is_sized(tcx.at(DUMMY_SP), param_env) { return false; } @@ -886,9 +903,9 @@ fn find_vtable_types_for_unsizing<'tcx>( } } -fn create_fn_mono_item(instance: Instance<'_>) -> MonoItem<'_> { +fn create_fn_mono_item(instance: Instance<'_>, source: Span) -> Spanned> { debug!("create_fn_mono_item(instance={})", instance); - MonoItem::Fn(instance) + respan(source, MonoItem::Fn(instance)) } /// Creates a `MonoItem` for each method that is referenced by the vtable for @@ -897,7 +914,8 @@ fn create_mono_items_for_vtable_methods<'tcx>( tcx: TyCtxt<'tcx>, trait_ty: Ty<'tcx>, impl_ty: Ty<'tcx>, - output: &mut Vec>, + source: Span, + output: &mut Vec>>, ) { assert!( !trait_ty.needs_subst() @@ -927,12 +945,12 @@ fn create_mono_items_for_vtable_methods<'tcx>( .unwrap() }) .filter(|&instance| should_monomorphize_locally(tcx, &instance)) - .map(create_fn_mono_item); + .map(|item| create_fn_mono_item(item, source)); output.extend(methods); } // Also add the destructor. - visit_drop_use(tcx, impl_ty, false, output); + visit_drop_use(tcx, impl_ty, false, source, output); } } @@ -943,7 +961,7 @@ fn create_mono_items_for_vtable_methods<'tcx>( struct RootCollector<'a, 'tcx> { tcx: TyCtxt<'tcx>, mode: MonoItemCollectionMode, - output: &'a mut Vec>, + output: &'a mut Vec>>, entry_fn: Option<(LocalDefId, EntryFnType)>, } @@ -980,7 +998,7 @@ impl ItemLikeVisitor<'v> for RootCollector<'_, 'v> { let ty = Instance::new(def_id.to_def_id(), InternalSubsts::empty()) .monomorphic_ty(self.tcx); - visit_drop_use(self.tcx, ty, true, self.output); + visit_drop_use(self.tcx, ty, true, DUMMY_SP, self.output); } } } @@ -989,12 +1007,12 @@ impl ItemLikeVisitor<'v> for RootCollector<'_, 'v> { "RootCollector: ItemKind::GlobalAsm({})", def_id_to_string(self.tcx, self.tcx.hir().local_def_id(item.hir_id)) ); - self.output.push(MonoItem::GlobalAsm(item.hir_id)); + self.output.push(dummy_spanned(MonoItem::GlobalAsm(item.hir_id))); } hir::ItemKind::Static(..) => { let def_id = self.tcx.hir().local_def_id(item.hir_id); debug!("RootCollector: ItemKind::Static({})", def_id_to_string(self.tcx, def_id)); - self.output.push(MonoItem::Static(def_id.to_def_id())); + self.output.push(dummy_spanned(MonoItem::Static(def_id.to_def_id()))); } hir::ItemKind::Const(..) => { // const items only generate mono items if they are @@ -1051,7 +1069,7 @@ impl RootCollector<'_, 'v> { debug!("RootCollector::push_if_root: found root def_id={:?}", def_id); let instance = Instance::mono(self.tcx, def_id.to_def_id()); - self.output.push(create_fn_mono_item(instance)); + self.output.push(create_fn_mono_item(instance, DUMMY_SP)); } } @@ -1088,7 +1106,7 @@ impl RootCollector<'_, 'v> { .unwrap() .unwrap(); - self.output.push(create_fn_mono_item(start_instance)); + self.output.push(create_fn_mono_item(start_instance, DUMMY_SP)); } } @@ -1100,7 +1118,7 @@ fn item_requires_monomorphization(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { fn create_mono_items_for_default_impls<'tcx>( tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>, - output: &mut Vec>, + output: &mut Vec>>, ) { match item.kind { hir::ItemKind::Impl { ref generics, ref items, .. } => { @@ -1145,8 +1163,9 @@ fn create_mono_items_for_default_impls<'tcx>( .unwrap() .unwrap(); - let mono_item = create_fn_mono_item(instance); - if mono_item.is_instantiable(tcx) && should_monomorphize_locally(tcx, &instance) + let mono_item = create_fn_mono_item(instance, DUMMY_SP); + if mono_item.node.is_instantiable(tcx) + && should_monomorphize_locally(tcx, &instance) { output.push(mono_item); } @@ -1158,14 +1177,18 @@ fn create_mono_items_for_default_impls<'tcx>( } /// Scans the miri alloc in order to find function calls, closures, and drop-glue. -fn collect_miri<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut Vec>) { +fn collect_miri<'tcx>( + tcx: TyCtxt<'tcx>, + alloc_id: AllocId, + output: &mut Vec>>, +) { match tcx.global_alloc(alloc_id) { GlobalAlloc::Static(def_id) => { assert!(!tcx.is_thread_local_static(def_id)); let instance = Instance::mono(tcx, def_id); if should_monomorphize_locally(tcx, &instance) { trace!("collecting static {:?}", def_id); - output.push(MonoItem::Static(def_id)); + output.push(dummy_spanned(MonoItem::Static(def_id))); } } GlobalAlloc::Memory(alloc) => { @@ -1179,7 +1202,7 @@ fn collect_miri<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut Vec { if should_monomorphize_locally(tcx, &fn_instance) { trace!("collecting {:?} with {:#?}", alloc_id, fn_instance); - output.push(create_fn_mono_item(fn_instance)); + output.push(create_fn_mono_item(fn_instance, DUMMY_SP)); } } } @@ -1189,7 +1212,7 @@ fn collect_miri<'tcx>(tcx: TyCtxt<'tcx>, alloc_id: AllocId, output: &mut Vec( tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, - output: &mut Vec>, + output: &mut Vec>>, ) { debug!("collect_neighbours: {:?}", instance.def_id()); let body = tcx.instance_mir(instance.def); @@ -1207,7 +1230,7 @@ fn def_id_to_string(tcx: TyCtxt<'_>, def_id: LocalDefId) -> String { fn collect_const_value<'tcx>( tcx: TyCtxt<'tcx>, value: ConstValue<'tcx>, - output: &mut Vec>, + output: &mut Vec>>, ) { match value { ConstValue::Scalar(Scalar::Ptr(ptr)) => collect_miri(tcx, ptr.alloc_id, output), diff --git a/src/test/ui/infinite/infinite-instantiation.rs b/src/test/ui/infinite/infinite-instantiation.rs index 6f53680f7c81d..9fee01c1ba623 100644 --- a/src/test/ui/infinite/infinite-instantiation.rs +++ b/src/test/ui/infinite/infinite-instantiation.rs @@ -1,9 +1,3 @@ -// -// We get an error message at the top of file (dummy span). -// This is not helpful, but also kind of annoying to prevent, -// so for now just live with it. -// This test case was originally for issue #2258. - // build-fail trait ToOpt: Sized { @@ -23,11 +17,9 @@ impl ToOpt for Option { } fn function(counter: usize, t: T) { -//~^ ERROR reached the recursion limit while instantiating `function:: 0 { function(counter - 1, t.to_option()); - // FIXME(#4287) Error message should be here. It should be - // a type error to instantiate `test` at a type other than T. + //~^ ERROR reached the recursion limit while instantiating `function::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` - --> $DIR/infinite-instantiation.rs:25:1 + --> $DIR/infinite-instantiation.rs:21:9 + | +LL | function(counter - 1, t.to_option()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `function` defined here + --> $DIR/infinite-instantiation.rs:19:1 | LL | / fn function(counter: usize, t: T) { -LL | | LL | | if counter > 0 { LL | | function(counter - 1, t.to_option()); -... | +LL | | LL | | } LL | | } | |_^ diff --git a/src/test/ui/issues/issue-67552.rs b/src/test/ui/issues/issue-67552.rs index 1400c6f97b605..b0fcb74764b98 100644 --- a/src/test/ui/issues/issue-67552.rs +++ b/src/test/ui/issues/issue-67552.rs @@ -18,7 +18,6 @@ fn identity(x: T) -> T { } fn rec(mut it: T) -//~^ ERROR reached the recursion limit while instantiating where T: Iterator, { @@ -26,5 +25,6 @@ where T::count(it); } else { rec(identity(&mut it)) + //~^ ERROR reached the recursion limit while instantiating } } diff --git a/src/test/ui/issues/issue-67552.stderr b/src/test/ui/issues/issue-67552.stderr index 881f9d221d6ae..3bb2016f07d24 100644 --- a/src/test/ui/issues/issue-67552.stderr +++ b/src/test/ui/issues/issue-67552.stderr @@ -1,10 +1,16 @@ error: reached the recursion limit while instantiating `rec::<&mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut Empty>` + --> $DIR/issue-67552.rs:27:9 + | +LL | rec(identity(&mut it)) + | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: `rec` defined here --> $DIR/issue-67552.rs:20:1 | LL | / fn rec(mut it: T) -LL | | LL | | where LL | | T: Iterator, +LL | | { ... | LL | | } LL | | } diff --git a/src/test/ui/issues/issue-8727.rs b/src/test/ui/issues/issue-8727.rs index 80f360155cb49..14bdd8511119e 100644 --- a/src/test/ui/issues/issue-8727.rs +++ b/src/test/ui/issues/issue-8727.rs @@ -3,12 +3,10 @@ // build-fail -fn generic() { +fn generic() { //~ WARN function cannot return without recursing generic::>(); } -//~^^^ ERROR reached the recursion limit while instantiating `generic::>(); = help: a `loop` may express intention better if this is on purpose error: reached the recursion limit while instantiating `generic::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + --> $DIR/issue-8727.rs:7:5 + | +LL | generic::>(); + | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: `generic` defined here --> $DIR/issue-8727.rs:6:1 | LL | / fn generic() { diff --git a/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr b/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr index de6df4cd0268c..0552847c48ca9 100644 --- a/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr +++ b/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr @@ -1,4 +1,22 @@ error: reached the recursion limit while instantiating `std::intrinsics::drop_in_place::> - shim(Some(S))` + --> $SRC_DIR/libcore/ptr/mod.rs:LL:COL + | +LL | / pub unsafe fn drop_in_place(to_drop: *mut T) { +LL | | // Code here does not matter - this is replaced by the +LL | | // real drop glue by the compiler. +LL | | drop_in_place(to_drop) +LL | | } + | |_^ + | +note: `std::intrinsics::drop_in_place` defined here + --> $SRC_DIR/libcore/ptr/mod.rs:LL:COL + | +LL | / pub unsafe fn drop_in_place(to_drop: *mut T) { +LL | | // Code here does not matter - this is replaced by the +LL | | // real drop glue by the compiler. +LL | | drop_in_place(to_drop) +LL | | } + | |_^ error: aborting due to previous error diff --git a/src/test/ui/recursion/recursion.rs b/src/test/ui/recursion/recursion.rs index bf1eaef367d69..373cc17d0e0fe 100644 --- a/src/test/ui/recursion/recursion.rs +++ b/src/test/ui/recursion/recursion.rs @@ -12,11 +12,10 @@ impl Dot for Cons { self.head * other.head + self.tail.dot(other.tail) } } -fn test (n:isize, i:isize, first:T, second:T) ->isize { //~ ERROR recursion limit +fn test (n:isize, i:isize, first:T, second:T) ->isize { match n { 0 => {first.dot(second)} - // FIXME(#4287) Error message should be here. It should be - // a type error to instantiate `test` at a type other than T. _ => {test (n-1, i+1, Cons {head:2*i+1, tail:first}, Cons{head:i*i, tail:second})} + //~^ ERROR recursion limit } } pub fn main() { diff --git a/src/test/ui/recursion/recursion.stderr b/src/test/ui/recursion/recursion.stderr index 1a65b0e84f6a3..0c0eba68c83b4 100644 --- a/src/test/ui/recursion/recursion.stderr +++ b/src/test/ui/recursion/recursion.stderr @@ -1,11 +1,16 @@ error: reached the recursion limit while instantiating `test::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` + --> $DIR/recursion.rs:17:11 + | +LL | _ => {test (n-1, i+1, Cons {head:2*i+1, tail:first}, Cons{head:i*i, tail:second})} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `test` defined here --> $DIR/recursion.rs:15:1 | LL | / fn test (n:isize, i:isize, first:T, second:T) ->isize { LL | | match n { 0 => {first.dot(second)} -LL | | // FIXME(#4287) Error message should be here. It should be -LL | | // a type error to instantiate `test` at a type other than T. LL | | _ => {test (n-1, i+1, Cons {head:2*i+1, tail:first}, Cons{head:i*i, tail:second})} +LL | | LL | | } LL | | } | |_^ From f4a79385cf28b263894be9ebd2e541532ae82898 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 14:11:55 -0700 Subject: [PATCH 15/24] implemented query for coverage data This commit adds a query that allows the CoverageData to be pulled from a call on tcx, avoiding the need to change the `codegen_intrinsic_call()` signature (no need to pass in the FunctionCx or any additional arguments. The commit does not change where/when the CoverageData is computed. It's still done in the `pass`, and saved in the MIR `Body`. See discussion (in progress) here: https://github.com/rust-lang/rust/pull/73488#discussion_r443825646 --- src/librustc_codegen_llvm/intrinsic.rs | 12 +++++++----- src/librustc_codegen_ssa/mir/block.rs | 2 +- src/librustc_codegen_ssa/mir/mod.rs | 4 ++-- src/librustc_codegen_ssa/traits/intrinsic.rs | 6 ++---- src/librustc_middle/query/mod.rs | 6 ++++++ src/librustc_mir/transform/mod.rs | 8 +++++++- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index c6e7820a60ed4..f1104ca3a98b3 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -16,7 +16,6 @@ use rustc_codegen_ssa::common::TypeKind; use rustc_codegen_ssa::glue; use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue}; use rustc_codegen_ssa::mir::place::PlaceRef; -use rustc_codegen_ssa::mir::FunctionCx; use rustc_codegen_ssa::traits::*; use rustc_codegen_ssa::MemFlags; use rustc_hir as hir; @@ -82,14 +81,14 @@ fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: &str) -> Option<&'ll Valu } impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { - fn codegen_intrinsic_call<'b, Bx: BuilderMethods<'b, 'tcx>>( + fn codegen_intrinsic_call( &mut self, - fx: &FunctionCx<'b, 'tcx, Bx>, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, args: &[OperandRef<'tcx, &'ll Value>], llresult: &'ll Value, span: Span, + caller_instance: ty::Instance<'tcx>, ) { let tcx = self.tcx; let callee_ty = instance.monomorphic_ty(tcx); @@ -141,8 +140,11 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { - let coverage_data = fx.mir.coverage_data.as_ref().unwrap(); - let mangled_fn = tcx.symbol_name(fx.instance); + let coverage_data = tcx + .coverage_data(caller_instance.def_id()) + .as_ref() + .expect("LLVM intrinsic count_code_region call has associated coverage_data"); + let mangled_fn = tcx.symbol_name(caller_instance); let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); let hash = self.const_u64(coverage_data.hash); let index = args[0].immediate(); diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 945c3d4843471..d56c816811b3c 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -688,12 +688,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { .collect(); bx.codegen_intrinsic_call( - self, *instance.as_ref().unwrap(), &fn_abi, &args, dest, terminator.source_info.span, + self.instance, ); if let ReturnDest::IndirectOperand(dst, _) = ret_dest { diff --git a/src/librustc_codegen_ssa/mir/mod.rs b/src/librustc_codegen_ssa/mir/mod.rs index fd2e779f681be..00b4bf96afa59 100644 --- a/src/librustc_codegen_ssa/mir/mod.rs +++ b/src/librustc_codegen_ssa/mir/mod.rs @@ -21,9 +21,9 @@ use self::operand::{OperandRef, OperandValue}; /// Master context for codegenning from MIR. pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { - pub instance: Instance<'tcx>, + instance: Instance<'tcx>, - pub mir: &'tcx mir::Body<'tcx>, + mir: &'tcx mir::Body<'tcx>, debug_context: Option>, diff --git a/src/librustc_codegen_ssa/traits/intrinsic.rs b/src/librustc_codegen_ssa/traits/intrinsic.rs index 0036eadf4db81..f62019498511c 100644 --- a/src/librustc_codegen_ssa/traits/intrinsic.rs +++ b/src/librustc_codegen_ssa/traits/intrinsic.rs @@ -1,7 +1,5 @@ use super::BackendTypes; use crate::mir::operand::OperandRef; -use crate::mir::FunctionCx; -use crate::traits::BuilderMethods; use rustc_middle::ty::{self, Ty}; use rustc_span::Span; use rustc_target::abi::call::FnAbi; @@ -10,14 +8,14 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes { /// Remember to add all intrinsics here, in librustc_typeck/check/mod.rs, /// and in libcore/intrinsics.rs; if you need access to any llvm intrinsics, /// add them to librustc_codegen_llvm/context.rs - fn codegen_intrinsic_call<'a, Bx: BuilderMethods<'a, 'tcx>>( + fn codegen_intrinsic_call( &mut self, - fx: &FunctionCx<'a, 'tcx, Bx>, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, args: &[OperandRef<'tcx, Self::Value>], llresult: Self::Value, span: Span, + caller_instance: ty::Instance<'tcx>, ); fn abort(&mut self); diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 3b6d54a1bc1ee..1092e6c306411 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -214,6 +214,12 @@ rustc_queries! { cache_on_disk_if { key.is_local() } } + query coverage_data(key: DefId) -> Option { + desc { |tcx| "retrieving coverage data, if computed from MIR for `{}`", tcx.def_path_str(key) } + storage(ArenaCacheSelector<'tcx>) + cache_on_disk_if { key.is_local() } + } + query promoted_mir(key: DefId) -> IndexVec> { desc { |tcx| "optimizing promoted MIR for `{}`", tcx.def_path_str(key) } storage(ArenaCacheSelector<'tcx>) diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 846ed1f86d8d6..a2fdc7efd184d 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -6,7 +6,7 @@ use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::Visitor as _; -use rustc_middle::mir::{traversal, Body, ConstQualifs, MirPhase, Promoted}; +use rustc_middle::mir::{traversal, Body, ConstQualifs, CoverageData, MirPhase, Promoted}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::steal::Steal; use rustc_middle::ty::{InstanceDef, TyCtxt, TypeFoldable}; @@ -53,6 +53,7 @@ pub(crate) fn provide(providers: &mut Providers<'_>) { mir_drops_elaborated_and_const_checked, optimized_mir, is_mir_available, + coverage_data, promoted_mir, ..*providers }; @@ -422,6 +423,11 @@ fn run_optimization_passes<'tcx>( ); } +fn coverage_data(tcx: TyCtxt<'_>, def_id: DefId) -> Option { + let body = tcx.optimized_mir(def_id); + body.coverage_data.clone() +} + fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> Body<'_> { if tcx.is_constructor(def_id) { // There's no reason to run all of the MIR passes on constructors when From f84b7e1b052fd135ae2e754499b4fe286d5ba699 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 22 Jun 2020 12:28:16 -0700 Subject: [PATCH 16/24] Provide context on E0308 involving fn items --- .../infer/error_reporting/mod.rs | 2 +- src/librustc_typeck/check/demand.rs | 1 + src/librustc_typeck/check/mod.rs | 42 +++++++++++++++++++ src/test/ui/fn/fn-item-type.rs | 34 ++++++++++++--- src/test/ui/fn/fn-item-type.stderr | 31 ++++++++++++-- 5 files changed, 99 insertions(+), 11 deletions(-) diff --git a/src/librustc_infer/infer/error_reporting/mod.rs b/src/librustc_infer/infer/error_reporting/mod.rs index 9cfa11dd7c813..7fdcbd31df3c5 100644 --- a/src/librustc_infer/infer/error_reporting/mod.rs +++ b/src/librustc_infer/infer/error_reporting/mod.rs @@ -1256,7 +1256,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { (ty::FnDef(did1, substs1), ty::FnPtr(sig2)) => { let sig1 = self.tcx.fn_sig(*did1).subst(self.tcx, substs1); let mut values = self.cmp_fn_sig(&sig1, sig2); - values.0.push_normal(format!( + values.0.push_highlighted(format!( " {{{}}}", self.tcx.def_path_str_with_substs(*did1, substs1) )); diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 019b4ca66060c..f4f630e94a70a 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -34,6 +34,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty); self.suggest_missing_await(err, expr, expected, expr_ty); + self.note_need_for_fn_pointer(err, expected, expr_ty); } // Requires that the two types unify, and prints an error message if diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index b60b06567d6fa..234a573b725ee 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5496,6 +5496,48 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + fn note_need_for_fn_pointer( + &self, + err: &mut DiagnosticBuilder<'_>, + expected: Ty<'tcx>, + found: Ty<'tcx>, + ) { + match (&expected.kind, &found.kind) { + (ty::FnDef(did1, substs1), ty::FnDef(did2, substs2)) => { + let sig1 = self.tcx.fn_sig(*did1).subst(self.tcx, substs1); + let sig2 = self.tcx.fn_sig(*did2).subst(self.tcx, substs2); + if sig1 != sig2 { + return; + } + err.note( + "different `fn` items always have unique types, even if their signatures are \ + the same", + ); + err.help(&format!("change the expectation to require function pointer `{}`", sig1)); + err.help(&format!( + "if the expectation is due to type inference, cast the expected `fn` to a \ + function pointer: `{} as {}`", + self.tcx.def_path_str_with_substs(*did1, substs1), + sig1 + )); + } + (ty::FnDef(did, substs), ty::FnPtr(sig2)) => { + let sig1 = self.tcx.fn_sig(*did).subst(self.tcx, substs); + if sig1 != *sig2 { + return; + } + err.help(&format!("change the expectation to require function pointer `{}`", sig1)); + err.help(&format!( + "if the expectation is due to type inference, cast the expected `fn` to a \ + function pointer: `{} as {}`", + self.tcx.def_path_str_with_substs(*did, substs), + sig1 + )); + } + _ => {} + } + } + /// A common error is to add an extra semicolon: /// /// ``` diff --git a/src/test/ui/fn/fn-item-type.rs b/src/test/ui/fn/fn-item-type.rs index 68b75c18a43dc..256b9d45755c3 100644 --- a/src/test/ui/fn/fn-item-type.rs +++ b/src/test/ui/fn/fn-item-type.rs @@ -12,22 +12,44 @@ impl Foo for T { /* `foo` is still default here */ } fn main() { eq(foo::, bar::); //~^ ERROR mismatched types - //~| expected fn item `fn(_) -> _ {foo::}` - //~| found fn item `fn(_) -> _ {bar::}` - //~| expected fn item, found a different fn item + //~| expected fn item `fn(_) -> _ {foo::}` + //~| found fn item `fn(_) -> _ {bar::}` + //~| expected fn item, found a different fn item + //~| different `fn` items always have unique types, even if their signatures are the same + //~| change the expectation to require function pointer + //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer eq(foo::, foo::); //~^ ERROR mismatched types //~| expected `u8`, found `i8` + //~| different `fn` items always have unique types, even if their signatures are the same + //~| change the expectation to require function pointer + //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer eq(bar::, bar::>); //~^ ERROR mismatched types - //~| expected fn item `fn(_) -> _ {bar::}` - //~| found fn item `fn(_) -> _ {bar::>}` - //~| expected struct `std::string::String`, found struct `std::vec::Vec` + //~| expected fn item `fn(_) -> _ {bar::}` + //~| found fn item `fn(_) -> _ {bar::>}` + //~| expected struct `std::string::String`, found struct `std::vec::Vec` + //~| different `fn` items always have unique types, even if their signatures are the same + //~| change the expectation to require function pointer + //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer // Make sure we distinguish between trait methods correctly. eq(::foo, ::foo); //~^ ERROR mismatched types //~| expected `u8`, found `u16` + //~| different `fn` items always have unique types, even if their signatures are the same + //~| change the expectation to require function pointer + //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer + + eq(foo::, bar:: as fn(isize) -> isize); + //~^ ERROR mismatched types + //~| expected fn item `fn(_) -> _ {foo::}` + //~| found fn pointer `fn(_) -> _` + //~| expected fn item, found fn pointer + //~| change the expectation to require function pointer + //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer + + eq(foo:: as fn(isize) -> isize, bar::); // ok! } diff --git a/src/test/ui/fn/fn-item-type.stderr b/src/test/ui/fn/fn-item-type.stderr index 4cce25c43c485..84f5e034340eb 100644 --- a/src/test/ui/fn/fn-item-type.stderr +++ b/src/test/ui/fn/fn-item-type.stderr @@ -6,34 +6,57 @@ LL | eq(foo::, bar::); | = note: expected fn item `fn(_) -> _ {foo::}` found fn item `fn(_) -> _ {bar::}` + = note: different `fn` items always have unique types, even if their signatures are the same + = help: change the expectation to require function pointer `fn(isize) -> isize` + = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` error[E0308]: mismatched types - --> $DIR/fn-item-type.rs:19:19 + --> $DIR/fn-item-type.rs:22:19 | LL | eq(foo::, foo::); | ^^^^^^^^^ expected `u8`, found `i8` | = note: expected fn item `fn(_) -> _ {foo::}` found fn item `fn(_) -> _ {foo::}` + = note: different `fn` items always have unique types, even if their signatures are the same + = help: change the expectation to require function pointer `fn(isize) -> isize` + = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` error[E0308]: mismatched types - --> $DIR/fn-item-type.rs:23:23 + --> $DIR/fn-item-type.rs:29:23 | LL | eq(bar::, bar::>); | ^^^^^^^^^^^^^^ expected struct `std::string::String`, found struct `std::vec::Vec` | = note: expected fn item `fn(_) -> _ {bar::}` found fn item `fn(_) -> _ {bar::>}` + = note: different `fn` items always have unique types, even if their signatures are the same + = help: change the expectation to require function pointer `fn(isize) -> isize` + = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `bar:: as fn(isize) -> isize` error[E0308]: mismatched types - --> $DIR/fn-item-type.rs:30:26 + --> $DIR/fn-item-type.rs:39:26 | LL | eq(::foo, ::foo); | ^^^^^^^^^^^^^^^^^ expected `u8`, found `u16` | = note: expected fn item `fn() {::foo}` found fn item `fn() {::foo}` + = note: different `fn` items always have unique types, even if their signatures are the same + = help: change the expectation to require function pointer `fn()` + = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `::foo as fn()` -error: aborting due to 4 previous errors +error[E0308]: mismatched types + --> $DIR/fn-item-type.rs:46:19 + | +LL | eq(foo::, bar:: as fn(isize) -> isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn item, found fn pointer + | + = note: expected fn item `fn(_) -> _ {foo::}` + found fn pointer `fn(_) -> _` + = help: change the expectation to require function pointer `fn(isize) -> isize` + = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` + +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0308`. From 994d9d03272363d57a655ebacbf58f0069b3b177 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 15:54:28 -0700 Subject: [PATCH 17/24] Address remaining feedback items --- src/librustc_metadata/locator.rs | 2 +- src/librustc_middle/hir/map/mod.rs | 2 +- src/librustc_middle/query/mod.rs | 2 +- .../transform/instrument_coverage.rs | 27 ++----------------- 4 files changed, 5 insertions(+), 28 deletions(-) diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index 662794f0aa11f..1bdac1039b55a 100644 --- a/src/librustc_metadata/locator.rs +++ b/src/librustc_metadata/locator.rs @@ -489,7 +489,7 @@ impl<'a> CrateLocator<'a> { { err.note(&format!("the `{}` target may not be installed", self.triple)); } else if self.crate_name == sym::profiler_builtins { - err.note(&"the compiler may have been built without `profiler = true`"); + err.note(&"the compiler may have been built without the profiler runtime"); } err.span_label(self.span, "can't find crate"); err diff --git a/src/librustc_middle/hir/map/mod.rs b/src/librustc_middle/hir/map/mod.rs index e3e0856ffc52e..d60d24aa9aed5 100644 --- a/src/librustc_middle/hir/map/mod.rs +++ b/src/librustc_middle/hir/map/mod.rs @@ -56,7 +56,7 @@ fn fn_sig<'hir>(node: Node<'hir>) -> Option<&'hir FnSig<'hir>> { } } -fn associated_body<'hir>(node: Node<'hir>) -> Option { +pub fn associated_body<'hir>(node: Node<'hir>) -> Option { match node { Node::Item(Item { kind: ItemKind::Const(_, body) | ItemKind::Static(.., body) | ItemKind::Fn(.., body), diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 1092e6c306411..993b48afb7a9a 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -215,7 +215,7 @@ rustc_queries! { } query coverage_data(key: DefId) -> Option { - desc { |tcx| "retrieving coverage data, if computed from MIR for `{}`", tcx.def_path_str(key) } + desc { |tcx| "retrieving coverage data from MIR for `{}`", tcx.def_path_str(key) } storage(ArenaCacheSelector<'tcx>) cache_on_disk_if { key.is_local() } } diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index a24d0acf4212c..94aa26b3081e5 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -3,7 +3,7 @@ use crate::util::patch::MirPatch; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::lang_items; -use rustc_hir::*; +use rustc_middle::hir; use rustc_middle::ich::StableHashingContext; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::{ @@ -140,7 +140,7 @@ fn placeholder_block(span: Span) -> BasicBlockData<'tcx> { fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, src: &MirSource<'tcx>) -> u64 { let fn_body_id = match tcx.hir().get_if_local(src.def_id()) { - Some(node) => match associated_body(node) { + Some(node) => match hir::map::associated_body(node) { Some(body_id) => body_id, _ => bug!("instrumented MirSource does not include a function body: {:?}", node), }, @@ -159,26 +159,3 @@ fn hash( node.hash_stable(hcx, &mut stable_hasher); stable_hasher.finish() } - -fn associated_body<'hir>(node: Node<'hir>) -> Option { - match node { - Node::Item(Item { - kind: ItemKind::Const(_, body) | ItemKind::Static(.., body) | ItemKind::Fn(.., body), - .. - }) - | Node::TraitItem(TraitItem { - kind: - TraitItemKind::Const(_, Some(body)) | TraitItemKind::Fn(_, TraitFn::Provided(body)), - .. - }) - | Node::ImplItem(ImplItem { - kind: ImplItemKind::Const(_, body) | ImplItemKind::Fn(_, body), - .. - }) - | Node::Expr(Expr { kind: ExprKind::Closure(.., body, _, _), .. }) => Some(*body), - - Node::AnonConst(constant) => Some(constant.body), - - _ => None, - } -} From 08ec4cbb9e0672118946c85f410b50ea4001b1dd Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 19:21:56 -0700 Subject: [PATCH 18/24] moves coverage data computation from pass to query --- src/librustc_codegen_llvm/intrinsic.rs | 9 +-- src/librustc_middle/mir/mod.rs | 34 +++++------ src/librustc_middle/query/mod.rs | 2 +- .../transform/instrument_coverage.rs | 58 +++++++++++-------- src/librustc_mir/transform/mod.rs | 9 +-- 5 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index f1104ca3a98b3..b9193a85b1e48 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -140,18 +140,15 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { - let coverage_data = tcx - .coverage_data(caller_instance.def_id()) - .as_ref() - .expect("LLVM intrinsic count_code_region call has associated coverage_data"); + let coverage_data = tcx.coverage_data(caller_instance.def_id()); let mangled_fn = tcx.symbol_name(caller_instance); let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); let hash = self.const_u64(coverage_data.hash); - let index = args[0].immediate(); let num_counters = self.const_u32(coverage_data.num_counters); + let index = args[0].immediate(); debug!( "count_code_region to LLVM intrinsic instrprof.increment(fn_name={}, hash={:?}, num_counters={:?}, index={:?})", - mangled_fn.name, hash, index, num_counters + mangled_fn.name, hash, num_counters, index ); self.instrprof_increment(mangled_fn_name, hash, num_counters, index) } diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index e88329db992f5..854fda095b65b 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -88,19 +88,6 @@ impl MirPhase { } } -/// Coverage data computed by the `InstrumentCoverage` MIR pass, when compiling with -/// `-Zinstrument_coverage`. -#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable, TypeFoldable)] -pub struct CoverageData { - /// A hash value that can be used by the consumer of the coverage profile data to detect - /// changes to the instrumented source of the associated MIR body (typically, for an - /// individual function). - pub hash: u64, - - /// The total number of coverage region counters added to this MIR Body. - pub num_counters: u32, -} - /// The lowered representation of a single function. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable, TypeFoldable)] pub struct Body<'tcx> { @@ -184,10 +171,6 @@ pub struct Body<'tcx> { /// FIXME(oli-obk): rewrite the promoted during promotion to eliminate the cell components. pub ignore_interior_mut_in_const_validation: bool, - /// If compiling with `-Zinstrument_coverage`, the `InstrumentCoverage` pass stores summary - /// information associated with the MIR, used in code generation of the coverage counters. - pub coverage_data: Option, - predecessor_cache: PredecessorCache, } @@ -228,7 +211,6 @@ impl<'tcx> Body<'tcx> { required_consts: Vec::new(), ignore_interior_mut_in_const_validation: false, control_flow_destroyed, - coverage_data: None, predecessor_cache: PredecessorCache::new(), } } @@ -256,7 +238,6 @@ impl<'tcx> Body<'tcx> { generator_kind: None, var_debug_info: Vec::new(), ignore_interior_mut_in_const_validation: false, - coverage_data: None, predecessor_cache: PredecessorCache::new(), } } @@ -2938,3 +2919,18 @@ impl Location { } } } + +/// Coverage data associated with each function (MIR) instrumented with coverage counters, when +/// compiled with `-Zinstrument_coverage`. The query `tcx.coverage_data(DefId)` computes these +/// values on demand (during code generation). This query is only valid after executing the MIR pass +/// `InstrumentCoverage`. +#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] +pub struct CoverageData { + /// A hash value that can be used by the consumer of the coverage profile data to detect + /// changes to the instrumented source of the associated MIR body (typically, for an + /// individual function). + pub hash: u64, + + /// The total number of coverage region counters added to the MIR `Body`. + pub num_counters: u32, +} diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 993b48afb7a9a..4815f2617b69b 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -214,7 +214,7 @@ rustc_queries! { cache_on_disk_if { key.is_local() } } - query coverage_data(key: DefId) -> Option { + query coverage_data(key: DefId) -> mir::CoverageData { desc { |tcx| "retrieving coverage data from MIR for `{}`", tcx.def_path_str(key) } storage(ArenaCacheSelector<'tcx>) cache_on_disk_if { key.is_local() } diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 94aa26b3081e5..8d263c3089c47 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -7,10 +7,12 @@ use rustc_middle::hir; use rustc_middle::ich::StableHashingContext; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::{ - self, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, StatementKind, - Terminator, TerminatorKind, START_BLOCK, + self, traversal, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, + StatementKind, Terminator, TerminatorKind, START_BLOCK, }; use rustc_middle::ty; +use rustc_middle::ty::query::Providers; +use rustc_middle::ty::FnDef; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; use rustc_span::Span; @@ -19,6 +21,31 @@ use rustc_span::Span; /// the intrinsic llvm.instrprof.increment. pub struct InstrumentCoverage; +/// The `query` provider for `CoverageData`, requested by `codegen_intrinsic_call()` when +/// constructing the arguments for `llvm.instrprof.increment`. +pub(crate) fn provide(providers: &mut Providers<'_>) { + providers.coverage_data = |tcx, def_id| { + let body = tcx.optimized_mir(def_id); + let count_code_region_fn = + tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None); + let mut num_counters: u32 = 0; + for (_, data) in traversal::preorder(body) { + if let Some(terminator) = &data.terminator { + if let TerminatorKind::Call { func: Operand::Constant(func), .. } = &terminator.kind + { + if let FnDef(called_fn_def_id, _) = func.literal.ty.kind { + if called_fn_def_id == count_code_region_fn { + num_counters += 1; + } + } + } + } + } + let hash = if num_counters > 0 { hash_mir_source(tcx, def_id) } else { 0 }; + CoverageData { num_counters, hash } + }; +} + struct Instrumentor<'tcx> { tcx: TyCtxt<'tcx>, num_counters: u32, @@ -30,20 +57,12 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { // If the InstrumentCoverage pass is called on promoted MIRs, skip them. // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 if src.promoted.is_none() { - assert!(mir_body.coverage_data.is_none()); - - let hash = hash_mir_source(tcx, &src); - debug!( - "instrumenting {:?}, hash: {}, span: {}", + "instrumenting {:?}, span: {}", src.def_id(), - hash, tcx.sess.source_map().span_to_string(mir_body.span) ); - - let num_counters = Instrumentor::new(tcx).inject_counters(mir_body); - - mir_body.coverage_data = Some(CoverageData { hash, num_counters }); + Instrumentor::new(tcx).inject_counters(mir_body); } } } @@ -60,15 +79,13 @@ impl<'tcx> Instrumentor<'tcx> { next } - fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) -> u32 { + fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) { // FIXME(richkadel): As a first step, counters are only injected at the top of each // function. The complete solution will inject counters at each conditional code branch. let top_of_function = START_BLOCK; let entire_function = mir_body.span; self.inject_counter(mir_body, top_of_function, entire_function); - - self.num_counters } fn inject_counter( @@ -138,14 +155,9 @@ fn placeholder_block(span: Span) -> BasicBlockData<'tcx> { } } -fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, src: &MirSource<'tcx>) -> u64 { - let fn_body_id = match tcx.hir().get_if_local(src.def_id()) { - Some(node) => match hir::map::associated_body(node) { - Some(body_id) => body_id, - _ => bug!("instrumented MirSource does not include a function body: {:?}", node), - }, - None => bug!("instrumented MirSource is not local: {:?}", src), - }; +fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> u64 { + let hir_node = tcx.hir().get_if_local(def_id).expect("DefId is local"); + let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); let hir_body = tcx.hir().body(fn_body_id); let mut hcx = tcx.create_no_span_stable_hashing_context(); hash(&mut hcx, &hir_body.value).to_smaller_hash() diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index a2fdc7efd184d..8ca240d2c7da7 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -6,7 +6,7 @@ use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::Visitor as _; -use rustc_middle::mir::{traversal, Body, ConstQualifs, CoverageData, MirPhase, Promoted}; +use rustc_middle::mir::{traversal, Body, ConstQualifs, MirPhase, Promoted}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::steal::Steal; use rustc_middle::ty::{InstanceDef, TyCtxt, TypeFoldable}; @@ -53,10 +53,10 @@ pub(crate) fn provide(providers: &mut Providers<'_>) { mir_drops_elaborated_and_const_checked, optimized_mir, is_mir_available, - coverage_data, promoted_mir, ..*providers }; + instrument_coverage::provide(providers); } fn is_mir_available(tcx: TyCtxt<'_>, def_id: DefId) -> bool { @@ -423,11 +423,6 @@ fn run_optimization_passes<'tcx>( ); } -fn coverage_data(tcx: TyCtxt<'_>, def_id: DefId) -> Option { - let body = tcx.optimized_mir(def_id); - body.coverage_data.clone() -} - fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> Body<'_> { if tcx.is_constructor(def_id) { // There's no reason to run all of the MIR passes on constructors when From 3d0192e7c80b4aaa53f7609c1a73118bc91d30aa Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 19:27:48 -0700 Subject: [PATCH 19/24] PR no longer requires u32 impl TypeFoldable --- src/librustc_middle/ty/structural_impls.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_middle/ty/structural_impls.rs b/src/librustc_middle/ty/structural_impls.rs index 463e2c57d46c9..f04d31601ea5b 100644 --- a/src/librustc_middle/ty/structural_impls.rs +++ b/src/librustc_middle/ty/structural_impls.rs @@ -262,7 +262,6 @@ CloneTypeFoldableAndLiftImpls! { bool, usize, ::rustc_target::abi::VariantIdx, - u32, u64, String, crate::middle::region::Scope, From a04514026824f9342ab93d9b608e3ec5dab53dad Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 19:30:52 -0700 Subject: [PATCH 20/24] using "mir_body" (vs "body") in InstrumentCoverage The mod uses both MIR bodies and HIR bodies, so I'm trying to maintain consistency with these names. --- src/librustc_mir/transform/instrument_coverage.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 8d263c3089c47..27aaf47bbf2ae 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -25,11 +25,11 @@ pub struct InstrumentCoverage; /// constructing the arguments for `llvm.instrprof.increment`. pub(crate) fn provide(providers: &mut Providers<'_>) { providers.coverage_data = |tcx, def_id| { - let body = tcx.optimized_mir(def_id); + let mir_body = tcx.optimized_mir(def_id); let count_code_region_fn = tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None); let mut num_counters: u32 = 0; - for (_, data) in traversal::preorder(body) { + for (_, data) in traversal::preorder(mir_body) { if let Some(terminator) = &data.terminator { if let TerminatorKind::Call { func: Operand::Constant(func), .. } = &terminator.kind { From 977ce57d915914139c4aa643e63f368913e5f437 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 23:31:41 -0700 Subject: [PATCH 21/24] Updated query for num_counters to compute from max index Also added FIXME comments to note the possible need to accommodate counter increment calls in source-based functions that differ from the function context of the caller instance (e.g., inline functions). --- src/librustc_codegen_llvm/intrinsic.rs | 3 ++ .../transform/instrument_coverage.rs | 28 ++++++++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index b9193a85b1e48..dfe97b1ee2e9d 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -140,6 +140,9 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { + // FIXME(richkadel): The current implementation assumes the MIR for the given + // caller_instance represents a single function. Validate and/or correct if inlining + // and/or monomorphization invalidates these assumptions. let coverage_data = tcx.coverage_data(caller_instance.def_id()); let mangled_fn = tcx.symbol_name(caller_instance); let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 27aaf47bbf2ae..06b648ab5a908 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -5,15 +5,15 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::lang_items; use rustc_middle::hir; use rustc_middle::ich::StableHashingContext; -use rustc_middle::mir::interpret::Scalar; +use rustc_middle::mir::interpret::{ConstValue, Scalar}; use rustc_middle::mir::{ self, traversal, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, StatementKind, Terminator, TerminatorKind, START_BLOCK, }; use rustc_middle::ty; use rustc_middle::ty::query::Providers; -use rustc_middle::ty::FnDef; use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{ConstKind, FnDef}; use rustc_span::def_id::DefId; use rustc_span::Span; @@ -26,16 +26,36 @@ pub struct InstrumentCoverage; pub(crate) fn provide(providers: &mut Providers<'_>) { providers.coverage_data = |tcx, def_id| { let mir_body = tcx.optimized_mir(def_id); + // FIXME(richkadel): The current implementation assumes the MIR for the given DefId + // represents a single function. Validate and/or correct if inlining and/or monomorphization + // invalidates these assumptions. let count_code_region_fn = tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None); let mut num_counters: u32 = 0; + // The `num_counters` argument to `llvm.instrprof.increment` is the number of injected + // counters, with each counter having an index from `0..num_counters-1`. MIR optimization + // may split and duplicate some BasicBlock sequences. Simply counting the calls may not + // not work; but computing the num_counters by adding `1` to the highest index (for a given + // instrumented function) is valid. for (_, data) in traversal::preorder(mir_body) { if let Some(terminator) = &data.terminator { - if let TerminatorKind::Call { func: Operand::Constant(func), .. } = &terminator.kind + if let TerminatorKind::Call { func: Operand::Constant(func), args, .. } = + &terminator.kind { if let FnDef(called_fn_def_id, _) = func.literal.ty.kind { if called_fn_def_id == count_code_region_fn { - num_counters += 1; + if let Operand::Constant(constant) = + args.get(0).expect("count_code_region has at least one arg") + { + if let ConstKind::Value(ConstValue::Scalar(value)) = + constant.literal.val + { + let index = value + .to_u32() + .expect("count_code_region index at arg0 is u32"); + num_counters = std::cmp::max(num_counters, index + 1); + } + } } } } From 5fa8b0880825d83eb01151e43e7de1e94e05cd2d Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 22 Jun 2020 14:03:18 +0200 Subject: [PATCH 22/24] The const propagator cannot trace references. Thus we avoid propagation of a local the moment we encounter references to it. --- src/librustc_mir/transform/const_prop.rs | 54 +++++++++---- .../32bit/rustc.main.ConstProp.diff | 18 +---- .../64bit/rustc.main.ConstProp.diff | 18 +---- src/test/mir-opt/const_prop_miscompile.rs | 22 ++++++ .../rustc.bar.ConstProp.diff | 75 +++++++++++++++++++ .../rustc.foo.ConstProp.diff | 63 ++++++++++++++++ src/test/ui/mir/mir_detects_invalid_ops.rs | 2 +- .../ui/mir/mir_detects_invalid_ops.stderr | 8 +- 8 files changed, 204 insertions(+), 56 deletions(-) create mode 100644 src/test/mir-opt/const_prop_miscompile.rs create mode 100644 src/test/mir-opt/const_prop_miscompile/rustc.bar.ConstProp.diff create mode 100644 src/test/mir-opt/const_prop_miscompile/rustc.foo.ConstProp.diff diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 529e63ab96791..0044ed36bdf5e 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -575,8 +575,16 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } // Do not try creating references (#67862) - Rvalue::Ref(_, _, place_ref) => { - trace!("skipping Ref({:?})", place_ref); + Rvalue::AddressOf(_, place) | Rvalue::Ref(_, _, place) => { + trace!("skipping AddressOf | Ref for {:?}", place); + + // This may be creating mutable references or immutable references to cells. + // If that happens, the pointed to value could be mutated via that reference. + // Since we aren't tracking references, the const propagator loses track of what + // value the local has right now. + // Thus, all locals that have their reference taken + // must not take part in propagation. + Self::remove_const(&mut self.ecx, place.local); return None; } @@ -716,6 +724,9 @@ enum ConstPropMode { OnlyInsideOwnBlock, /// The `Local` can be propagated into but reads cannot be propagated. OnlyPropagateInto, + /// The `Local` cannot be part of propagation at all. Any statement + /// referencing it either for reading or writing will not get propagated. + NoPropagation, } struct CanConstProp { @@ -781,7 +792,9 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { // end of the block anyway, and inside the block we overwrite previous // states as applicable. ConstPropMode::OnlyInsideOwnBlock => {} - other => { + ConstPropMode::NoPropagation => {} + ConstPropMode::OnlyPropagateInto => {} + other @ ConstPropMode::FullConstProp => { trace!( "local {:?} can't be propagated because of multiple assignments", local, @@ -813,7 +826,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { | MutatingUse(MutatingUseContext::Borrow) | MutatingUse(MutatingUseContext::AddressOf) => { trace!("local {:?} can't be propagaged because it's used: {:?}", local, context); - self.can_const_prop[local] = ConstPropMode::OnlyPropagateInto; + self.can_const_prop[local] = ConstPropMode::NoPropagation; } } } @@ -858,19 +871,22 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { } } } - if can_const_prop == ConstPropMode::OnlyInsideOwnBlock { - trace!( - "found local restricted to its block. Will remove it from const-prop after block is finished. Local: {:?}", - place.local - ); - self.locals_of_current_block.insert(place.local); - } - - if can_const_prop == ConstPropMode::OnlyPropagateInto { - trace!("can't propagate into {:?}", place); - if place.local != RETURN_PLACE { - Self::remove_const(&mut self.ecx, place.local); + match can_const_prop { + ConstPropMode::OnlyInsideOwnBlock => { + trace!( + "found local restricted to its block. \ + Will remove it from const-prop after block is finished. Local: {:?}", + place.local + ); + self.locals_of_current_block.insert(place.local); } + ConstPropMode::OnlyPropagateInto | ConstPropMode::NoPropagation => { + trace!("can't propagate into {:?}", place); + if place.local != RETURN_PLACE { + Self::remove_const(&mut self.ecx, place.local); + } + } + ConstPropMode::FullConstProp => {} } } else { // Const prop failed, so erase the destination, ensuring that whatever happens @@ -890,6 +906,12 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> { ); Self::remove_const(&mut self.ecx, place.local); } + } else { + trace!( + "cannot propagate into {:?}, because the type of the local is generic.", + place, + ); + Self::remove_const(&mut self.ecx, place.local); } } else { match statement.kind { diff --git a/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/32bit/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/32bit/rustc.main.ConstProp.diff index 7071f31dbf104..7ceec94d81e76 100644 --- a/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/32bit/rustc.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/32bit/rustc.main.ConstProp.diff @@ -46,22 +46,8 @@ // mir::Constant // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:23: 7:24 // + literal: Const { ty: usize, val: Value(Scalar(0x00000003)) } -- _7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -- _8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ _7 = const 3usize; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // ty::Const -+ // + ty: usize -+ // + val: Value(Scalar(0x00000003)) -+ // mir::Constant -+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // + literal: Const { ty: usize, val: Value(Scalar(0x00000003)) } -+ _8 = const false; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // ty::Const -+ // + ty: bool -+ // + val: Value(Scalar(0x00)) -+ // mir::Constant -+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) } + _7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 + _8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 assert(move _8, "index out of bounds: the len is {} but the index is {}", move _7, _6) -> bb1; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 } diff --git a/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/64bit/rustc.main.ConstProp.diff b/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/64bit/rustc.main.ConstProp.diff index 15995ab070019..483a6f232ef79 100644 --- a/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/64bit/rustc.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/bad_op_unsafe_oob_for_slices/64bit/rustc.main.ConstProp.diff @@ -46,22 +46,8 @@ // mir::Constant // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:23: 7:24 // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000003)) } -- _7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -- _8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ _7 = const 3usize; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // ty::Const -+ // + ty: usize -+ // + val: Value(Scalar(0x0000000000000003)) -+ // mir::Constant -+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // + literal: Const { ty: usize, val: Value(Scalar(0x0000000000000003)) } -+ _8 = const false; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // ty::Const -+ // + ty: bool -+ // + val: Value(Scalar(0x00)) -+ // mir::Constant -+ // + span: $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 -+ // + literal: Const { ty: bool, val: Value(Scalar(0x00)) } + _7 = Len((*_1)); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 + _8 = Lt(_6, _7); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 assert(move _8, "index out of bounds: the len is {} but the index is {}", move _7, _6) -> bb1; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25 } diff --git a/src/test/mir-opt/const_prop_miscompile.rs b/src/test/mir-opt/const_prop_miscompile.rs new file mode 100644 index 0000000000000..043b22870f49e --- /dev/null +++ b/src/test/mir-opt/const_prop_miscompile.rs @@ -0,0 +1,22 @@ +#![feature(raw_ref_op)] + +// EMIT_MIR rustc.foo.ConstProp.diff +fn foo() { + let mut u = (1,); + *&mut u.0 = 5; + let y = { u.0 } == 5; +} + +// EMIT_MIR rustc.bar.ConstProp.diff +fn bar() { + let mut v = (1,); + unsafe { + *&raw mut v.0 = 5; + } + let y = { v.0 } == 5; +} + +fn main() { + foo(); + bar(); +} diff --git a/src/test/mir-opt/const_prop_miscompile/rustc.bar.ConstProp.diff b/src/test/mir-opt/const_prop_miscompile/rustc.bar.ConstProp.diff new file mode 100644 index 0000000000000..c87f67bf9f587 --- /dev/null +++ b/src/test/mir-opt/const_prop_miscompile/rustc.bar.ConstProp.diff @@ -0,0 +1,75 @@ +- // MIR for `bar` before ConstProp ++ // MIR for `bar` after ConstProp + + fn bar() -> () { + let mut _0: (); // return place in scope 0 at $DIR/const_prop_miscompile.rs:11:10: 11:10 + let mut _1: (i32,); // in scope 0 at $DIR/const_prop_miscompile.rs:12:9: 12:14 + let _2: (); // in scope 0 at $DIR/const_prop_miscompile.rs:13:5: 15:6 + let mut _3: *mut i32; // in scope 0 at $DIR/const_prop_miscompile.rs:14:10: 14:22 + let mut _5: i32; // in scope 0 at $DIR/const_prop_miscompile.rs:16:13: 16:20 + scope 1 { + debug v => _1; // in scope 1 at $DIR/const_prop_miscompile.rs:12:9: 12:14 + let _4: bool; // in scope 1 at $DIR/const_prop_miscompile.rs:16:9: 16:10 + scope 2 { + } + scope 3 { + debug y => _4; // in scope 3 at $DIR/const_prop_miscompile.rs:16:9: 16:10 + } + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/const_prop_miscompile.rs:12:9: 12:14 +- _1 = (const 1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:12:17: 12:21 ++ _1 = const (1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:12:17: 12:21 + // ty::Const +- // + ty: i32 ++ // + ty: (i32,) + // + val: Value(Scalar(0x00000001)) + // mir::Constant +- // + span: $DIR/const_prop_miscompile.rs:12:18: 12:19 +- // + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) } ++ // + span: $DIR/const_prop_miscompile.rs:12:17: 12:21 ++ // + literal: Const { ty: (i32,), val: Value(Scalar(0x00000001)) } + StorageLive(_2); // scope 1 at $DIR/const_prop_miscompile.rs:13:5: 15:6 + StorageLive(_3); // scope 2 at $DIR/const_prop_miscompile.rs:14:10: 14:22 + _3 = &raw mut (_1.0: i32); // scope 2 at $DIR/const_prop_miscompile.rs:14:10: 14:22 + (*_3) = const 5i32; // scope 2 at $DIR/const_prop_miscompile.rs:14:9: 14:26 + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x00000005)) + // mir::Constant + // + span: $DIR/const_prop_miscompile.rs:14:25: 14:26 + // + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) } + StorageDead(_3); // scope 2 at $DIR/const_prop_miscompile.rs:14:26: 14:27 + _2 = const (); // scope 2 at $DIR/const_prop_miscompile.rs:13:5: 15:6 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/const_prop_miscompile.rs:13:5: 15:6 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_2); // scope 1 at $DIR/const_prop_miscompile.rs:15:5: 15:6 + StorageLive(_4); // scope 1 at $DIR/const_prop_miscompile.rs:16:9: 16:10 + StorageLive(_5); // scope 1 at $DIR/const_prop_miscompile.rs:16:13: 16:20 + _5 = (_1.0: i32); // scope 1 at $DIR/const_prop_miscompile.rs:16:15: 16:18 + _4 = Eq(move _5, const 5i32); // scope 1 at $DIR/const_prop_miscompile.rs:16:13: 16:25 + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x00000005)) + // mir::Constant + // + span: $DIR/const_prop_miscompile.rs:16:24: 16:25 + // + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) } + StorageDead(_5); // scope 1 at $DIR/const_prop_miscompile.rs:16:24: 16:25 + _0 = const (); // scope 0 at $DIR/const_prop_miscompile.rs:11:10: 17:2 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/const_prop_miscompile.rs:11:10: 17:2 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_4); // scope 1 at $DIR/const_prop_miscompile.rs:17:1: 17:2 + StorageDead(_1); // scope 0 at $DIR/const_prop_miscompile.rs:17:1: 17:2 + return; // scope 0 at $DIR/const_prop_miscompile.rs:17:2: 17:2 + } + } + diff --git a/src/test/mir-opt/const_prop_miscompile/rustc.foo.ConstProp.diff b/src/test/mir-opt/const_prop_miscompile/rustc.foo.ConstProp.diff new file mode 100644 index 0000000000000..8a6850d2fe3ad --- /dev/null +++ b/src/test/mir-opt/const_prop_miscompile/rustc.foo.ConstProp.diff @@ -0,0 +1,63 @@ +- // MIR for `foo` before ConstProp ++ // MIR for `foo` after ConstProp + + fn foo() -> () { + let mut _0: (); // return place in scope 0 at $DIR/const_prop_miscompile.rs:4:10: 4:10 + let mut _1: (i32,); // in scope 0 at $DIR/const_prop_miscompile.rs:5:9: 5:14 + let mut _2: &mut i32; // in scope 0 at $DIR/const_prop_miscompile.rs:6:6: 6:14 + let mut _4: i32; // in scope 0 at $DIR/const_prop_miscompile.rs:7:13: 7:20 + scope 1 { + debug u => _1; // in scope 1 at $DIR/const_prop_miscompile.rs:5:9: 5:14 + let _3: bool; // in scope 1 at $DIR/const_prop_miscompile.rs:7:9: 7:10 + scope 2 { + debug y => _3; // in scope 2 at $DIR/const_prop_miscompile.rs:7:9: 7:10 + } + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/const_prop_miscompile.rs:5:9: 5:14 +- _1 = (const 1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:5:17: 5:21 ++ _1 = const (1i32,); // scope 0 at $DIR/const_prop_miscompile.rs:5:17: 5:21 + // ty::Const +- // + ty: i32 ++ // + ty: (i32,) + // + val: Value(Scalar(0x00000001)) + // mir::Constant +- // + span: $DIR/const_prop_miscompile.rs:5:18: 5:19 +- // + literal: Const { ty: i32, val: Value(Scalar(0x00000001)) } ++ // + span: $DIR/const_prop_miscompile.rs:5:17: 5:21 ++ // + literal: Const { ty: (i32,), val: Value(Scalar(0x00000001)) } + StorageLive(_2); // scope 1 at $DIR/const_prop_miscompile.rs:6:6: 6:14 + _2 = &mut (_1.0: i32); // scope 1 at $DIR/const_prop_miscompile.rs:6:6: 6:14 + (*_2) = const 5i32; // scope 1 at $DIR/const_prop_miscompile.rs:6:5: 6:18 + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x00000005)) + // mir::Constant + // + span: $DIR/const_prop_miscompile.rs:6:17: 6:18 + // + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) } + StorageDead(_2); // scope 1 at $DIR/const_prop_miscompile.rs:6:18: 6:19 + StorageLive(_3); // scope 1 at $DIR/const_prop_miscompile.rs:7:9: 7:10 + StorageLive(_4); // scope 1 at $DIR/const_prop_miscompile.rs:7:13: 7:20 + _4 = (_1.0: i32); // scope 1 at $DIR/const_prop_miscompile.rs:7:15: 7:18 + _3 = Eq(move _4, const 5i32); // scope 1 at $DIR/const_prop_miscompile.rs:7:13: 7:25 + // ty::Const + // + ty: i32 + // + val: Value(Scalar(0x00000005)) + // mir::Constant + // + span: $DIR/const_prop_miscompile.rs:7:24: 7:25 + // + literal: Const { ty: i32, val: Value(Scalar(0x00000005)) } + StorageDead(_4); // scope 1 at $DIR/const_prop_miscompile.rs:7:24: 7:25 + _0 = const (); // scope 0 at $DIR/const_prop_miscompile.rs:4:10: 8:2 + // ty::Const + // + ty: () + // + val: Value(Scalar()) + // mir::Constant + // + span: $DIR/const_prop_miscompile.rs:4:10: 8:2 + // + literal: Const { ty: (), val: Value(Scalar()) } + StorageDead(_3); // scope 1 at $DIR/const_prop_miscompile.rs:8:1: 8:2 + StorageDead(_1); // scope 0 at $DIR/const_prop_miscompile.rs:8:1: 8:2 + return; // scope 0 at $DIR/const_prop_miscompile.rs:8:2: 8:2 + } + } + diff --git a/src/test/ui/mir/mir_detects_invalid_ops.rs b/src/test/ui/mir/mir_detects_invalid_ops.rs index 0940dbe6a5e87..136c03cd9f1bc 100644 --- a/src/test/ui/mir/mir_detects_invalid_ops.rs +++ b/src/test/ui/mir/mir_detects_invalid_ops.rs @@ -19,6 +19,6 @@ fn mod_by_zero() { fn oob_error_for_slices() { let a: *const [_] = &[1, 2, 3]; unsafe { - let _b = (*a)[3]; //~ ERROR this operation will panic at runtime [unconditional_panic] + let _b = (*a)[3]; } } diff --git a/src/test/ui/mir/mir_detects_invalid_ops.stderr b/src/test/ui/mir/mir_detects_invalid_ops.stderr index 41f03789f237f..0b6dbfd7c3d85 100644 --- a/src/test/ui/mir/mir_detects_invalid_ops.stderr +++ b/src/test/ui/mir/mir_detects_invalid_ops.stderr @@ -12,11 +12,5 @@ error: this operation will panic at runtime LL | let _z = 1 % y; | ^^^^^ attempt to calculate the remainder with a divisor of zero -error: this operation will panic at runtime - --> $DIR/mir_detects_invalid_ops.rs:22:18 - | -LL | let _b = (*a)[3]; - | ^^^^^^^ index out of bounds: the len is 3 but the index is 3 - -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors From 0c2b02536c857b78cd9560e447fa669d4ca2ba3e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 23 Jun 2020 09:41:56 -0700 Subject: [PATCH 23/24] rustc: Modernize wasm checks for atomics This commit modernizes how rustc checks for whether the `atomics` feature is enabled for the wasm target. The `sess.target_features` set is consulted instead of fiddling around with dealing with various aspects of LLVM and that syntax. --- src/librustc_codegen_llvm/back/write.rs | 3 ++- src/librustc_codegen_ssa/back/linker.rs | 5 ++--- src/librustc_span/symbol.rs | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 868ce876a8192..54271d3dd0491 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -23,6 +23,7 @@ use rustc_middle::bug; use rustc_middle::ty::TyCtxt; use rustc_session::config::{self, Lto, OutputType, Passes, SanitizerSet, SwitchWithOptPath}; use rustc_session::Session; +use rustc_span::symbol::sym; use rustc_span::InnerSpan; use rustc_target::spec::{CodeModel, RelocModel}; @@ -140,7 +141,7 @@ pub fn target_machine_factory( // lower atomic operations to single-threaded operations. if singlethread && sess.target.target.llvm_target.contains("wasm32") - && features.iter().any(|s| *s == "+atomics") + && sess.target_features.contains(&sym::atomics) { singlethread = false; } diff --git a/src/librustc_codegen_ssa/back/linker.rs b/src/librustc_codegen_ssa/back/linker.rs index 6011d422ca682..d6ef94bfd1727 100644 --- a/src/librustc_codegen_ssa/back/linker.rs +++ b/src/librustc_codegen_ssa/back/linker.rs @@ -1,6 +1,7 @@ use super::archive; use super::command::Command; use super::symbol_export; +use rustc_span::symbol::sym; use std::ffi::{OsStr, OsString}; use std::fs::{self, File}; @@ -1036,9 +1037,7 @@ impl<'a> WasmLd<'a> { // // * `--export=*tls*` - when `#[thread_local]` symbols are used these // symbols are how the TLS segments are initialized and configured. - let atomics = sess.opts.cg.target_feature.contains("+atomics") - || sess.target.target.options.features.contains("+atomics"); - if atomics { + if sess.target_features.contains(&sym::atomics) { cmd.arg("--shared-memory"); cmd.arg("--max-memory=1073741824"); cmd.arg("--import-memory"); diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 06d1f36622b94..6dab25dccad40 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -159,6 +159,7 @@ symbols! { assume_init, async_await, async_closure, + atomics, attr, attributes, attr_literals, From 6e8aa1ff27297694e5092f19f5e2d6d244d076a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jun 2020 13:01:24 -0700 Subject: [PATCH 24/24] review comments: wording and style --- src/librustc_typeck/check/mod.rs | 29 ++++++++++++----------------- src/test/ui/fn/fn-item-type.rs | 20 ++++++++++---------- src/test/ui/fn/fn-item-type.stderr | 20 ++++++++++---------- 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 234a573b725ee..761807213d2a7 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5502,7 +5502,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, found: Ty<'tcx>, ) { - match (&expected.kind, &found.kind) { + let (sig, did, substs) = match (&expected.kind, &found.kind) { (ty::FnDef(did1, substs1), ty::FnDef(did2, substs2)) => { let sig1 = self.tcx.fn_sig(*did1).subst(self.tcx, substs1); let sig2 = self.tcx.fn_sig(*did2).subst(self.tcx, substs2); @@ -5513,29 +5513,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "different `fn` items always have unique types, even if their signatures are \ the same", ); - err.help(&format!("change the expectation to require function pointer `{}`", sig1)); - err.help(&format!( - "if the expectation is due to type inference, cast the expected `fn` to a \ - function pointer: `{} as {}`", - self.tcx.def_path_str_with_substs(*did1, substs1), - sig1 - )); + (sig1, *did1, substs1) } (ty::FnDef(did, substs), ty::FnPtr(sig2)) => { let sig1 = self.tcx.fn_sig(*did).subst(self.tcx, substs); if sig1 != *sig2 { return; } - err.help(&format!("change the expectation to require function pointer `{}`", sig1)); - err.help(&format!( - "if the expectation is due to type inference, cast the expected `fn` to a \ - function pointer: `{} as {}`", - self.tcx.def_path_str_with_substs(*did, substs), - sig1 - )); + (sig1, *did, substs) } - _ => {} - } + _ => return, + }; + err.help(&format!("change the expected type to be function pointer `{}`", sig)); + err.help(&format!( + "if the expected type is due to type inference, cast the expected `fn` to a function \ + pointer: `{} as {}`", + self.tcx.def_path_str_with_substs(did, substs), + sig + )); } /// A common error is to add an extra semicolon: diff --git a/src/test/ui/fn/fn-item-type.rs b/src/test/ui/fn/fn-item-type.rs index 256b9d45755c3..abae40162a0fc 100644 --- a/src/test/ui/fn/fn-item-type.rs +++ b/src/test/ui/fn/fn-item-type.rs @@ -16,15 +16,15 @@ fn main() { //~| found fn item `fn(_) -> _ {bar::}` //~| expected fn item, found a different fn item //~| different `fn` items always have unique types, even if their signatures are the same - //~| change the expectation to require function pointer - //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer + //~| change the expected type to be function pointer + //~| if the expected type is due to type inference, cast the expected `fn` to a function pointer eq(foo::, foo::); //~^ ERROR mismatched types //~| expected `u8`, found `i8` //~| different `fn` items always have unique types, even if their signatures are the same - //~| change the expectation to require function pointer - //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer + //~| change the expected type to be function pointer + //~| if the expected type is due to type inference, cast the expected `fn` to a function pointer eq(bar::, bar::>); //~^ ERROR mismatched types @@ -32,24 +32,24 @@ fn main() { //~| found fn item `fn(_) -> _ {bar::>}` //~| expected struct `std::string::String`, found struct `std::vec::Vec` //~| different `fn` items always have unique types, even if their signatures are the same - //~| change the expectation to require function pointer - //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer + //~| change the expected type to be function pointer + //~| if the expected type is due to type inference, cast the expected `fn` to a function pointer // Make sure we distinguish between trait methods correctly. eq(::foo, ::foo); //~^ ERROR mismatched types //~| expected `u8`, found `u16` //~| different `fn` items always have unique types, even if their signatures are the same - //~| change the expectation to require function pointer - //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer + //~| change the expected type to be function pointer + //~| if the expected type is due to type inference, cast the expected `fn` to a function pointer eq(foo::, bar:: as fn(isize) -> isize); //~^ ERROR mismatched types //~| expected fn item `fn(_) -> _ {foo::}` //~| found fn pointer `fn(_) -> _` //~| expected fn item, found fn pointer - //~| change the expectation to require function pointer - //~| if the expectation is due to type inference, cast the expected `fn` to a function pointer + //~| change the expected type to be function pointer + //~| if the expected type is due to type inference, cast the expected `fn` to a function pointer eq(foo:: as fn(isize) -> isize, bar::); // ok! } diff --git a/src/test/ui/fn/fn-item-type.stderr b/src/test/ui/fn/fn-item-type.stderr index 84f5e034340eb..bfa9efa219f4c 100644 --- a/src/test/ui/fn/fn-item-type.stderr +++ b/src/test/ui/fn/fn-item-type.stderr @@ -7,8 +7,8 @@ LL | eq(foo::, bar::); = note: expected fn item `fn(_) -> _ {foo::}` found fn item `fn(_) -> _ {bar::}` = note: different `fn` items always have unique types, even if their signatures are the same - = help: change the expectation to require function pointer `fn(isize) -> isize` - = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` + = help: change the expected type to be function pointer `fn(isize) -> isize` + = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` error[E0308]: mismatched types --> $DIR/fn-item-type.rs:22:19 @@ -19,8 +19,8 @@ LL | eq(foo::, foo::); = note: expected fn item `fn(_) -> _ {foo::}` found fn item `fn(_) -> _ {foo::}` = note: different `fn` items always have unique types, even if their signatures are the same - = help: change the expectation to require function pointer `fn(isize) -> isize` - = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` + = help: change the expected type to be function pointer `fn(isize) -> isize` + = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` error[E0308]: mismatched types --> $DIR/fn-item-type.rs:29:23 @@ -31,8 +31,8 @@ LL | eq(bar::, bar::>); = note: expected fn item `fn(_) -> _ {bar::}` found fn item `fn(_) -> _ {bar::>}` = note: different `fn` items always have unique types, even if their signatures are the same - = help: change the expectation to require function pointer `fn(isize) -> isize` - = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `bar:: as fn(isize) -> isize` + = help: change the expected type to be function pointer `fn(isize) -> isize` + = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `bar:: as fn(isize) -> isize` error[E0308]: mismatched types --> $DIR/fn-item-type.rs:39:26 @@ -43,8 +43,8 @@ LL | eq(::foo, ::foo); = note: expected fn item `fn() {::foo}` found fn item `fn() {::foo}` = note: different `fn` items always have unique types, even if their signatures are the same - = help: change the expectation to require function pointer `fn()` - = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `::foo as fn()` + = help: change the expected type to be function pointer `fn()` + = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `::foo as fn()` error[E0308]: mismatched types --> $DIR/fn-item-type.rs:46:19 @@ -54,8 +54,8 @@ LL | eq(foo::, bar:: as fn(isize) -> isize); | = note: expected fn item `fn(_) -> _ {foo::}` found fn pointer `fn(_) -> _` - = help: change the expectation to require function pointer `fn(isize) -> isize` - = help: if the expectation is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` + = help: change the expected type to be function pointer `fn(isize) -> isize` + = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `foo:: as fn(isize) -> isize` error: aborting due to 5 previous errors