From 4379a43e461f7727af2a37b12c7c83f290fc7856 Mon Sep 17 00:00:00 2001 From: Justus K Date: Thu, 8 Oct 2020 22:23:01 +0200 Subject: [PATCH 01/18] Suggest turbofish for uninferred const argument --- .../infer/error_reporting/need_type_info.rs | 32 ++++++++++++++++--- .../infer/cannot-infer-const-args.full.stderr | 5 +++ .../infer/cannot-infer-const-args.min.stderr | 5 +++ .../const-generics/infer/issue-77092.stderr | 5 +++ .../infer/method-chain.full.stderr | 5 +++ .../infer/method-chain.min.stderr | 5 +++ .../infer/one-param-uninferred.full.stderr | 14 ++++++++ .../infer/one-param-uninferred.min.stderr | 14 ++++++++ .../infer/one-param-uninferred.rs | 17 ++++++++++ .../infer/uninferred-consts.full.stderr | 7 +++- .../infer/uninferred-consts.min.stderr | 7 +++- .../const-generics/infer/uninferred-consts.rs | 2 +- 12 files changed, 110 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/const-generics/infer/one-param-uninferred.full.stderr create mode 100644 src/test/ui/const-generics/infer/one-param-uninferred.min.stderr create mode 100644 src/test/ui/const-generics/infer/one-param-uninferred.rs diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs index fd8f46a6926c0..373f0a602c0ef 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs @@ -124,6 +124,11 @@ impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> { return; } } + + // FIXME(const_generics): Currently, any uninferred `const` generics arguments + // are handled specially, but instead they should be handled in `annotate_method_call`, + // which currently doesn't work because this evaluates to `false` for const arguments. + // See https://github.com/rust-lang/rust/pull/77758 for more details. if self.node_ty_contains_target(expr.hir_id).is_some() { match expr.kind { ExprKind::Closure(..) => self.found_closure = Some(&expr), @@ -345,11 +350,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ) -> DiagnosticBuilder<'tcx> { let arg = self.resolve_vars_if_possible(arg); let arg_data = self.extract_inference_diagnostics_data(arg, None); - let kind_str = match arg.unpack() { - GenericArgKind::Type(_) => "type", - GenericArgKind::Const(_) => "the value", - GenericArgKind::Lifetime(_) => bug!("unexpected lifetime"), - }; let mut local_visitor = FindHirNodeVisitor::new(&self, arg, span); let ty_to_string = |ty: Ty<'tcx>| -> String { @@ -618,6 +618,28 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { .any(|span_label| span_label.label.is_some() && span_label.span == span) && local_visitor.found_arg_pattern.is_none() { + let (kind_str, const_value) = match arg.unpack() { + GenericArgKind::Type(_) => ("type", None), + GenericArgKind::Const(_) => ("the value", Some(())), + GenericArgKind::Lifetime(_) => bug!("unexpected lifetime"), + }; + + // FIXME(const_generics): we would like to handle const arguments + // as part of the normal diagnostics flow below, but there appear to + // be subtleties in doing so, so for now we special-case const args + // here. + if let Some(suggestion) = const_value + .and_then(|_| arg_data.parent_name.as_ref()) + .map(|parent| format!("{}::<{}>", parent, arg_data.name)) + { + err.span_suggestion_verbose( + span, + "consider specifying the const argument", + suggestion, + Applicability::MaybeIncorrect, + ); + } + // Avoid multiple labels pointing at `span`. err.span_label( span, diff --git a/src/test/ui/const-generics/infer/cannot-infer-const-args.full.stderr b/src/test/ui/const-generics/infer/cannot-infer-const-args.full.stderr index b438ed3ad6508..05bf67a5ff7c6 100644 --- a/src/test/ui/const-generics/infer/cannot-infer-const-args.full.stderr +++ b/src/test/ui/const-generics/infer/cannot-infer-const-args.full.stderr @@ -3,6 +3,11 @@ error[E0282]: type annotations needed | LL | foo(); | ^^^ cannot infer the value of const parameter `X` declared on the function `foo` + | +help: consider specifying the const argument + | +LL | foo::(); + | ^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/const-generics/infer/cannot-infer-const-args.min.stderr b/src/test/ui/const-generics/infer/cannot-infer-const-args.min.stderr index b438ed3ad6508..05bf67a5ff7c6 100644 --- a/src/test/ui/const-generics/infer/cannot-infer-const-args.min.stderr +++ b/src/test/ui/const-generics/infer/cannot-infer-const-args.min.stderr @@ -3,6 +3,11 @@ error[E0282]: type annotations needed | LL | foo(); | ^^^ cannot infer the value of const parameter `X` declared on the function `foo` + | +help: consider specifying the const argument + | +LL | foo::(); + | ^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/const-generics/infer/issue-77092.stderr b/src/test/ui/const-generics/infer/issue-77092.stderr index 63facbf3b8c0f..99894173bc8f6 100644 --- a/src/test/ui/const-generics/infer/issue-77092.stderr +++ b/src/test/ui/const-generics/infer/issue-77092.stderr @@ -3,6 +3,11 @@ error[E0282]: type annotations needed | LL | println!("{:?}", take_array_from_mut(&mut arr, i)); | ^^^^^^^^^^^^^^^^^^^ cannot infer the value of const parameter `N` declared on the function `take_array_from_mut` + | +help: consider specifying the const argument + | +LL | println!("{:?}", take_array_from_mut::(&mut arr, i)); + | ^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/const-generics/infer/method-chain.full.stderr b/src/test/ui/const-generics/infer/method-chain.full.stderr index 1fb0b23cf1157..7aa3bd44df844 100644 --- a/src/test/ui/const-generics/infer/method-chain.full.stderr +++ b/src/test/ui/const-generics/infer/method-chain.full.stderr @@ -3,6 +3,11 @@ error[E0282]: type annotations needed | LL | Foo.bar().bar().bar().bar().baz(); | ^^^ cannot infer the value of const parameter `N` declared on the associated function `baz` + | +help: consider specifying the const argument + | +LL | Foo.bar().bar().bar().bar().baz::(); + | ^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/const-generics/infer/method-chain.min.stderr b/src/test/ui/const-generics/infer/method-chain.min.stderr index 1fb0b23cf1157..7aa3bd44df844 100644 --- a/src/test/ui/const-generics/infer/method-chain.min.stderr +++ b/src/test/ui/const-generics/infer/method-chain.min.stderr @@ -3,6 +3,11 @@ error[E0282]: type annotations needed | LL | Foo.bar().bar().bar().bar().baz(); | ^^^ cannot infer the value of const parameter `N` declared on the associated function `baz` + | +help: consider specifying the const argument + | +LL | Foo.bar().bar().bar().bar().baz::(); + | ^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/const-generics/infer/one-param-uninferred.full.stderr b/src/test/ui/const-generics/infer/one-param-uninferred.full.stderr new file mode 100644 index 0000000000000..cc6c9a475104c --- /dev/null +++ b/src/test/ui/const-generics/infer/one-param-uninferred.full.stderr @@ -0,0 +1,14 @@ +error[E0282]: type annotations needed + --> $DIR/one-param-uninferred.rs:15:23 + | +LL | let _: [u8; 17] = foo(); + | ^^^ cannot infer the value of const parameter `M` declared on the function `foo` + | +help: consider specifying the const argument + | +LL | let _: [u8; 17] = foo::(); + | ^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/src/test/ui/const-generics/infer/one-param-uninferred.min.stderr b/src/test/ui/const-generics/infer/one-param-uninferred.min.stderr new file mode 100644 index 0000000000000..cc6c9a475104c --- /dev/null +++ b/src/test/ui/const-generics/infer/one-param-uninferred.min.stderr @@ -0,0 +1,14 @@ +error[E0282]: type annotations needed + --> $DIR/one-param-uninferred.rs:15:23 + | +LL | let _: [u8; 17] = foo(); + | ^^^ cannot infer the value of const parameter `M` declared on the function `foo` + | +help: consider specifying the const argument + | +LL | let _: [u8; 17] = foo::(); + | ^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/src/test/ui/const-generics/infer/one-param-uninferred.rs b/src/test/ui/const-generics/infer/one-param-uninferred.rs new file mode 100644 index 0000000000000..0e947131f4cdb --- /dev/null +++ b/src/test/ui/const-generics/infer/one-param-uninferred.rs @@ -0,0 +1,17 @@ +// Test that we emit an error if we cannot properly infer a constant. +// revisions: full min + +#![cfg_attr(full, feature(const_generics))] +#![cfg_attr(full, allow(incomplete_features))] +#![cfg_attr(min, feature(min_const_generics))] + +fn foo() -> [u8; N] { + todo!() +} + +fn main() { + // FIXME(const_generics): Currently this only suggests one const parameter, + // but instead it should suggest to provide all parameters. + let _: [u8; 17] = foo(); + //~^ ERROR type annotations needed +} diff --git a/src/test/ui/const-generics/infer/uninferred-consts.full.stderr b/src/test/ui/const-generics/infer/uninferred-consts.full.stderr index 7a451903e9630..4be625ba90930 100644 --- a/src/test/ui/const-generics/infer/uninferred-consts.full.stderr +++ b/src/test/ui/const-generics/infer/uninferred-consts.full.stderr @@ -2,7 +2,12 @@ error[E0282]: type annotations needed --> $DIR/uninferred-consts.rs:14:9 | LL | Foo.foo(); - | ^^^ cannot infer the value of const parameter `N` declared on the associated function `foo` + | ^^^ cannot infer the value of const parameter `A` declared on the associated function `foo` + | +help: consider specifying the const argument + | +LL | Foo.foo::(); + | ^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/const-generics/infer/uninferred-consts.min.stderr b/src/test/ui/const-generics/infer/uninferred-consts.min.stderr index 7a451903e9630..4be625ba90930 100644 --- a/src/test/ui/const-generics/infer/uninferred-consts.min.stderr +++ b/src/test/ui/const-generics/infer/uninferred-consts.min.stderr @@ -2,7 +2,12 @@ error[E0282]: type annotations needed --> $DIR/uninferred-consts.rs:14:9 | LL | Foo.foo(); - | ^^^ cannot infer the value of const parameter `N` declared on the associated function `foo` + | ^^^ cannot infer the value of const parameter `A` declared on the associated function `foo` + | +help: consider specifying the const argument + | +LL | Foo.foo::(); + | ^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/const-generics/infer/uninferred-consts.rs b/src/test/ui/const-generics/infer/uninferred-consts.rs index ec5b3ffe5440b..00fb6eac99208 100644 --- a/src/test/ui/const-generics/infer/uninferred-consts.rs +++ b/src/test/ui/const-generics/infer/uninferred-consts.rs @@ -8,7 +8,7 @@ // taken from https://github.com/rust-lang/rust/issues/70507#issuecomment-615268893 struct Foo; impl Foo { - fn foo(self) {} + fn foo(self) {} } fn main() { Foo.foo(); From 6e17ab57fc3a1c12850c4ec0ae40319d9f18e63b Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 15 Nov 2020 20:29:01 +0100 Subject: [PATCH 02/18] Lower `if let` before the arms. --- compiler/rustc_ast_lowering/src/expr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index f83fc29577bfb..314e5103cc2de 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -353,7 +353,6 @@ impl<'hir> LoweringContext<'_, 'hir> { let else_arm = self.arm(else_pat, else_expr); // Handle then + scrutinee: - let then_expr = self.lower_block_expr(then); let (then_pat, scrutinee, desugar) = match cond.kind { // ` => `: ExprKind::Let(ref pat, ref scrutinee) => { @@ -375,6 +374,7 @@ impl<'hir> LoweringContext<'_, 'hir> { (pat, cond, hir::MatchSource::IfDesugar { contains_else_clause }) } }; + let then_expr = self.lower_block_expr(then); let then_arm = self.arm(then_pat, self.arena.alloc(then_expr)); hir::ExprKind::Match(scrutinee, arena_vec![self; then_arm, else_arm], desugar) @@ -400,7 +400,6 @@ impl<'hir> LoweringContext<'_, 'hir> { }; // Handle then + scrutinee: - let then_expr = self.lower_block_expr(body); let (then_pat, scrutinee, desugar, source) = match cond.kind { ExprKind::Let(ref pat, ref scrutinee) => { // to: @@ -440,6 +439,7 @@ impl<'hir> LoweringContext<'_, 'hir> { (pat, cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While) } }; + let then_expr = self.lower_block_expr(body); let then_arm = self.arm(then_pat, self.arena.alloc(then_expr)); // `match { ... }` From 27c60bad8a9c9600a8bcd5462fa6341bc717d5fc Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 15 Nov 2020 23:19:55 +0100 Subject: [PATCH 03/18] Remove Pat pre-lowering. --- compiler/rustc_ast_lowering/src/lib.rs | 48 +++----------------------- 1 file changed, 5 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index d93655e59050d..f7c693cc94d1f 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -425,7 +425,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { /// declared for every type and trait definition. struct MiscCollector<'tcx, 'lowering, 'hir> { lctx: &'tcx mut LoweringContext<'lowering, 'hir>, - hir_id_owner: Option, } impl MiscCollector<'_, '_, '_> { @@ -452,30 +451,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } } - - fn with_hir_id_owner( - &mut self, - owner: Option, - f: impl FnOnce(&mut Self) -> T, - ) -> T { - let old = mem::replace(&mut self.hir_id_owner, owner); - let r = f(self); - self.hir_id_owner = old; - r - } } impl<'tcx> Visitor<'tcx> for MiscCollector<'tcx, '_, '_> { - fn visit_pat(&mut self, p: &'tcx Pat) { - if let PatKind::Paren(..) | PatKind::Rest = p.kind { - // Doesn't generate a HIR node - } else if let Some(owner) = self.hir_id_owner { - self.lctx.lower_node_id_with_owner(p.id, owner); - } - - visit::walk_pat(self, p) - } - fn visit_item(&mut self, item: &'tcx Item) { let hir_id = self.lctx.allocate_hir_id_counter(item.id); @@ -499,24 +477,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { _ => {} } - self.with_hir_id_owner(Some(item.id), |this| { - visit::walk_item(this, item); - }); + visit::walk_item(self, item); } fn visit_assoc_item(&mut self, item: &'tcx AssocItem, ctxt: AssocCtxt) { self.lctx.allocate_hir_id_counter(item.id); - let owner = match (&item.kind, ctxt) { - // Ignore patterns in trait methods without bodies. - (AssocItemKind::Fn(_, _, _, None), AssocCtxt::Trait) => None, - _ => Some(item.id), - }; - self.with_hir_id_owner(owner, |this| visit::walk_assoc_item(this, item, ctxt)); - } - - fn visit_foreign_item(&mut self, i: &'tcx ForeignItem) { - // Ignore patterns in foreign items - self.with_hir_id_owner(None, |this| visit::walk_foreign_item(this, i)); + visit::walk_assoc_item(self, item, ctxt); } fn visit_ty(&mut self, t: &'tcx Ty) { @@ -527,18 +493,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // Mirrors visit::walk_fn_decl for parameter in &f.decl.inputs { // We don't lower the ids of argument patterns - self.with_hir_id_owner(None, |this| { - this.visit_pat(¶meter.pat); - }); + self.visit_pat(¶meter.pat); self.visit_ty(¶meter.ty) } self.visit_fn_ret_ty(&f.decl.output) } TyKind::ImplTrait(def_node_id, _) => { self.lctx.allocate_hir_id_counter(def_node_id); - self.with_hir_id_owner(Some(def_node_id), |this| { - visit::walk_ty(this, t); - }); + visit::walk_ty(self, t); } _ => visit::walk_ty(self, t), } @@ -548,7 +510,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { self.lower_node_id(CRATE_NODE_ID); debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == Some(hir::CRATE_HIR_ID)); - visit::walk_crate(&mut MiscCollector { lctx: &mut self, hir_id_owner: None }, c); + visit::walk_crate(&mut MiscCollector { lctx: &mut self }, c); visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c); let module = self.lower_mod(&c.module); From 1d8c381c014f202eeae59994b9b664841e91cb72 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 23 Nov 2020 12:56:07 -0800 Subject: [PATCH 04/18] Upgrades the coverage map to Version 4 Changes the coverage map injected into binaries compiled with `-Zinstrument-coverage` to LLVM Coverage Mapping Format, Version 4 (from Version 3). Note, binaries compiled with this version will require LLVM tools from at least LLVM Version 11. --- .../src/coverageinfo/mapgen.rs | 201 +++++++++--------- .../src/coverageinfo/mod.rs | 54 ++++- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 8 +- compiler/rustc_codegen_llvm/src/llvm/mod.rs | 12 ++ .../llvm-wrapper/CoverageMappingWrapper.cpp | 47 ++-- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 2 +- .../coverage-llvmir-base/Makefile | 6 + .../filecheck.testprog.txt | 5 +- 8 files changed, 203 insertions(+), 132 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 41827a91ba4be..a6372a77a70ad 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -4,7 +4,7 @@ use crate::llvm; use llvm::coverageinfo::CounterMappingRegion; use rustc_codegen_ssa::coverageinfo::map::{Counter, CounterExpression}; -use rustc_codegen_ssa::traits::{BaseTypeMethods, ConstMethods}; +use rustc_codegen_ssa::traits::ConstMethods; use rustc_data_structures::fx::FxIndexSet; use rustc_llvm::RustString; use rustc_middle::mir::coverage::CodeRegion; @@ -38,46 +38,50 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { let mut mapgen = CoverageMapGenerator::new(); // Encode coverage mappings and generate function records - let mut function_records = Vec::<&'ll llvm::Value>::new(); - let coverage_mappings_buffer = llvm::build_byte_buffer(|coverage_mappings_buffer| { - for (instance, function_coverage) in function_coverage_map.into_iter() { - debug!("Generate coverage map for: {:?}", instance); - - let mangled_function_name = cx.tcx.symbol_name(instance).to_string(); - let function_source_hash = function_coverage.source_hash(); - let (expressions, counter_regions) = - function_coverage.get_expressions_and_counter_regions(); - - let old_len = coverage_mappings_buffer.len(); - mapgen.write_coverage_mappings(expressions, counter_regions, coverage_mappings_buffer); - let mapping_data_size = coverage_mappings_buffer.len() - old_len; - debug_assert!( - mapping_data_size > 0, - "Every `FunctionCoverage` should have at least one counter" - ); - - let function_record = mapgen.make_function_record( - cx, - mangled_function_name, - function_source_hash, - mapping_data_size, - ); - function_records.push(function_record); - } - }); + let mut function_data = Vec::new(); + for (instance, function_coverage) in function_coverage_map.into_iter() { + debug!("Generate coverage map for: {:?}", instance); + + let mangled_function_name = cx.tcx.symbol_name(instance).to_string(); + let function_source_hash = function_coverage.source_hash(); + let (expressions, counter_regions) = + function_coverage.get_expressions_and_counter_regions(); + + let coverage_mapping_buffer = llvm::build_byte_buffer(|coverage_mapping_buffer| { + mapgen.write_coverage_mapping(expressions, counter_regions, coverage_mapping_buffer); + }); + debug_assert!( + coverage_mapping_buffer.len() > 0, + "Every `FunctionCoverage` should have at least one counter" + ); + + function_data.push((mangled_function_name, function_source_hash, coverage_mapping_buffer)); + } // Encode all filenames referenced by counters/expressions in this module let filenames_buffer = llvm::build_byte_buffer(|filenames_buffer| { coverageinfo::write_filenames_section_to_buffer(&mapgen.filenames, filenames_buffer); }); + let filenames_size = filenames_buffer.len(); + let filenames_val = cx.const_bytes(&filenames_buffer[..]); + let filenames_ref = coverageinfo::hash_bytes(filenames_buffer); + // Generate the LLVM IR representation of the coverage map and store it in a well-known global - mapgen.save_generated_coverage_map( - cx, - function_records, - filenames_buffer, - coverage_mappings_buffer, - ); + let cov_data_val = mapgen.generate_coverage_map(cx, filenames_size, filenames_val); + + for (mangled_function_name, function_source_hash, coverage_mapping_buffer) in function_data { + save_function_record( + cx, + mangled_function_name, + function_source_hash, + filenames_ref, + coverage_mapping_buffer, + ); + } + + // Save the coverage data value to LLVM IR + coverageinfo::save_cov_data_to_mod(cx, cov_data_val); } struct CoverageMapGenerator { @@ -92,12 +96,12 @@ impl CoverageMapGenerator { /// Using the `expressions` and `counter_regions` collected for the current function, generate /// the `mapping_regions` and `virtual_file_mapping`, and capture any new filenames. Then use /// LLVM APIs to encode the `virtual_file_mapping`, `expressions`, and `mapping_regions` into - /// the given `coverage_mappings` byte buffer, compliant with the LLVM Coverage Mapping format. - fn write_coverage_mappings( + /// the given `coverage_mapping` byte buffer, compliant with the LLVM Coverage Mapping format. + fn write_coverage_mapping( &mut self, expressions: Vec, counter_regions: impl Iterator, - coverage_mappings_buffer: &RustString, + coverage_mapping_buffer: &RustString, ) { let mut counter_regions = counter_regions.collect::>(); if counter_regions.is_empty() { @@ -145,89 +149,78 @@ impl CoverageMapGenerator { virtual_file_mapping, expressions, mapping_regions, - coverage_mappings_buffer, + coverage_mapping_buffer, ); } - /// Generate and return the function record `Value` - fn make_function_record( - &mut self, - cx: &CodegenCx<'ll, 'tcx>, - mangled_function_name: String, - function_source_hash: u64, - mapping_data_size: usize, - ) -> &'ll llvm::Value { - let name_ref = coverageinfo::compute_hash(&mangled_function_name); - let name_ref_val = cx.const_u64(name_ref); - let mapping_data_size_val = cx.const_u32(mapping_data_size as u32); - let func_hash_val = cx.const_u64(function_source_hash); - cx.const_struct( - &[name_ref_val, mapping_data_size_val, func_hash_val], - /*packed=*/ true, - ) - } - - /// Combine the filenames and coverage mappings buffers, construct coverage map header and the - /// array of function records, and combine everything into the complete coverage map. Save the - /// coverage map data into the LLVM IR as a static global using a specific, well-known section - /// and name. - fn save_generated_coverage_map( + /// Construct coverage map header and the array of function records, and combine them into the + /// coverage map. Save the coverage map data into the LLVM IR as a static global using a + /// specific, well-known section and name. + fn generate_coverage_map( self, cx: &CodegenCx<'ll, 'tcx>, - function_records: Vec<&'ll llvm::Value>, - filenames_buffer: Vec, - mut coverage_mappings_buffer: Vec, - ) { - // Concatenate the encoded filenames and encoded coverage mappings, and add additional zero - // bytes as-needed to ensure 8-byte alignment. - let mut coverage_size = coverage_mappings_buffer.len(); - let filenames_size = filenames_buffer.len(); - let remaining_bytes = - (filenames_size + coverage_size) % coverageinfo::COVMAP_VAR_ALIGN_BYTES; - if remaining_bytes > 0 { - let pad = coverageinfo::COVMAP_VAR_ALIGN_BYTES - remaining_bytes; - coverage_mappings_buffer.append(&mut [0].repeat(pad)); - coverage_size += pad; - } - let filenames_and_coverage_mappings = [filenames_buffer, coverage_mappings_buffer].concat(); - let filenames_and_coverage_mappings_val = - cx.const_bytes(&filenames_and_coverage_mappings[..]); - + filenames_size: usize, + filenames_val: &'ll llvm::Value, + ) -> &'ll llvm::Value { debug!( - "cov map: n_records = {}, filenames_size = {}, coverage_size = {}, 0-based version = {}", - function_records.len(), + "cov map: filenames_size = {}, 0-based version = {}", filenames_size, - coverage_size, coverageinfo::mapping_version() ); - // Create the coverage data header - let n_records_val = cx.const_u32(function_records.len() as u32); + // Create the coverage data header (Note, fields 0 and 2 are now always zero, + // as of `llvm::coverage::CovMapVersion::Version4`. + let zero_was_n_records_val = cx.const_u32(0); let filenames_size_val = cx.const_u32(filenames_size as u32); - let coverage_size_val = cx.const_u32(coverage_size as u32); + let zero_was_coverage_size_val = cx.const_u32(0 as u32); let version_val = cx.const_u32(coverageinfo::mapping_version()); let cov_data_header_val = cx.const_struct( - &[n_records_val, filenames_size_val, coverage_size_val, version_val], + &[zero_was_n_records_val, filenames_size_val, zero_was_coverage_size_val, version_val], /*packed=*/ false, ); - // Create the function records array - let name_ref_from_u64 = cx.type_i64(); - let mapping_data_size_from_u32 = cx.type_i32(); - let func_hash_from_u64 = cx.type_i64(); - let function_record_ty = cx.type_struct( - &[name_ref_from_u64, mapping_data_size_from_u32, func_hash_from_u64], - /*packed=*/ true, - ); - let function_records_val = cx.const_array(function_record_ty, &function_records[..]); - // Create the complete LLVM coverage data value to add to the LLVM IR - let cov_data_val = cx.const_struct( - &[cov_data_header_val, function_records_val, filenames_and_coverage_mappings_val], - /*packed=*/ false, - ); - - // Save the coverage data value to LLVM IR - coverageinfo::save_map_to_mod(cx, cov_data_val); + cx.const_struct(&[cov_data_header_val, filenames_val], /*packed=*/ false) } } + +/// Construct a function record and combine it with the function's coverage mapping data. +/// Save the function record into the LLVM IR as a static global using a +/// specific, well-known section and name. +fn save_function_record( + cx: &CodegenCx<'ll, 'tcx>, + mangled_function_name: String, + function_source_hash: u64, + filenames_ref: u64, + coverage_mapping_buffer: Vec, +) { + // Concatenate the encoded coverage mappings + let coverage_mapping_size = coverage_mapping_buffer.len(); + let coverage_mapping_val = cx.const_bytes(&coverage_mapping_buffer[..]); + + let func_name_hash = coverageinfo::hash_str(&mangled_function_name); + let func_name_hash_val = cx.const_u64(func_name_hash); + let coverage_mapping_size_val = cx.const_u32(coverage_mapping_size as u32); + let func_hash_val = cx.const_u64(function_source_hash); + let filenames_ref_val = cx.const_u64(filenames_ref); + let func_record_val = cx.const_struct( + &[ + func_name_hash_val, + coverage_mapping_size_val, + func_hash_val, + filenames_ref_val, + coverage_mapping_val, + ], + /*packed=*/ true, + ); + + // At the present time, the coverage map for Rust assumes every instrumented function `is_used`. + // Note that Clang marks functions as "unused" in `CodeGenPGO::emitEmptyCounterMapping`. (See: + // https://github.com/rust-lang/llvm-project/blob/de02a75e398415bad4df27b4547c25b896c8bf3b/clang%2Flib%2FCodeGen%2FCodeGenPGO.cpp#L877-L878 + // for example.) + // + // It's not yet clear if or how this may be applied to Rust in the future, but the `is_used` + // argument is available and handled similarly. + let is_used = true; + coverageinfo::save_func_record_to_mod(cx, func_name_hash, func_record_val, is_used); +} diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index e21e03822ebb3..e777f363eb084 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -23,7 +23,7 @@ use tracing::debug; pub mod mapgen; -const COVMAP_VAR_ALIGN_BYTES: usize = 8; +const VAR_ALIGN_BYTES: usize = 8; /// A context object for maintaining all state needed by the coverageinfo module. pub struct CrateCoverageContext<'tcx> { @@ -177,17 +177,20 @@ pub(crate) fn write_mapping_to_buffer( ); } } +pub(crate) fn hash_str(strval: &str) -> u64 { + let strval = CString::new(strval).expect("null error converting hashable str to C string"); + unsafe { llvm::LLVMRustCoverageHashCString(strval.as_ptr()) } +} -pub(crate) fn compute_hash(name: &str) -> u64 { - let name = CString::new(name).expect("null error converting hashable name to C string"); - unsafe { llvm::LLVMRustCoverageComputeHash(name.as_ptr()) } +pub(crate) fn hash_bytes(bytes: Vec) -> u64 { + unsafe { llvm::LLVMRustCoverageHashByteArray(bytes.as_ptr().cast(), bytes.len()) } } pub(crate) fn mapping_version() -> u32 { unsafe { llvm::LLVMRustCoverageMappingVersion() } } -pub(crate) fn save_map_to_mod<'ll, 'tcx>( +pub(crate) fn save_cov_data_to_mod<'ll, 'tcx>( cx: &CodegenCx<'ll, 'tcx>, cov_data_val: &'ll llvm::Value, ) { @@ -198,7 +201,7 @@ pub(crate) fn save_map_to_mod<'ll, 'tcx>( debug!("covmap var name: {:?}", covmap_var_name); let covmap_section_name = llvm::build_string(|s| unsafe { - llvm::LLVMRustCoverageWriteSectionNameToString(cx.llmod, s); + llvm::LLVMRustCoverageWriteMapSectionNameToString(cx.llmod, s); }) .expect("Rust Coverage section name failed UTF-8 conversion"); debug!("covmap section name: {:?}", covmap_section_name); @@ -206,8 +209,43 @@ pub(crate) fn save_map_to_mod<'ll, 'tcx>( let llglobal = llvm::add_global(cx.llmod, cx.val_ty(cov_data_val), &covmap_var_name); llvm::set_initializer(llglobal, cov_data_val); llvm::set_global_constant(llglobal, true); - llvm::set_linkage(llglobal, llvm::Linkage::InternalLinkage); + llvm::set_linkage(llglobal, llvm::Linkage::PrivateLinkage); llvm::set_section(llglobal, &covmap_section_name); - llvm::set_alignment(llglobal, COVMAP_VAR_ALIGN_BYTES); + llvm::set_alignment(llglobal, VAR_ALIGN_BYTES); + cx.add_used_global(llglobal); +} + +pub(crate) fn save_func_record_to_mod<'ll, 'tcx>( + cx: &CodegenCx<'ll, 'tcx>, + func_name_hash: u64, + func_record_val: &'ll llvm::Value, + is_used: bool, +) { + // Assign a name to the function record. This is used to merge duplicates. + // + // In LLVM, a "translation unit" (effectively, a `Crate` in Rust) can describe functions that + // are included-but-not-used. If (or when) Rust generates functions that are + // included-but-not-used, note that a dummy description for a function included-but-not-used + // in a Crate can be replaced by full description provided by a different Crate. The two kinds + // of descriptions play distinct roles in LLVM IR; therefore, assign them different names (by + // appending "u" to the end of the function record var name, to prevent `linkonce_odr` merging. + let func_record_var_name = + format!("__covrec_{:X}{}", func_name_hash, if is_used { "u" } else { "" }); + debug!("function record var name: {:?}", func_record_var_name); + + let func_record_section_name = llvm::build_string(|s| unsafe { + llvm::LLVMRustCoverageWriteFuncSectionNameToString(cx.llmod, s); + }) + .expect("Rust Coverage function record section name failed UTF-8 conversion"); + debug!("function record section name: {:?}", func_record_section_name); + + let llglobal = llvm::add_global(cx.llmod, cx.val_ty(func_record_val), &func_record_var_name); + llvm::set_initializer(llglobal, func_record_val); + llvm::set_global_constant(llglobal, true); + llvm::set_linkage(llglobal, llvm::Linkage::LinkOnceODRLinkage); + llvm::set_visibility(llglobal, llvm::Visibility::Hidden); + llvm::set_section(llglobal, &func_record_section_name); + llvm::set_alignment(llglobal, VAR_ALIGN_BYTES); + llvm::set_comdat(cx.llmod, llglobal, &func_record_var_name); cx.add_used_global(llglobal); } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 8b15c8b0eb607..69a37dba0f1e6 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1791,10 +1791,14 @@ extern "C" { pub fn LLVMRustCoverageCreatePGOFuncNameVar(F: &'a Value, FuncName: *const c_char) -> &'a Value; - pub fn LLVMRustCoverageComputeHash(Name: *const c_char) -> u64; + pub fn LLVMRustCoverageHashCString(StrVal: *const c_char) -> u64; + pub fn LLVMRustCoverageHashByteArray(Bytes: *const c_char, NumBytes: size_t) -> u64; #[allow(improper_ctypes)] - pub fn LLVMRustCoverageWriteSectionNameToString(M: &Module, Str: &RustString); + pub fn LLVMRustCoverageWriteMapSectionNameToString(M: &Module, Str: &RustString); + + #[allow(improper_ctypes)] + pub fn LLVMRustCoverageWriteFuncSectionNameToString(M: &Module, Str: &RustString); #[allow(improper_ctypes)] pub fn LLVMRustCoverageWriteMappingVarNameToString(Str: &RustString); diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index 53a404ee01944..fc40065a9664e 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -220,12 +220,24 @@ pub fn set_linkage(llglobal: &Value, linkage: Linkage) { } } +pub fn set_visibility(llglobal: &Value, visibility: Visibility) { + unsafe { + LLVMRustSetVisibility(llglobal, visibility); + } +} + pub fn set_alignment(llglobal: &Value, bytes: usize) { unsafe { ffi::LLVMSetAlignment(llglobal, bytes as c_uint); } } +pub fn set_comdat(llmod: &Module, llglobal: &Value, name: &str) { + unsafe { + LLVMRustSetComdat(llmod, llglobal, name.as_ptr().cast(), name.len()); + } +} + /// Safe wrapper around `LLVMGetParam`, because segfaults are no fun. pub fn get_param(llfn: &Value, index: c_uint) -> &Value { unsafe { diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 2b1143a4ecff5..6700482f2b7bd 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -3,7 +3,6 @@ #include "llvm/ProfileData/Coverage/CoverageMappingWriter.h" #include "llvm/ProfileData/InstrProf.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/Support/LEB128.h" #include @@ -13,15 +12,14 @@ extern "C" void LLVMRustCoverageWriteFilenamesSectionToBuffer( const char* const Filenames[], size_t FilenamesLen, RustStringRef BufferOut) { - // LLVM 11's CoverageFilenamesSectionWriter uses its new `Version4` format, - // so we're manually writing the `Version3` format ourselves. - RawRustStringOstream OS(BufferOut); - encodeULEB128(FilenamesLen, OS); + SmallVector FilenameRefs; for (size_t i = 0; i < FilenamesLen; i++) { - StringRef Filename(Filenames[i]); - encodeULEB128(Filename.size(), OS); - OS << Filename; + FilenameRefs.push_back(StringRef(Filenames[i])); } + auto FilenamesWriter = coverage::CoverageFilenamesSectionWriter( + makeArrayRef(FilenameRefs)); + RawRustStringOstream OS(BufferOut); + FilenamesWriter.write(OS); } extern "C" void LLVMRustCoverageWriteMappingToBuffer( @@ -45,20 +43,37 @@ extern "C" LLVMValueRef LLVMRustCoverageCreatePGOFuncNameVar(LLVMValueRef F, con return wrap(createPGOFuncNameVar(*cast(unwrap(F)), FuncNameRef)); } -extern "C" uint64_t LLVMRustCoverageComputeHash(const char *Name) { - StringRef NameRef(Name); - return IndexedInstrProf::ComputeHash(NameRef); +extern "C" uint64_t LLVMRustCoverageHashCString(const char *StrVal) { + StringRef StrRef(StrVal); + return IndexedInstrProf::ComputeHash(StrRef); +} + +extern "C" uint64_t LLVMRustCoverageHashByteArray( + const char *Bytes, + unsigned NumBytes) { + StringRef StrRef(Bytes, NumBytes); + return IndexedInstrProf::ComputeHash(StrRef); } -extern "C" void LLVMRustCoverageWriteSectionNameToString(LLVMModuleRef M, - RustStringRef Str) { +static void WriteSectionNameToString(LLVMModuleRef M, + InstrProfSectKind SK, + RustStringRef Str) { Triple TargetTriple(unwrap(M)->getTargetTriple()); - auto name = getInstrProfSectionName(IPSK_covmap, - TargetTriple.getObjectFormat()); + auto name = getInstrProfSectionName(SK, TargetTriple.getObjectFormat()); RawRustStringOstream OS(Str); OS << name; } +extern "C" void LLVMRustCoverageWriteMapSectionNameToString(LLVMModuleRef M, + RustStringRef Str) { + WriteSectionNameToString(M, IPSK_covmap, Str); +} + +extern "C" void LLVMRustCoverageWriteFuncSectionNameToString(LLVMModuleRef M, + RustStringRef Str) { + WriteSectionNameToString(M, IPSK_covfun, Str); +} + extern "C" void LLVMRustCoverageWriteMappingVarNameToString(RustStringRef Str) { auto name = getCoverageMappingVarName(); RawRustStringOstream OS(Str); @@ -66,5 +81,5 @@ extern "C" void LLVMRustCoverageWriteMappingVarNameToString(RustStringRef Str) { } extern "C" uint32_t LLVMRustCoverageMappingVersion() { - return coverage::CovMapVersion::Version3; + return coverage::CovMapVersion::Version4; } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 9b0c176b69203..e17f933932e6a 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1462,7 +1462,7 @@ extern "C" void LLVMRustSetComdat(LLVMModuleRef M, LLVMValueRef V, const char *Name, size_t NameLen) { Triple TargetTriple(unwrap(M)->getTargetTriple()); GlobalObject *GV = unwrap(V); - if (!TargetTriple.isOSBinFormatMachO()) { + if (TargetTriple.supportsCOMDAT()) { StringRef NameRef(Name, NameLen); GV->setComdat(unwrap(M)->getOrInsertComdat(NameRef)); } diff --git a/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile b/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile index e84642922d934..b1368bdb79365 100644 --- a/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile +++ b/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile @@ -12,10 +12,12 @@ ifeq ($(UNAME),Darwin) INSTR_PROF_DATA_SUFFIX=,regular,live_support DATA_SECTION_PREFIX=__DATA, LLVM_COV_SECTION_PREFIX=__LLVM_COV, + COMDAT_IF_SUPPORTED= else INSTR_PROF_DATA_SUFFIX= DATA_SECTION_PREFIX= LLVM_COV_SECTION_PREFIX= + COMDAT_IF_SUPPORTED=, comdat endif ifeq ($(LINK_DEAD_CODE),yes) @@ -29,24 +31,28 @@ ifdef IS_WINDOWS -check-prefixes=CHECK,WINDOWS \ -DPRIVATE_GLOBAL='internal global' \ -DDEFINE_INTERNAL='$(DEFINE_INTERNAL)' \ + -DCOMDAT_IF_SUPPORTED='$(COMDAT_IF_SUPPORTED)' \ -DINSTR_PROF_DATA='.lprfd$$M' \ -DINSTR_PROF_NAME='.lprfn$$M' \ -DINSTR_PROF_CNTS='.lprfc$$M' \ -DINSTR_PROF_VALS='.lprfv$$M' \ -DINSTR_PROF_VNODES='.lprfnd$$M' \ -DINSTR_PROF_COVMAP='.lcovmap$$M' \ + -DINSTR_PROF_COVFUN='.lcovfun$$M' \ -DINSTR_PROF_ORDERFILE='.lorderfile$$M' else LLVM_FILECHECK_OPTIONS=\ -check-prefixes=CHECK \ -DPRIVATE_GLOBAL='private global' \ -DDEFINE_INTERNAL='$(DEFINE_INTERNAL)' \ + -DCOMDAT_IF_SUPPORTED='$(COMDAT_IF_SUPPORTED)' \ -DINSTR_PROF_DATA='$(DATA_SECTION_PREFIX)__llvm_prf_data$(INSTR_PROF_DATA_SUFFIX)' \ -DINSTR_PROF_NAME='$(DATA_SECTION_PREFIX)__llvm_prf_names' \ -DINSTR_PROF_CNTS='$(DATA_SECTION_PREFIX)__llvm_prf_cnts' \ -DINSTR_PROF_VALS='$(DATA_SECTION_PREFIX)__llvm_prf_vals' \ -DINSTR_PROF_VNODES='$(DATA_SECTION_PREFIX)__llvm_prf_vnds' \ -DINSTR_PROF_COVMAP='$(LLVM_COV_SECTION_PREFIX)__llvm_covmap' \ + -DINSTR_PROF_COVFUN='$(LLVM_COV_SECTION_PREFIX)__llvm_covfun' \ -DINSTR_PROF_ORDERFILE='$(DATA_SECTION_PREFIX)__llvm_orderfile' endif diff --git a/src/test/run-make-fulldeps/coverage-llvmir-base/filecheck.testprog.txt b/src/test/run-make-fulldeps/coverage-llvmir-base/filecheck.testprog.txt index bd2a2475d9e10..a312ec48e8498 100644 --- a/src/test/run-make-fulldeps/coverage-llvmir-base/filecheck.testprog.txt +++ b/src/test/run-make-fulldeps/coverage-llvmir-base/filecheck.testprog.txt @@ -3,7 +3,10 @@ WINDOWS: $__llvm_profile_runtime_user = comdat any -CHECK: @__llvm_coverage_mapping = internal constant +CHECK: @__covrec_{{[A-F0-9]+}}u = linkonce_odr hidden constant +CHECK-SAME: section "[[INSTR_PROF_COVFUN]]"[[COMDAT_IF_SUPPORTED]], align 8 + +CHECK: @__llvm_coverage_mapping = private constant CHECK-SAME: section "[[INSTR_PROF_COVMAP]]", align 8 WINDOWS: @__llvm_profile_runtime = external global i32 From 5d5dc4c9d86499854a50a54a8dab73c180f7cbbc Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 23 Nov 2020 19:11:56 -0800 Subject: [PATCH 05/18] Updated links to LLVM 11 docs and types --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 6 +++--- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 6 +++--- compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs | 8 ++++---- compiler/rustc_middle/src/mir/coverage.rs | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index a6372a77a70ad..e4a9ac7a5e866 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -15,9 +15,9 @@ use tracing::debug; /// Generates and exports the Coverage Map. /// -/// This Coverage Map complies with Coverage Mapping Format version 3 (zero-based encoded as 2), -/// as defined at [LLVM Code Coverage Mapping Format](https://github.com/rust-lang/llvm-project/blob/llvmorg-8.0.0/llvm/docs/CoverageMappingFormat.rst#llvm-code-coverage-mapping-format) -/// and published in Rust's current (July 2020) fork of LLVM. This version is supported by the +/// This Coverage Map complies with Coverage Mapping Format version 4 (zero-based encoded as 3), +/// as defined at [LLVM Code Coverage Mapping Format](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#llvm-code-coverage-mapping-format) +/// and published in Rust's current (November 2020) fork of LLVM. This version is supported by the /// LLVM coverage tools (`llvm-profdata` and `llvm-cov`) bundled with Rust's fork of LLVM. /// /// Consequently, Rust's bundled version of Clang also generates Coverage Maps compliant with diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 69a37dba0f1e6..41482d18946ad 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -642,7 +642,7 @@ pub type InlineAsmDiagHandler = unsafe extern "C" fn(&SMDiagnostic, *const c_voi pub mod coverageinfo { use super::coverage_map; - /// Aligns with [llvm::coverage::CounterMappingRegion::RegionKind](https://github.com/rust-lang/llvm-project/blob/rustc/10.0-2020-05-05/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L205-L221) + /// Aligns with [llvm::coverage::CounterMappingRegion::RegionKind](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L206-L222) #[derive(Copy, Clone, Debug)] #[repr(C)] pub enum RegionKind { @@ -665,13 +665,13 @@ pub mod coverageinfo { /// This struct provides LLVM's representation of a "CoverageMappingRegion", encoded into the /// coverage map, in accordance with the - /// [LLVM Code Coverage Mapping Format](https://github.com/rust-lang/llvm-project/blob/llvmorg-8.0.0/llvm/docs/CoverageMappingFormat.rst#llvm-code-coverage-mapping-format). + /// [LLVM Code Coverage Mapping Format](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#llvm-code-coverage-mapping-format). /// The struct composes fields representing the `Counter` type and value(s) (injected counter /// ID, or expression type and operands), the source file (an indirect index into a "filenames /// array", encoded separately), and source location (start and end positions of the represented /// code region). /// - /// Aligns with [llvm::coverage::CounterMappingRegion](https://github.com/rust-lang/llvm-project/blob/rustc/10.0-2020-05-05/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L223-L226) + /// Aligns with [llvm::coverage::CounterMappingRegion](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L224-L227) /// Important: The Rust struct layout (order and types of fields) must match its C++ /// counterpart. #[derive(Copy, Clone, Debug)] diff --git a/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs index bcac2c90fdc20..af6c476292bd1 100644 --- a/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_ssa/src/coverageinfo/ffi.rs @@ -1,6 +1,6 @@ use rustc_middle::mir::coverage::{CounterValueReference, MappedExpressionIndex}; -/// Aligns with [llvm::coverage::Counter::CounterKind](https://github.com/rust-lang/llvm-project/blob/rustc/10.0-2020-05-05/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L91) +/// Aligns with [llvm::coverage::Counter::CounterKind](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L206-L222) #[derive(Copy, Clone, Debug)] #[repr(C)] pub enum CounterKind { @@ -17,7 +17,7 @@ pub enum CounterKind { /// `instrprof.increment()`) /// * For `CounterKind::Expression`, `id` is the index into the coverage map's array of /// counter expressions. -/// Aligns with [llvm::coverage::Counter](https://github.com/rust-lang/llvm-project/blob/rustc/10.0-2020-05-05/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L98-L99) +/// Aligns with [llvm::coverage::Counter](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L99-L100) /// Important: The Rust struct layout (order and types of fields) must match its C++ counterpart. #[derive(Copy, Clone, Debug)] #[repr(C)] @@ -41,7 +41,7 @@ impl Counter { } } -/// Aligns with [llvm::coverage::CounterExpression::ExprKind](https://github.com/rust-lang/llvm-project/blob/rustc/10.0-2020-05-05/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L146) +/// Aligns with [llvm::coverage::CounterExpression::ExprKind](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L147) #[derive(Copy, Clone, Debug)] #[repr(C)] pub enum ExprKind { @@ -49,7 +49,7 @@ pub enum ExprKind { Add = 1, } -/// Aligns with [llvm::coverage::CounterExpression](https://github.com/rust-lang/llvm-project/blob/rustc/10.0-2020-05-05/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L147-L148) +/// Aligns with [llvm::coverage::CounterExpression](https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h#L148-L149) /// Important: The Rust struct layout (order and types of fields) must match its C++ /// counterpart. #[derive(Copy, Clone, Debug)] diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 6b46d7c497d3e..72050013854dd 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -21,9 +21,9 @@ rustc_index::newtype_index! { impl ExpressionOperandId { /// An expression operand for a "zero counter", as described in the following references: /// - /// * - /// * - /// * + /// * https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#counter + /// * https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#tag + /// * https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#counter-expressions /// /// This operand can be used to count two or more separate code regions with a single counter, /// if they run sequentially with no branches, by injecting the `Counter` in a `BasicBlock` for From 51268d2735ed59b4f6b6351fd75edd0f4cf19c9f Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 24 Nov 2020 11:50:24 -0800 Subject: [PATCH 06/18] Check for LLVM 11+ when using `-Z instrument-coverage` * `rustc` should now compile under LLVM 9 or 10 * Compiler generates an error if `-Z instrument-coverage` is specified but LLVM version is less than 11 * Coverage tests that require `-Z instrument-coverage` and run codegen should be skipped if LLVM version is less than 11 --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 4 ++-- .../rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp | 8 ++++++++ .../run-make-fulldeps/coverage-llvmir-base/Makefile | 10 +++++++++- .../run-make-fulldeps/coverage-reports-base/Makefile | 10 +++++++++- .../run-make-fulldeps/coverage-spanview-base/Makefile | 5 +++++ src/test/run-make-fulldeps/coverage/coverage_tools.mk | 7 +++++++ 6 files changed, 40 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index e4a9ac7a5e866..d3de960fbec84 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -39,7 +39,7 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { // Encode coverage mappings and generate function records let mut function_data = Vec::new(); - for (instance, function_coverage) in function_coverage_map.into_iter() { + for (instance, function_coverage) in function_coverage_map { debug!("Generate coverage map for: {:?}", instance); let mangled_function_name = cx.tcx.symbol_name(instance).to_string(); @@ -172,7 +172,7 @@ impl CoverageMapGenerator { // as of `llvm::coverage::CovMapVersion::Version4`. let zero_was_n_records_val = cx.const_u32(0); let filenames_size_val = cx.const_u32(filenames_size as u32); - let zero_was_coverage_size_val = cx.const_u32(0 as u32); + let zero_was_coverage_size_val = cx.const_u32(0); let version_val = cx.const_u32(coverageinfo::mapping_version()); let cov_data_header_val = cx.const_struct( &[zero_was_n_records_val, filenames_size_val, zero_was_coverage_size_val, version_val], diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 6700482f2b7bd..2b76cd5849d3e 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -71,7 +71,11 @@ extern "C" void LLVMRustCoverageWriteMapSectionNameToString(LLVMModuleRef M, extern "C" void LLVMRustCoverageWriteFuncSectionNameToString(LLVMModuleRef M, RustStringRef Str) { +#if LLVM_VERSION_GE(11, 0) WriteSectionNameToString(M, IPSK_covfun, Str); +#else + report_fatal_error("rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); +#endif } extern "C" void LLVMRustCoverageWriteMappingVarNameToString(RustStringRef Str) { @@ -81,5 +85,9 @@ extern "C" void LLVMRustCoverageWriteMappingVarNameToString(RustStringRef Str) { } extern "C" uint32_t LLVMRustCoverageMappingVersion() { +#if LLVM_VERSION_GE(11, 0) return coverage::CovMapVersion::Version4; +#else + report_fatal_error("rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); +#endif } diff --git a/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile b/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile index b1368bdb79365..2b803b95053ae 100644 --- a/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile +++ b/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile @@ -56,7 +56,14 @@ else -DINSTR_PROF_ORDERFILE='$(DATA_SECTION_PREFIX)__llvm_orderfile' endif +ifeq ($(LLVM_VERSION_11_PLUS),false) +all: test_llvm_ir +else +$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.) all: +endif + +test_llvm_ir: # Compile the test program with non-experimental coverage instrumentation, and generate LLVM IR # # Note: `-Clink-dead-code=no` disables the option, needed because the option is automatically @@ -68,4 +75,5 @@ all: -Clink-dead-code=$(LINK_DEAD_CODE) \ --emit=llvm-ir - cat "$(TMPDIR)"/testprog.ll | "$(LLVM_FILECHECK)" $(BASEDIR)/filecheck.testprog.txt $(LLVM_FILECHECK_OPTIONS) + cat "$(TMPDIR)"/testprog.ll | \ + "$(LLVM_FILECHECK)" $(BASEDIR)/filecheck.testprog.txt $(LLVM_FILECHECK_OPTIONS) diff --git a/src/test/run-make-fulldeps/coverage-reports-base/Makefile b/src/test/run-make-fulldeps/coverage-reports-base/Makefile index 1e2aa056e4008..2dac8fc2225bf 100644 --- a/src/test/run-make-fulldeps/coverage-reports-base/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports-base/Makefile @@ -18,7 +18,10 @@ SOURCEDIR=../coverage # `llvm/release_debuginfo`. Note that some CI builds disable debug assertions (by setting # `NO_LLVM_ASSERTIONS=1`), so it is not OK to fail the test, but `bless`ed test results cannot be # generated without debug assertions. -LLVM_COV_DEBUG := $(shell "$(LLVM_BIN_DIR)"/llvm-cov show --debug 2>&1 | grep -q "Unknown command line argument '--debug'"; echo $$?) +LLVM_COV_DEBUG := $(shell \ + "$(LLVM_BIN_DIR)"/llvm-cov show --debug 2>&1 | \ + grep -q "Unknown command line argument '--debug'"; \ + echo $$?) ifeq ($(LLVM_COV_DEBUG), 1) DEBUG_FLAG=--debug endif @@ -30,7 +33,12 @@ ifdef RUSTC_BLESS_TEST DEBUG_FLAG=--debug endif +ifeq ($(LLVM_VERSION_11_PLUS),true) all: $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs)) +else +$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.) +all: +endif # Ensure there are no `expected` results for tests that may have been removed or renamed .PHONY: clear_expected_if_blessed diff --git a/src/test/run-make-fulldeps/coverage-spanview-base/Makefile b/src/test/run-make-fulldeps/coverage-spanview-base/Makefile index 03ef04776a03b..9f9440340e0ed 100644 --- a/src/test/run-make-fulldeps/coverage-spanview-base/Makefile +++ b/src/test/run-make-fulldeps/coverage-spanview-base/Makefile @@ -24,7 +24,12 @@ For revisions in Pull Requests (PR): endef export SPANVIEW_HEADER +ifeq ($(LLVM_VERSION_11_PLUS),true) all: $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs)) +else +$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.) +all: +endif # Ensure there are no `expected` results for tests that may have been removed or renamed .PHONY: clear_expected_if_blessed diff --git a/src/test/run-make-fulldeps/coverage/coverage_tools.mk b/src/test/run-make-fulldeps/coverage/coverage_tools.mk index 17f7696a8cf1d..99a2e0ba9523e 100644 --- a/src/test/run-make-fulldeps/coverage/coverage_tools.mk +++ b/src/test/run-make-fulldeps/coverage/coverage_tools.mk @@ -38,6 +38,13 @@ endif UNAME = $(shell uname) +# Rust option `-Z instrument-coverage` uses LLVM Coverage Mapping Format version 4, +# which requires LLVM 11 or greater. +LLVM_VERSION_11_PLUS := $(shell \ + LLVM_VERSION=$$("$(LLVM_BIN_DIR)"/llvm-config --version) && \ + LLVM_VERSION_MAJOR=$${LLVM_VERSION/.*/} && \ + [ $$LLVM_VERSION_MAJOR -ge 11 ] && echo true || echo false) + # FIXME(richkadel): Can any of the features tested by `run-make-fulldeps/coverage-*` tests be tested # just as completely by more focused unit tests of the code logic itself, to reduce the number of # test result files generated and maintained, and to help identify specific test failures and root From 5481c1bd6dc6b6cfdf7e75676028f561a39b78b9 Mon Sep 17 00:00:00 2001 From: Arlie Davis Date: Thu, 12 Nov 2020 11:24:10 -0800 Subject: [PATCH 07/18] Move lev_distance to rustc_ast, make non-generic rustc_ast currently has a few dependencies on rustc_lexer. Ideally, an AST would not have any dependency its lexer, for minimizing unnecessarily design-time dependencies. Breaking this dependency would also have practical benefits, since modifying rustc_lexer would not trigger a rebuild of rustc_ast. This commit does not remove the rustc_ast --> rustc_lexer dependency, but it does remove one of the sources of this dependency, which is the code that handles fuzzy matching between symbol names for making suggestions in diagnostics. Since that code depends only on Symbol, it is easy to move it to rustc_span. It might even be best to move it to a separate crate, since other tools such as Cargo use the same algorithm, and have simply contain a duplicate of the code. This changes the signature of find_best_match_for_name so that it is no longer generic over its input. I checked the optimized binaries, and this function was duplicated at nearly every call site, because most call sites used short-lived iterator chains, generic over Map and such. But there's no good reason for a function like this to be generic, since all it does is immediately convert the generic input (the Iterator impl) to a concrete Vec. This has all of the costs of generics (duplicated method bodies) with no benefit. Changing find_best_match_for_name to be non-generic removed about 10KB of code from the optimized binary. I know it's a drop in the bucket, but we have to start reducing binary size, and beginning to tame over-use of generics is part of that. --- compiler/rustc_ast/src/lib.rs | 1 - compiler/rustc_interface/src/util.rs | 9 ++-- compiler/rustc_lint/src/context.rs | 4 +- compiler/rustc_resolve/src/diagnostics.rs | 4 +- compiler/rustc_resolve/src/imports.rs | 48 ++++++++++--------- .../rustc_resolve/src/late/diagnostics.rs | 9 ++-- .../util => rustc_span/src}/lev_distance.rs | 23 ++++----- .../src}/lev_distance/tests.rs | 17 +++---- compiler/rustc_span/src/lib.rs | 1 + compiler/rustc_typeck/src/astconv/errors.rs | 4 +- compiler/rustc_typeck/src/astconv/mod.rs | 8 +++- compiler/rustc_typeck/src/check/expr.rs | 28 ++++++----- .../rustc_typeck/src/check/method/probe.rs | 9 ++-- .../rustc_typeck/src/check/method/suggest.rs | 4 +- compiler/rustc_typeck/src/check/pat.rs | 7 +-- src/tools/clippy/clippy_lints/src/attrs.rs | 4 +- 16 files changed, 96 insertions(+), 84 deletions(-) rename compiler/{rustc_ast/src/util => rustc_span/src}/lev_distance.rs (86%) rename compiler/{rustc_ast/src/util => rustc_span/src}/lev_distance/tests.rs (69%) diff --git a/compiler/rustc_ast/src/lib.rs b/compiler/rustc_ast/src/lib.rs index 6e47ff7d74081..8a20dd7968590 100644 --- a/compiler/rustc_ast/src/lib.rs +++ b/compiler/rustc_ast/src/lib.rs @@ -34,7 +34,6 @@ macro_rules! unwrap_or { pub mod util { pub mod classify; pub mod comments; - pub mod lev_distance; pub mod literal; pub mod parser; } diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 20a7b47313ecf..b43cbf46d61e3 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -1,6 +1,5 @@ use rustc_ast::mut_visit::{visit_clobber, MutVisitor, *}; use rustc_ast::ptr::P; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_ast::{self as ast, AttrVec, BlockCheckMode}; use rustc_codegen_ssa::traits::CodegenBackend; use rustc_data_structures::fingerprint::Fingerprint; @@ -20,6 +19,7 @@ use rustc_session::parse::CrateConfig; use rustc_session::CrateDisambiguator; use rustc_session::{early_error, filesearch, output, DiagnosticOutput, Session}; use rustc_span::edition::Edition; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::source_map::FileLoader; use rustc_span::symbol::{sym, Symbol}; use smallvec::SmallVec; @@ -512,8 +512,11 @@ pub(crate) fn check_attr_crate_type( if let ast::MetaItemKind::NameValue(spanned) = a.meta().unwrap().kind { let span = spanned.span; - let lev_candidate = - find_best_match_for_name(CRATE_TYPES.iter().map(|(k, _)| k), n, None); + let lev_candidate = find_best_match_for_name( + &CRATE_TYPES.iter().map(|(k, _)| *k).collect::>(), + n, + None, + ); if let Some(candidate) = lev_candidate { lint_buffer.buffer_lint_with_diagnostic( lint::builtin::UNKNOWN_CRATE_TYPES, diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 4cfeb0d968b95..16563d21ff133 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -19,7 +19,6 @@ use self::TargetLint::*; use crate::levels::LintLevelsBuilder; use crate::passes::{EarlyLintPassObject, LateLintPassObject}; use rustc_ast as ast; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync; use rustc_errors::{add_elided_lifetime_in_path_suggestion, struct_span_err, Applicability}; @@ -37,6 +36,7 @@ use rustc_session::lint::BuiltinLintDiagnostics; use rustc_session::lint::{FutureIncompatibleInfo, Level, Lint, LintBuffer, LintId}; use rustc_session::Session; use rustc_session::SessionLintStore; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::{symbol::Symbol, MultiSpan, Span, DUMMY_SP}; use rustc_target::abi::LayoutOf; @@ -411,7 +411,7 @@ impl LintStore { self.by_name.keys().map(|name| Symbol::intern(&name)).collect::>(); let suggestion = find_best_match_for_name( - symbols.iter(), + &symbols, Symbol::intern(&lint_name.to_lowercase()), None, ); diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index a0d5b61e8bd1b..809de9beff625 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1,7 +1,6 @@ use std::cmp::Reverse; use std::ptr; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_ast::{self as ast, Path}; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashSet; @@ -14,6 +13,7 @@ use rustc_middle::bug; use rustc_middle::ty::{self, DefIdTree}; use rustc_session::Session; use rustc_span::hygiene::MacroKind; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::source_map::SourceMap; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, MultiSpan, Span}; @@ -716,7 +716,7 @@ impl<'a> Resolver<'a> { suggestions.sort_by_cached_key(|suggestion| suggestion.candidate.as_str()); match find_best_match_for_name( - suggestions.iter().map(|suggestion| &suggestion.candidate), + &suggestions.iter().map(|suggestion| suggestion.candidate).collect::>(), ident.name, None, ) { diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 026cf8be73801..cb1f0834ce7ac 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -10,7 +10,6 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet use crate::{NameBinding, NameBindingKind, PathResult, PrivacyError, ToNameBinding}; use rustc_ast::unwrap_or; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_ast::NodeId; use rustc_ast_lowering::ResolverAstLowering; use rustc_data_structures::fx::FxHashSet; @@ -25,6 +24,7 @@ use rustc_session::lint::builtin::{PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPOR use rustc_session::lint::BuiltinLintDiagnostics; use rustc_session::DiagnosticMessageId; use rustc_span::hygiene::ExpnId; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::{MultiSpan, Span}; @@ -1096,33 +1096,37 @@ impl<'a, 'b> ImportResolver<'a, 'b> { _ => None, }; let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter()); - let names = resolutions.filter_map(|(BindingKey { ident: i, .. }, resolution)| { - if *i == ident { - return None; - } // Never suggest the same name - match *resolution.borrow() { - NameResolution { binding: Some(name_binding), .. } => { - match name_binding.kind { - NameBindingKind::Import { binding, .. } => { - match binding.kind { - // Never suggest the name that has binding error - // i.e., the name that cannot be previously resolved - NameBindingKind::Res(Res::Err, _) => None, - _ => Some(&i.name), + let names = resolutions + .filter_map(|(BindingKey { ident: i, .. }, resolution)| { + if *i == ident { + return None; + } // Never suggest the same name + match *resolution.borrow() { + NameResolution { binding: Some(name_binding), .. } => { + match name_binding.kind { + NameBindingKind::Import { binding, .. } => { + match binding.kind { + // Never suggest the name that has binding error + // i.e., the name that cannot be previously resolved + NameBindingKind::Res(Res::Err, _) => None, + _ => Some(i.name), + } } + _ => Some(i.name), } - _ => Some(&i.name), } + NameResolution { ref single_imports, .. } + if single_imports.is_empty() => + { + None + } + _ => Some(i.name), } - NameResolution { ref single_imports, .. } if single_imports.is_empty() => { - None - } - _ => Some(&i.name), - } - }); + }) + .collect::>(); let lev_suggestion = - find_best_match_for_name(names, ident.name, None).map(|suggestion| { + find_best_match_for_name(&names, ident.name, None).map(|suggestion| { ( vec![(ident.span, suggestion.to_string())], String::from("a similar name exists in the module"), diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 2473436a91675..6ce299a941708 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -5,7 +5,6 @@ use crate::path_names_to_string; use crate::{CrateLint, Module, ModuleKind, ModuleOrUniformRoot}; use crate::{PathResult, PathSource, Segment}; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_ast::visit::FnKind; use rustc_ast::{self as ast, Expr, ExprKind, Item, ItemKind, NodeId, Path, Ty, TyKind}; use rustc_ast_pretty::pprust::path_segment_to_string; @@ -18,6 +17,7 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::PrimTy; use rustc_session::parse::feature_err; use rustc_span::hygiene::MacroKind; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP}; @@ -1206,7 +1206,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { names.sort_by_cached_key(|suggestion| suggestion.candidate.as_str()); match find_best_match_for_name( - names.iter().map(|suggestion| &suggestion.candidate), + &names.iter().map(|suggestion| suggestion.candidate).collect::>(), name, None, ) { @@ -1592,9 +1592,10 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { .bindings .iter() .filter(|(id, _)| id.span.ctxt() == label.span.ctxt()) - .map(|(id, _)| &id.name); + .map(|(id, _)| id.name) + .collect::>(); - find_best_match_for_name(names, label.name, None).map(|symbol| { + find_best_match_for_name(&names, label.name, None).map(|symbol| { // Upon finding a similar name, get the ident that it was from - the span // contained within helps make a useful diagnostic. In addition, determine // whether this candidate is within scope. diff --git a/compiler/rustc_ast/src/util/lev_distance.rs b/compiler/rustc_span/src/lev_distance.rs similarity index 86% rename from compiler/rustc_ast/src/util/lev_distance.rs rename to compiler/rustc_span/src/lev_distance.rs index 21c2c925bc479..edc6625a6ead7 100644 --- a/compiler/rustc_ast/src/util/lev_distance.rs +++ b/compiler/rustc_span/src/lev_distance.rs @@ -1,6 +1,4 @@ -// FIXME(Centril): Move to rustc_span? - -use rustc_span::symbol::Symbol; +use crate::symbol::Symbol; use std::cmp; #[cfg(test)] @@ -45,17 +43,14 @@ pub fn lev_distance(a: &str, b: &str) -> usize { /// /// Besides Levenshtein, we use case insensitive comparison to improve accuracy on an edge case with /// a lower(upper)case letters mismatch. -pub fn find_best_match_for_name<'a, T>( - iter_names: T, +#[cold] +pub fn find_best_match_for_name( + name_vec: &[Symbol], lookup: Symbol, dist: Option, -) -> Option -where - T: Iterator, -{ +) -> Option { let lookup = &lookup.as_str(); let max_dist = dist.unwrap_or_else(|| cmp::max(lookup.len(), 3) / 3); - let name_vec: Vec<&Symbol> = iter_names.collect(); let (case_insensitive_match, levenshtein_match) = name_vec .iter() @@ -83,18 +78,18 @@ where // 2. Levenshtein distance match // 3. Sorted word match if let Some(candidate) = case_insensitive_match { - Some(*candidate) + Some(candidate) } else if levenshtein_match.is_some() { - levenshtein_match.map(|(candidate, _)| *candidate) + levenshtein_match.map(|(candidate, _)| candidate) } else { find_match_by_sorted_words(name_vec, lookup) } } -fn find_match_by_sorted_words<'a>(iter_names: Vec<&'a Symbol>, lookup: &str) -> Option { +fn find_match_by_sorted_words(iter_names: &[Symbol], lookup: &str) -> Option { iter_names.iter().fold(None, |result, candidate| { if sort_by_words(&candidate.as_str()) == sort_by_words(lookup) { - Some(**candidate) + Some(*candidate) } else { result } diff --git a/compiler/rustc_ast/src/util/lev_distance/tests.rs b/compiler/rustc_span/src/lev_distance/tests.rs similarity index 69% rename from compiler/rustc_ast/src/util/lev_distance/tests.rs rename to compiler/rustc_span/src/lev_distance/tests.rs index 7ebedbcb76a36..7aa01cb8efe7e 100644 --- a/compiler/rustc_ast/src/util/lev_distance/tests.rs +++ b/compiler/rustc_span/src/lev_distance/tests.rs @@ -21,38 +21,35 @@ fn test_lev_distance() { #[test] fn test_find_best_match_for_name() { - use rustc_span::with_default_session_globals; + use crate::with_default_session_globals; with_default_session_globals(|| { let input = vec![Symbol::intern("aaab"), Symbol::intern("aaabc")]; assert_eq!( - find_best_match_for_name(input.iter(), Symbol::intern("aaaa"), None), + find_best_match_for_name(&input, Symbol::intern("aaaa"), None), Some(Symbol::intern("aaab")) ); - assert_eq!( - find_best_match_for_name(input.iter(), Symbol::intern("1111111111"), None), - None - ); + assert_eq!(find_best_match_for_name(&input, Symbol::intern("1111111111"), None), None); let input = vec![Symbol::intern("aAAA")]; assert_eq!( - find_best_match_for_name(input.iter(), Symbol::intern("AAAA"), None), + find_best_match_for_name(&input, Symbol::intern("AAAA"), None), Some(Symbol::intern("aAAA")) ); let input = vec![Symbol::intern("AAAA")]; // Returns None because `lev_distance > max_dist / 3` - assert_eq!(find_best_match_for_name(input.iter(), Symbol::intern("aaaa"), None), None); + assert_eq!(find_best_match_for_name(&input, Symbol::intern("aaaa"), None), None); let input = vec![Symbol::intern("AAAA")]; assert_eq!( - find_best_match_for_name(input.iter(), Symbol::intern("aaaa"), Some(4)), + find_best_match_for_name(&input, Symbol::intern("aaaa"), Some(4)), Some(Symbol::intern("AAAA")) ); let input = vec![Symbol::intern("a_longer_variable_name")]; assert_eq!( - find_best_match_for_name(input.iter(), Symbol::intern("a_variable_longer_name"), None), + find_best_match_for_name(&input, Symbol::intern("a_variable_longer_name"), None), Some(Symbol::intern("a_longer_variable_name")) ); }) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 0926561f4c513..11a49d1ab887d 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -34,6 +34,7 @@ use hygiene::Transparency; pub use hygiene::{DesugaringKind, ExpnData, ExpnId, ExpnKind, ForLoopLoc, MacroKind}; pub mod def_id; use def_id::{CrateNum, DefId, LOCAL_CRATE}; +pub mod lev_distance; mod span_encoding; pub use span_encoding::{Span, DUMMY_SP}; diff --git a/compiler/rustc_typeck/src/astconv/errors.rs b/compiler/rustc_typeck/src/astconv/errors.rs index 685243f54cb90..b04acd9660d45 100644 --- a/compiler/rustc_typeck/src/astconv/errors.rs +++ b/compiler/rustc_typeck/src/astconv/errors.rs @@ -1,11 +1,11 @@ use crate::astconv::AstConv; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{pluralize, struct_span_err, Applicability}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_middle::ty; use rustc_session::parse::feature_err; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::symbol::{sym, Ident}; use rustc_span::{Span, DUMMY_SP}; @@ -180,7 +180,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { .collect(); if let (Some(suggested_name), true) = ( - find_best_match_for_name(all_candidate_names.iter(), assoc_name.name, None), + find_best_match_for_name(&all_candidate_names, assoc_name.name, None), assoc_name.span != DUMMY_SP, ) { err.span_suggestion( diff --git a/compiler/rustc_typeck/src/astconv/mod.rs b/compiler/rustc_typeck/src/astconv/mod.rs index b011e26d64b9a..9b814f6b7ee66 100644 --- a/compiler/rustc_typeck/src/astconv/mod.rs +++ b/compiler/rustc_typeck/src/astconv/mod.rs @@ -13,7 +13,6 @@ use crate::errors::{ }; use crate::middle::resolve_lifetime as rl; use crate::require_c_abi_if_c_variadic; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{struct_span_err, Applicability, ErrorReported, FatalError}; use rustc_hir as hir; @@ -26,6 +25,7 @@ use rustc_middle::ty::subst::{self, InternalSubsts, Subst, SubstsRef}; use rustc_middle::ty::GenericParamDefKind; use rustc_middle::ty::{self, Const, DefIdTree, Ty, TyCtxt, TypeFoldable}; use rustc_session::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::{Span, DUMMY_SP}; use rustc_target::spec::abi; @@ -1579,7 +1579,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { let adt_def = qself_ty.ty_adt_def().expect("enum is not an ADT"); if let Some(suggested_name) = find_best_match_for_name( - adt_def.variants.iter().map(|variant| &variant.ident.name), + &adt_def + .variants + .iter() + .map(|variant| variant.ident.name) + .collect::>(), assoc_ident.name, None, ) { diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index f7f9e607a7441..26962d2222d32 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -22,7 +22,6 @@ use crate::type_error_struct; use crate::errors::{AddressOfTemporaryTaken, ReturnStmtOutsideOfFnBody, StructExprNonExhaustive}; use rustc_ast as ast; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::ErrorReported; @@ -40,6 +39,7 @@ use rustc_middle::ty::Ty; use rustc_middle::ty::TypeFoldable; use rustc_middle::ty::{AdtKind, Visibility}; use rustc_span::hygiene::DesugaringKind; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::source_map::Span; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_trait_selection::traits::{self, ObligationCauseCode}; @@ -1441,18 +1441,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { field: Symbol, skip: Vec, ) -> Option { - let names = variant.fields.iter().filter_map(|field| { - // ignore already set fields and private fields from non-local crates - if skip.iter().any(|&x| x == field.ident.name) - || (!variant.def_id.is_local() && field.vis != Visibility::Public) - { - None - } else { - Some(&field.ident.name) - } - }); + let names = variant + .fields + .iter() + .filter_map(|field| { + // ignore already set fields and private fields from non-local crates + if skip.iter().any(|&x| x == field.ident.name) + || (!variant.def_id.is_local() && field.vis != Visibility::Public) + { + None + } else { + Some(field.ident.name) + } + }) + .collect::>(); - find_best_match_for_name(names, field, None) + find_best_match_for_name(&names, field, None) } fn available_field_names(&self, variant: &'tcx ty::VariantDef) -> Vec { diff --git a/compiler/rustc_typeck/src/check/method/probe.rs b/compiler/rustc_typeck/src/check/method/probe.rs index 478f8a16169ed..39a79893b6441 100644 --- a/compiler/rustc_typeck/src/check/method/probe.rs +++ b/compiler/rustc_typeck/src/check/method/probe.rs @@ -9,7 +9,6 @@ use crate::hir::def::DefKind; use crate::hir::def_id::DefId; use rustc_ast as ast; -use rustc_ast::util::lev_distance::{find_best_match_for_name, lev_distance}; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; use rustc_hir as hir; @@ -27,6 +26,7 @@ use rustc_middle::ty::{ }; use rustc_session::lint; use rustc_span::def_id::LocalDefId; +use rustc_span::lev_distance::{find_best_match_for_name, lev_distance}; use rustc_span::{symbol::Ident, Span, Symbol, DUMMY_SP}; use rustc_trait_selection::autoderef::{self, Autoderef}; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; @@ -1538,8 +1538,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> { Ok(None) } else { let best_name = { - let names = applicable_close_candidates.iter().map(|cand| &cand.ident.name); - find_best_match_for_name(names, self.method_name.unwrap().name, None) + let names = applicable_close_candidates + .iter() + .map(|cand| cand.ident.name) + .collect::>(); + find_best_match_for_name(&names, self.method_name.unwrap().name, None) } .unwrap(); Ok(applicable_close_candidates diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 3d5ce57a491c5..a979bc470d8db 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -2,7 +2,6 @@ //! found or is otherwise invalid. use crate::check::FnCtxt; -use rustc_ast::util::lev_distance; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; @@ -17,6 +16,7 @@ use rustc_middle::ty::print::with_crate_prefix; use rustc_middle::ty::{ self, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, }; +use rustc_span::lev_distance; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{source_map, FileName, Span}; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; @@ -744,7 +744,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if actual.is_enum() { let adt_def = actual.ty_adt_def().expect("enum is not an ADT"); if let Some(suggestion) = lev_distance::find_best_match_for_name( - adt_def.variants.iter().map(|s| &s.ident.name), + &adt_def.variants.iter().map(|s| s.ident.name).collect::>(), item_name.name, None, ) { diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index a729912126e4f..bcb73e1b4e74e 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -1,7 +1,6 @@ use crate::check::FnCtxt; use rustc_ast as ast; -use rustc_ast::util::lev_distance::find_best_match_for_name; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; @@ -13,6 +12,7 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKi use rustc_middle::ty::subst::GenericArg; use rustc_middle::ty::{self, Adt, BindingMode, Ty, TypeFoldable}; use rustc_span::hygiene::DesugaringKind; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::source_map::{Span, Spanned}; use rustc_span::symbol::Ident; use rustc_trait_selection::traits::{ObligationCause, Pattern}; @@ -1302,8 +1302,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ), ); if plural == "" { - let input = unmentioned_fields.iter().map(|(_, field)| &field.name); - let suggested_name = find_best_match_for_name(input, ident.name, None); + let input = + unmentioned_fields.iter().map(|(_, field)| field.name).collect::>(); + let suggested_name = find_best_match_for_name(&input, ident.name, None); if let Some(suggested_name) = suggested_name { err.span_suggestion( ident.span, diff --git a/src/tools/clippy/clippy_lints/src/attrs.rs b/src/tools/clippy/clippy_lints/src/attrs.rs index 9a667aa61b4f5..15505fd79f4a1 100644 --- a/src/tools/clippy/clippy_lints/src/attrs.rs +++ b/src/tools/clippy/clippy_lints/src/attrs.rs @@ -5,7 +5,7 @@ use crate::utils::{ span_lint_and_sugg, span_lint_and_then, without_block_comments, }; use if_chain::if_chain; -use rustc_ast::util::lev_distance::find_best_match_for_name; +use rustc_span::lev_distance::find_best_match_for_name; use rustc_ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem}; use rustc_errors::Applicability; use rustc_hir::{ @@ -427,7 +427,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMet .map(|l| Symbol::intern(&l.name_lower())) .collect::>(); let sugg = find_best_match_for_name( - symbols.iter(), + &symbols, Symbol::intern(&format!("clippy::{}", name_lower)), None, ); From b5fef37d23f4e4471caea5508ab18da20522e51f Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 24 Nov 2020 18:34:10 -0800 Subject: [PATCH 08/18] Apply suggestions from code review Co-authored-by: Wesley Wiser --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 2 +- src/test/run-make-fulldeps/coverage-llvmir-base/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index d3de960fbec84..51baa2aafb7d8 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -169,7 +169,7 @@ impl CoverageMapGenerator { ); // Create the coverage data header (Note, fields 0 and 2 are now always zero, - // as of `llvm::coverage::CovMapVersion::Version4`. + // as of `llvm::coverage::CovMapVersion::Version4`.) let zero_was_n_records_val = cx.const_u32(0); let filenames_size_val = cx.const_u32(filenames_size as u32); let zero_was_coverage_size_val = cx.const_u32(0); diff --git a/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile b/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile index 2b803b95053ae..219ba15ad116d 100644 --- a/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile +++ b/src/test/run-make-fulldeps/coverage-llvmir-base/Makefile @@ -56,7 +56,7 @@ else -DINSTR_PROF_ORDERFILE='$(DATA_SECTION_PREFIX)__llvm_orderfile' endif -ifeq ($(LLVM_VERSION_11_PLUS),false) +ifeq ($(LLVM_VERSION_11_PLUS),true) all: test_llvm_ir else $(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.) From d61ea568848ae11ba62193fba2a9d54e8b600c59 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 25 Nov 2020 14:27:51 +0100 Subject: [PATCH 09/18] Clean up rustdoc tests by removing unnecessary features --- src/test/rustdoc-js/doc-alias-filter-out.rs | 2 -- src/test/rustdoc-js/doc-alias-filter.rs | 2 -- src/test/rustdoc-js/doc-alias-whitespace.rs | 2 -- src/test/rustdoc-js/doc-alias.rs | 2 -- .../check-doc-alias-attr-location.rs | 2 -- .../check-doc-alias-attr-location.stderr | 8 ++++---- src/test/rustdoc-ui/check-doc-alias-attr.rs | 1 - .../rustdoc-ui/check-doc-alias-attr.stderr | 18 +++++++++--------- src/test/rustdoc-ui/doc-alias-assoc-const.rs | 1 - .../rustdoc-ui/doc-alias-assoc-const.stderr | 2 +- .../rustdoc-ui/doc-test-doctest-feature.rs | 3 --- .../rustdoc-ui/doc-test-doctest-feature.stdout | 2 +- src/test/rustdoc/deprecated-future.rs | 2 -- src/test/rustdoc/deprecated.rs | 2 -- src/test/rustdoc/issue-76501.rs | 2 -- 15 files changed, 15 insertions(+), 36 deletions(-) diff --git a/src/test/rustdoc-js/doc-alias-filter-out.rs b/src/test/rustdoc-js/doc-alias-filter-out.rs index 815e8cedd16da..3f0c094794e41 100644 --- a/src/test/rustdoc-js/doc-alias-filter-out.rs +++ b/src/test/rustdoc-js/doc-alias-filter-out.rs @@ -1,4 +1,2 @@ -#![feature(doc_alias)] - #[doc(alias = "true")] pub struct Foo; diff --git a/src/test/rustdoc-js/doc-alias-filter.rs b/src/test/rustdoc-js/doc-alias-filter.rs index 8887f8c2b0149..d5227814c06a8 100644 --- a/src/test/rustdoc-js/doc-alias-filter.rs +++ b/src/test/rustdoc-js/doc-alias-filter.rs @@ -1,5 +1,3 @@ -#![feature(doc_alias)] - #[doc(alias = "true")] pub struct Foo; diff --git a/src/test/rustdoc-js/doc-alias-whitespace.rs b/src/test/rustdoc-js/doc-alias-whitespace.rs index bea3e382ae4b0..16c022c749890 100644 --- a/src/test/rustdoc-js/doc-alias-whitespace.rs +++ b/src/test/rustdoc-js/doc-alias-whitespace.rs @@ -1,4 +1,2 @@ -#![feature(doc_alias)] - #[doc(alias = "Demon Lord")] pub struct Struct; diff --git a/src/test/rustdoc-js/doc-alias.rs b/src/test/rustdoc-js/doc-alias.rs index 41caa98643cdd..750b7b757bc35 100644 --- a/src/test/rustdoc-js/doc-alias.rs +++ b/src/test/rustdoc-js/doc-alias.rs @@ -1,5 +1,3 @@ -#![feature(doc_alias)] - #[doc(alias = "StructItem")] pub struct Struct { #[doc(alias = "StructFieldItem")] diff --git a/src/test/rustdoc-ui/check-doc-alias-attr-location.rs b/src/test/rustdoc-ui/check-doc-alias-attr-location.rs index 545964c7bd61b..7de2caa189dc7 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr-location.rs +++ b/src/test/rustdoc-ui/check-doc-alias-attr-location.rs @@ -1,5 +1,3 @@ -#![feature(doc_alias)] - pub struct Bar; pub trait Foo { type X; diff --git a/src/test/rustdoc-ui/check-doc-alias-attr-location.stderr b/src/test/rustdoc-ui/check-doc-alias-attr-location.stderr index a66e9939eaf18..175626f49dcac 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr-location.stderr +++ b/src/test/rustdoc-ui/check-doc-alias-attr-location.stderr @@ -1,23 +1,23 @@ error: `#[doc(alias = "...")]` isn't allowed on extern block - --> $DIR/check-doc-alias-attr-location.rs:9:7 + --> $DIR/check-doc-alias-attr-location.rs:7:7 | LL | #[doc(alias = "foo")] | ^^^^^^^^^^^^^ error: `#[doc(alias = "...")]` isn't allowed on implementation block - --> $DIR/check-doc-alias-attr-location.rs:12:7 + --> $DIR/check-doc-alias-attr-location.rs:10:7 | LL | #[doc(alias = "bar")] | ^^^^^^^^^^^^^ error: `#[doc(alias = "...")]` isn't allowed on implementation block - --> $DIR/check-doc-alias-attr-location.rs:18:7 + --> $DIR/check-doc-alias-attr-location.rs:16:7 | LL | #[doc(alias = "foobar")] | ^^^^^^^^^^^^^^^^ error: `#[doc(alias = "...")]` isn't allowed on type alias in implementation block - --> $DIR/check-doc-alias-attr-location.rs:20:11 + --> $DIR/check-doc-alias-attr-location.rs:18:11 | LL | #[doc(alias = "assoc")] | ^^^^^^^^^^^^^^^ diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.rs b/src/test/rustdoc-ui/check-doc-alias-attr.rs index 0ca2349a43b0f..912e35f916545 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr.rs +++ b/src/test/rustdoc-ui/check-doc-alias-attr.rs @@ -1,5 +1,4 @@ #![crate_type = "lib"] -#![feature(doc_alias)] #[doc(alias = "foo")] // ok! pub struct Bar; diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.stderr b/src/test/rustdoc-ui/check-doc-alias-attr.stderr index 2c417a3bb65b5..8a729b02e72bf 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr.stderr +++ b/src/test/rustdoc-ui/check-doc-alias-attr.stderr @@ -1,35 +1,35 @@ error: doc alias attribute expects a string: #[doc(alias = "0")] - --> $DIR/check-doc-alias-attr.rs:7:7 + --> $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:8:7 + --> $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:9:7 + --> $DIR/check-doc-alias-attr.rs:8:7 | LL | #[doc(alias("bar"))] | ^^^^^^^^^^^^ error: '\"' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:10:7 + --> $DIR/check-doc-alias-attr.rs:9:7 | LL | #[doc(alias = "\"")] | ^^^^^^^^^^^^ error: '\n' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:11:7 + --> $DIR/check-doc-alias-attr.rs:10:7 | LL | #[doc(alias = "\n")] | ^^^^^^^^^^^^ error: '\n' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:12:7 + --> $DIR/check-doc-alias-attr.rs:11:7 | LL | #[doc(alias = " | _______^ @@ -37,19 +37,19 @@ LL | | ")] | |_^ error: '\t' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:14:7 + --> $DIR/check-doc-alias-attr.rs:13:7 | LL | #[doc(alias = "\t")] | ^^^^^^^^^^^^ error: `#[doc(alias = "...")]` cannot start or end with ' ' - --> $DIR/check-doc-alias-attr.rs:15:7 + --> $DIR/check-doc-alias-attr.rs:14:7 | LL | #[doc(alias = " hello")] | ^^^^^^^^^^^^^^^^ error: `#[doc(alias = "...")]` cannot start or end with ' ' - --> $DIR/check-doc-alias-attr.rs:16:7 + --> $DIR/check-doc-alias-attr.rs:15:7 | LL | #[doc(alias = "hello ")] | ^^^^^^^^^^^^^^^^ diff --git a/src/test/rustdoc-ui/doc-alias-assoc-const.rs b/src/test/rustdoc-ui/doc-alias-assoc-const.rs index 73e23c152f268..d95324734be47 100644 --- a/src/test/rustdoc-ui/doc-alias-assoc-const.rs +++ b/src/test/rustdoc-ui/doc-alias-assoc-const.rs @@ -1,4 +1,3 @@ -#![feature(doc_alias)] #![feature(trait_alias)] pub struct Foo; diff --git a/src/test/rustdoc-ui/doc-alias-assoc-const.stderr b/src/test/rustdoc-ui/doc-alias-assoc-const.stderr index 3c64548cc204d..cbca40e1364fa 100644 --- a/src/test/rustdoc-ui/doc-alias-assoc-const.stderr +++ b/src/test/rustdoc-ui/doc-alias-assoc-const.stderr @@ -1,5 +1,5 @@ error: `#[doc(alias = "...")]` isn't allowed on associated constant in trait implementation block - --> $DIR/doc-alias-assoc-const.rs:11:11 + --> $DIR/doc-alias-assoc-const.rs:10:11 | LL | #[doc(alias = "CONST_BAZ")] | ^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/rustdoc-ui/doc-test-doctest-feature.rs b/src/test/rustdoc-ui/doc-test-doctest-feature.rs index 9a79fb8838351..2798804880ad0 100644 --- a/src/test/rustdoc-ui/doc-test-doctest-feature.rs +++ b/src/test/rustdoc-ui/doc-test-doctest-feature.rs @@ -2,13 +2,10 @@ // compile-flags:--test // normalize-stdout-test: "src/test/rustdoc-ui" -> "$$DIR" -#![feature(cfg_doctest)] - // Make sure `cfg(doctest)` is set when finding doctests but not inside // the doctests. /// ``` -/// #![feature(cfg_doctest)] /// assert!(!cfg!(doctest)); /// ``` #[cfg(doctest)] diff --git a/src/test/rustdoc-ui/doc-test-doctest-feature.stdout b/src/test/rustdoc-ui/doc-test-doctest-feature.stdout index 75d29fab17d0d..b1cd74bf8520a 100644 --- a/src/test/rustdoc-ui/doc-test-doctest-feature.stdout +++ b/src/test/rustdoc-ui/doc-test-doctest-feature.stdout @@ -1,6 +1,6 @@ running 1 test -test $DIR/doc-test-doctest-feature.rs - Foo (line 10) ... ok +test $DIR/doc-test-doctest-feature.rs - Foo (line 8) ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out diff --git a/src/test/rustdoc/deprecated-future.rs b/src/test/rustdoc/deprecated-future.rs index c5248c52fb973..7db8cc6028179 100644 --- a/src/test/rustdoc/deprecated-future.rs +++ b/src/test/rustdoc/deprecated-future.rs @@ -1,5 +1,3 @@ -#![feature(deprecated)] - // @has deprecated_future/index.html '//*[@class="stab deprecated"]' \ // 'Deprecated' // @has deprecated_future/struct.S.html '//*[@class="stab deprecated"]' \ diff --git a/src/test/rustdoc/deprecated.rs b/src/test/rustdoc/deprecated.rs index 18a33438a2346..a286856b2c3c1 100644 --- a/src/test/rustdoc/deprecated.rs +++ b/src/test/rustdoc/deprecated.rs @@ -1,5 +1,3 @@ -#![feature(deprecated)] - // @has deprecated/index.html '//*[@class="docblock-short"]/span[@class="stab deprecated"]' \ // 'Deprecated' // @has - '//*[@class="docblock-short"]' 'Deprecated docs' diff --git a/src/test/rustdoc/issue-76501.rs b/src/test/rustdoc/issue-76501.rs index 605059fe0dd8d..d468f35e28003 100644 --- a/src/test/rustdoc/issue-76501.rs +++ b/src/test/rustdoc/issue-76501.rs @@ -1,5 +1,3 @@ -#![feature(const_fn)] - // @has 'issue_76501/fn.bloop.html' '//pre' 'pub const fn bloop() -> i32' /// A useless function that always returns 1. pub const fn bloop() -> i32 { From 1d587d8c4c6b7150131cbe1c421ee419dcb4c56d Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 25 Nov 2020 15:40:13 +0100 Subject: [PATCH 10/18] Fix persisted doctests on Windows / when using workspaces When using the unstable `--persist-doctests` option, Windows path separators were not escaped properly. Also when running the command in a workspace, crate files can overwrite each other. Before: `src\lib_rs_1_0\rust_out` After: `\crate_a_src_lib_rs_1_0\rust_out`, `\crate_b_src_lib_rs_1_0\rust_out` --- src/librustdoc/doctest.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 9f35e57df418b..a615701f253d3 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -754,12 +754,14 @@ impl Tester for Collector { let folder_name = filename .to_string() .chars() - .map(|c| if c == '/' || c == '.' { '_' } else { c }) + .map(|c| if c == '\\' || c == '/' || c == '.' { '_' } else { c }) .collect::(); path.push(format!( - "{name}_{line}_{number}", - name = folder_name, + "{krate}_{file}_{line}_{number}", + krate = cratename, + file = folder_name, + line = line, number = { // Increases the current test number, if this file already // exists or it creates a new entry with a test number of 0. @@ -768,7 +770,6 @@ impl Tester for Collector { .and_modify(|v| *v += 1) .or_insert(0) }, - line = line, )); std::fs::create_dir_all(&path) From b4668ecb7317fe821844d89d27a718e50c930215 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 25 Nov 2020 09:45:33 -0800 Subject: [PATCH 11/18] Improved version check --- .../src/coverageinfo/mapgen.rs | 16 +++++++++------- .../llvm-wrapper/CoverageMappingWrapper.cpp | 5 ++--- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 51baa2aafb7d8..ff65726c83793 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -26,6 +26,11 @@ use tracing::debug; /// undocumented details in Clang's implementation (that may or may not be important) were also /// replicated for Rust's Coverage Map. pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { + // Ensure LLVM supports Coverage Map Version 4 (encoded as a zero-based value: 3). + // If not, the LLVM Version must be less than 11. + let version = coverageinfo::mapping_version(); + assert_eq!(version, 3, "rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); + let function_coverage_map = match cx.coverage_context() { Some(ctx) => ctx.take_function_coverage_map(), None => return, @@ -68,7 +73,7 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { let filenames_ref = coverageinfo::hash_bytes(filenames_buffer); // Generate the LLVM IR representation of the coverage map and store it in a well-known global - let cov_data_val = mapgen.generate_coverage_map(cx, filenames_size, filenames_val); + let cov_data_val = mapgen.generate_coverage_map(cx, version, filenames_size, filenames_val); for (mangled_function_name, function_source_hash, coverage_mapping_buffer) in function_data { save_function_record( @@ -159,21 +164,18 @@ impl CoverageMapGenerator { fn generate_coverage_map( self, cx: &CodegenCx<'ll, 'tcx>, + version: u32, filenames_size: usize, filenames_val: &'ll llvm::Value, ) -> &'ll llvm::Value { - debug!( - "cov map: filenames_size = {}, 0-based version = {}", - filenames_size, - coverageinfo::mapping_version() - ); + debug!("cov map: filenames_size = {}, 0-based version = {}", filenames_size, version); // Create the coverage data header (Note, fields 0 and 2 are now always zero, // as of `llvm::coverage::CovMapVersion::Version4`.) let zero_was_n_records_val = cx.const_u32(0); let filenames_size_val = cx.const_u32(filenames_size as u32); let zero_was_coverage_size_val = cx.const_u32(0); - let version_val = cx.const_u32(coverageinfo::mapping_version()); + let version_val = cx.const_u32(version); let cov_data_header_val = cx.const_struct( &[zero_was_n_records_val, filenames_size_val, zero_was_coverage_size_val, version_val], /*packed=*/ false, diff --git a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp index 2b76cd5849d3e..25badc3f4e17b 100644 --- a/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/CoverageMappingWrapper.cpp @@ -73,8 +73,7 @@ extern "C" void LLVMRustCoverageWriteFuncSectionNameToString(LLVMModuleRef M, RustStringRef Str) { #if LLVM_VERSION_GE(11, 0) WriteSectionNameToString(M, IPSK_covfun, Str); -#else - report_fatal_error("rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); +// else do nothing; the `Version` check will abort codegen on the Rust side #endif } @@ -88,6 +87,6 @@ extern "C" uint32_t LLVMRustCoverageMappingVersion() { #if LLVM_VERSION_GE(11, 0) return coverage::CovMapVersion::Version4; #else - report_fatal_error("rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); + return coverage::CovMapVersion::Version3; #endif } From f4d99d3aeba44a7407f000edef7656fbaf44562b Mon Sep 17 00:00:00 2001 From: Nelson J Morais Date: Wed, 25 Nov 2020 19:09:11 +0000 Subject: [PATCH 12/18] fixes a word typo in librustdoc --- src/librustdoc/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index a88efba77b41c..751f230105392 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -513,7 +513,7 @@ fn main_options(options: config::Options) -> MainResult { } // need to move these items separately because we lose them by the time the closure is called, - // but we can't crates the Handler ahead of time because it's not Send + // but we can't create the Handler ahead of time because it's not Send let diag_opts = (options.error_format, options.edition, options.debugging_opts.clone()); let show_coverage = options.show_coverage; let run_check = options.run_check; From 29e8e7267576e9386a583d2464bb01162f378526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Wed, 25 Nov 2020 09:18:09 +0100 Subject: [PATCH 13/18] Fix typos --- compiler/rustc_middle/src/ty/query/plumbing.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/ty/query/plumbing.rs b/compiler/rustc_middle/src/ty/query/plumbing.rs index 9e97ed2fb2926..d0730bd121c98 100644 --- a/compiler/rustc_middle/src/ty/query/plumbing.rs +++ b/compiler/rustc_middle/src/ty/query/plumbing.rs @@ -128,7 +128,7 @@ impl<'tcx> TyCtxt<'tcx> { pub fn try_print_query_stack(handler: &Handler, num_frames: Option) { eprintln!("query stack during panic:"); - // Be careful reyling on global state here: this code is called from + // Be careful relying on global state here: this code is called from // a panic hook, which means that the global `Handler` may be in a weird // state if it was responsible for triggering the panic. let mut i = 0; @@ -507,7 +507,7 @@ macro_rules! define_queries_struct { (tcx: $tcx:tt, input: ($(([$($modifiers:tt)*] [$($attr:tt)*] [$name:ident]))*)) => { pub struct Queries<$tcx> { - /// This provides access to the incremental comilation on-disk cache for query results. + /// This provides access to the incremental compilation on-disk cache for query results. /// Do not access this directly. It is only meant to be used by /// `DepGraph::try_mark_green()` and the query infrastructure. /// This is `None` if we are not incremental compilation mode From b1df6c0e63ae1b5f42a1039e03ce77a491780147 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 25 Nov 2020 11:38:09 -0800 Subject: [PATCH 14/18] replace assert with condition and `fatal` error --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index ff65726c83793..87eada5d55706 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -29,7 +29,9 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { // Ensure LLVM supports Coverage Map Version 4 (encoded as a zero-based value: 3). // If not, the LLVM Version must be less than 11. let version = coverageinfo::mapping_version(); - assert_eq!(version, 3, "rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); + if version != 3 { + cx.tcx.sess.fatal("rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); + }``` let function_coverage_map = match cx.coverage_context() { Some(ctx) => ctx.take_function_coverage_map(), From d334f589c4d98dbc9736af033f68bd2c35cd8a79 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 25 Nov 2020 11:41:23 -0800 Subject: [PATCH 15/18] Update compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs --- compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 87eada5d55706..85aaa7e8893bf 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -31,7 +31,7 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) { let version = coverageinfo::mapping_version(); if version != 3 { cx.tcx.sess.fatal("rustc option `-Z instrument-coverage` requires LLVM 11 or higher."); - }``` + } let function_coverage_map = match cx.coverage_context() { Some(ctx) => ctx.take_function_coverage_map(), From 46a750e35b3c7231d503f462f7ee0c7d9d6dacea Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 25 Nov 2020 13:15:48 -0800 Subject: [PATCH 16/18] Fixup compiler docs The sublist was being rendered as a code block because it was indented 4 spaces. --- .../rustc_mir/src/transform/coverage/debug.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir/src/transform/coverage/debug.rs b/compiler/rustc_mir/src/transform/coverage/debug.rs index 7f1dc3844b21d..e9528557b3337 100644 --- a/compiler/rustc_mir/src/transform/coverage/debug.rs +++ b/compiler/rustc_mir/src/transform/coverage/debug.rs @@ -95,18 +95,18 @@ //! //! Depending on the values and combinations, counters can be labeled by: //! -//! * `id` - counter or expression ID (ascending counter IDs, starting at 1, or descending -//! expression IDs, starting at `u32:MAX`) -//! * `block` - the `BasicCoverageBlock` label (for example, `bcb0`) or edge label (for -//! example `bcb0->bcb1`), for counters or expressions assigned to count a -//! `BasicCoverageBlock` or edge. Intermediate expressions (not directly associated with -//! a BCB or edge) will be labeled by their expression ID, unless `operation` is also -//! specified. -//! * `operation` - applied to expressions only, labels include the left-hand-side counter -//! or expression label (lhs operand), the operator (`+` or `-`), and the right-hand-side -//! counter or expression (rhs operand). Expression operand labels are generated -//! recursively, generating labels with nested operations, enclosed in parentheses -//! (for example: `bcb2 + (bcb0 - bcb1)`). +//! * `id` - counter or expression ID (ascending counter IDs, starting at 1, or descending +//! expression IDs, starting at `u32:MAX`) +//! * `block` - the `BasicCoverageBlock` label (for example, `bcb0`) or edge label (for +//! example `bcb0->bcb1`), for counters or expressions assigned to count a +//! `BasicCoverageBlock` or edge. Intermediate expressions (not directly associated with +//! a BCB or edge) will be labeled by their expression ID, unless `operation` is also +//! specified. +//! * `operation` - applied to expressions only, labels include the left-hand-side counter +//! or expression label (lhs operand), the operator (`+` or `-`), and the right-hand-side +//! counter or expression (rhs operand). Expression operand labels are generated +//! recursively, generating labels with nested operations, enclosed in parentheses +//! (for example: `bcb2 + (bcb0 - bcb1)`). use super::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; use super::spans::CoverageSpan; From c5c70d4017b2128a3d7b7ae708e77ef1433ff53e Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 25 Nov 2020 12:05:04 -0800 Subject: [PATCH 17/18] Fix docs formatting for `thir::pattern::_match` A list was being rendered all on one line and there were other formatting issues as well. --- .../src/thir/pattern/_match.rs | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/pattern/_match.rs b/compiler/rustc_mir_build/src/thir/pattern/_match.rs index 79a74e387435c..f299663f67934 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/_match.rs @@ -1,12 +1,16 @@ //! Note: tests specific to this file can be found in: -//! - ui/pattern/usefulness -//! - ui/or-patterns -//! - ui/consts/const_in_pattern -//! - ui/rfc-2008-non-exhaustive -//! - ui/half-open-range-patterns -//! - probably many others +//! +//! - `ui/pattern/usefulness` +//! - `ui/or-patterns` +//! - `ui/consts/const_in_pattern` +//! - `ui/rfc-2008-non-exhaustive` +//! - `ui/half-open-range-patterns` +//! - probably many others +//! //! I (Nadrieril) prefer to put new tests in `ui/pattern/usefulness` unless there's a specific -//! reason not to, for example if they depend on a particular feature like or_patterns. +//! reason not to, for example if they depend on a particular feature like `or_patterns`. +//! +//! ----- //! //! This file includes the logic for exhaustiveness and usefulness checking for //! pattern-matching. Specifically, given a list of patterns for a type, we can @@ -14,8 +18,8 @@ //! (a) the patterns cover every possible constructor for the type (exhaustiveness) //! (b) each pattern is necessary (usefulness) //! -//! The algorithm implemented here is a modified version of the one described in: -//! +//! The algorithm implemented here is a modified version of the one described in +//! [this paper](http://moscova.inria.fr/~maranget/papers/warn/index.html). //! However, to save future implementors from reading the original paper, we //! summarise the algorithm here to hopefully save time and be a little clearer //! (without being so rigorous). @@ -131,18 +135,22 @@ //! //! This returns zero or more new pattern-stacks, as follows. We look at the pattern `p_1` //! on top of the stack, and we have four cases: -//! 1.1. `p_1 = c(r_1, .., r_a)`, i.e. the top of the stack has constructor `c`. We -//! push onto the stack the arguments of this constructor, and return the result: -//! r_1, .., r_a, p_2, .., p_n -//! 1.2. `p_1 = c'(r_1, .., r_a')` where `c ≠ c'`. We discard the current stack and -//! return nothing. +//! +//! 1.1. `p_1 = c(r_1, .., r_a)`, i.e. the top of the stack has constructor `c`. We +//! push onto the stack the arguments of this constructor, and return the result: +//! `r_1, .., r_a, p_2, .., p_n` +//! +//! 1.2. `p_1 = c'(r_1, .., r_a')` where `c ≠ c'`. We discard the current stack and +//! return nothing. +//! //! 1.3. `p_1 = _`. We push onto the stack as many wildcards as the constructor `c` has //! arguments (its arity), and return the resulting stack: -//! _, .., _, p_2, .., p_n +//! `_, .., _, p_2, .., p_n` +//! //! 1.4. `p_1 = r_1 | r_2`. We expand the OR-pattern and then recurse on each resulting //! stack: -//! S(c, (r_1, p_2, .., p_n)) -//! S(c, (r_2, p_2, .., p_n)) +//! - `S(c, (r_1, p_2, .., p_n))` +//! - `S(c, (r_2, p_2, .., p_n))` //! //! 2. We can pop a wildcard off the top of the stack. This is called `S(_, p)`, where `p` is //! a pattern-stack. Note: the paper calls this `D(p)`. @@ -157,8 +165,8 @@ //! p_2, .., p_n //! 2.3. `p_1 = r_1 | r_2`. We expand the OR-pattern and then recurse on each resulting //! stack. -//! S(_, (r_1, p_2, .., p_n)) -//! S(_, (r_2, p_2, .., p_n)) +//! - `S(_, (r_1, p_2, .., p_n))` +//! - `S(_, (r_2, p_2, .., p_n))` //! //! Note that the OR-patterns are not always used directly in Rust, but are used to derive the //! exhaustive integer matching rules, so they're written here for posterity. @@ -198,7 +206,7 @@ //! ] //! ``` //! -//! and `p` is [Some(false), 0], then we don't care about row 2 since we know `p` only +//! and `p` is `[Some(false), 0]`, then we don't care about row 2 since we know `p` only //! matches values that row 2 doesn't. For row 1 however, we need to dig into the //! arguments of `Some` to know whether some new value is covered. So we compute //! `U([[true, _]], [false, 0])`. @@ -222,7 +230,7 @@ //! ] //! ``` //! -//! and `p` is [_, false, _], the `Some` constructor doesn't appear in `P`. So if we +//! and `p` is `[_, false, _]`, the `Some` constructor doesn't appear in `P`. So if we //! only had row 2, we'd know that `p` is useful. However row 1 starts with a //! wildcard, so we need to check whether `U([[true, _]], [false, 1])`. //! @@ -243,7 +251,7 @@ //! ] //! ``` //! -//! and `p` is [_, false], both `None` and `Some` constructors appear in the first +//! and `p` is `[_, false]`, both `None` and `Some` constructors appear in the first //! components of `P`. We will therefore try popping both constructors in turn: we //! compute `U([[true, _]], [_, false])` for the `Some` constructor, and `U([[false]], //! [false])` for the `None` constructor. The first case returns true, so we know that @@ -294,6 +302,7 @@ //! + If some constructors are missing from the matrix, it turns out we don't need to do //! anything special (because we know none of the integers are actually wildcards: i.e., we //! can't span wildcards using ranges). + use self::Constructor::*; use self::SliceKind::*; use self::Usefulness::*; From fdbc121620704ab50bd427ff1a2bb3282ffac745 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 25 Nov 2020 13:30:33 -0800 Subject: [PATCH 18/18] fix URLs in doc comment The angle brackets were confusing my IDE and I thought they were unnecessary. I was wrong. --- compiler/rustc_middle/src/mir/coverage.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 72050013854dd..8a6bf9dff7b6f 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -21,9 +21,9 @@ rustc_index::newtype_index! { impl ExpressionOperandId { /// An expression operand for a "zero counter", as described in the following references: /// - /// * https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#counter - /// * https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#tag - /// * https://github.com/rust-lang/llvm-project/blob/rustc/11.0-2020-10-12/llvm/docs/CoverageMappingFormat.rst#counter-expressions + /// * + /// * + /// * /// /// This operand can be used to count two or more separate code regions with a single counter, /// if they run sequentially with no branches, by injecting the `Counter` in a `BasicBlock` for