From 0c88dd663a7095ccc405a2036047a90981137a51 Mon Sep 17 00:00:00 2001 From: Keno Fischer <keno@alumni.harvard.edu> Date: Tue, 23 Jun 2020 22:42:35 -0400 Subject: [PATCH 01/15] Update Box::from_raw example to generalize better I know very little about rust, so I saw this example and tried to generalize it by writing, ``` let layout = Layout::new::<T>(); let new_obj = unsafe { let ptr = alloc(layout) as *mut T; *ptr = obj; Box::from_raw(ptr) }; ``` for some more complicated `T`, which ended up crashing with SIGSEGV, because it tried to `drop_in_place` the previous object in `ptr` which is of course garbage. I also added a comment that explains why `.write` is used, but I think adding that comment is optional and may be too verbose here. I do however think that changing this example is a good idea to suggest the correct generalization. `.write` is also used in most of the rest of the documentation here, even if the example is `i32`, so it would additionally be more consistent. --- src/liballoc/boxed.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index d10cbf1afab30..02634825c2108 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -382,7 +382,10 @@ impl<T: ?Sized> Box<T> { /// /// unsafe { /// let ptr = alloc(Layout::new::<i32>()) as *mut i32; - /// *ptr = 5; + /// // In general .write is required to avoid attempting to destruct + /// // the (uninitialized) previous contents of `ptr`, though for this + /// // simple example `*ptr = 5` would have worked as well. + /// ptr.write(5); /// let x = Box::from_raw(ptr); /// } /// ``` From 2bbc2b3de42f3b14ccc8b62c2acffc8840177777 Mon Sep 17 00:00:00 2001 From: Alexis Bourget <alexis.bourget@gmail.com> Date: Thu, 25 Jun 2020 11:18:53 +0200 Subject: [PATCH 02/15] Document the static keyword --- src/libstd/keyword_docs.rs | 80 +++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index a4996d9eee810..746165beab8f5 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1030,9 +1030,85 @@ mod self_upper_keyword {} // /// A place that is valid for the duration of a program. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// A `static` item is similar to a [`const`] item in that it lives for the +/// entire duration of the program and need to have its type explicited, with a +/// `static` lifetime, outliving any other lifetime. Added to that, `static` +/// items represent a precise memory location. /// -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 +/// Static items do not call [`drop`] at the end of the program. +/// +/// There are two types of `static` items: those declared in association with +/// the [`mut`] keyword and those without. +/// +/// # Simple `static`s +/// +/// Non-[`mut`] `static` items that contain a type that is not interior mutable +/// may be placed in read-only memory. All access to a `static` item are +/// considered safe but some restrictions apply. See the [Reference] for more +/// information. +/// +/// ```rust +/// static FOO: [i32; 5] = [1, 2, 3, 4, 5]; +/// +/// let r1 = &FOO as *const _; +/// let r2 = &FOO as *const _; +/// // With a strictly read-only static, references will have the same adress +/// assert_eq!(r1, r2); +/// ``` +/// +/// # Mutable `static`s +/// +/// If a `static` item is declared with the [`mut`] keyword, then it is allowed +/// to be modified by the program. To make concurrency bugs hard to run into, +/// all access to a `static mut` require an [`unsafe`] block. Care should be +/// taken to ensure access (both read and write) are thread-safe. +/// +/// Despite their unsafety, mutable `static`s are very useful: they can be used +/// to represent global state shared by the whole program or be used in +/// [`extern`] blocks to bind to variables from C libraries. +/// +/// As global state: +/// +/// ```rust +/// # #![allow(unused_variables)] +/// # fn main() {} +/// # fn atomic_add(_: &mut u32, _: u32) -> u32 { 2 } +/// static mut LEVELS: u32 = 0; +/// +/// // This violates the idea of no shared state, and this doesn't internally +/// // protect against races, so this function is `unsafe` +/// unsafe fn bump_levels_unsafe1() -> u32 { +/// let ret = LEVELS; +/// LEVELS += 1; +/// return ret; +/// } +/// +/// // Assuming that we have an atomic_add function which returns the old value, +/// // this function is "safe" but the meaning of the return value may not be +/// // what callers expect, so it's still marked as `unsafe` +/// unsafe fn bump_levels_unsafe2() -> u32 { +/// return atomic_add(&mut LEVELS, 1); +/// } +/// ``` +/// +/// In an [`extern`] block: +/// +/// ```rust,no_run +/// # #![allow(dead_code)] +/// extern "C" { +/// static mut ERROR_MESSAGE: *mut std::os::raw::c_char; +/// } +/// ``` +/// +/// Mutable `static`s, just like simple `static`s, have some restrictions that +/// apply to them. See the [Reference] for more information. +/// +/// [`const`]: keyword.const.html +/// [`extern`]: keyword.extern.html +/// [`mut`]: keyword.mut.html +/// [`unsafe`]: keyword.unsafe.html +/// [`drop`]: mem/fn.drop.html +/// [Reference]: ../reference/items/static-items.html#static-items mod static_keyword {} #[doc(keyword = "struct")] From b71a3e1e3a20e7db3b8167d19002c372ffe69c54 Mon Sep 17 00:00:00 2001 From: Tyler Ruckinger <t.ruckinger@gmail.com> Date: Fri, 26 Jun 2020 00:43:34 -0400 Subject: [PATCH 03/15] Map ERROR_INVALID_PARAMETER to InvalidInput --- src/libstd/sys/windows/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstd/sys/windows/mod.rs b/src/libstd/sys/windows/mod.rs index 640c9f3636d4b..193ab5b47ef13 100644 --- a/src/libstd/sys/windows/mod.rs +++ b/src/libstd/sys/windows/mod.rs @@ -61,6 +61,7 @@ pub fn decode_error_kind(errno: i32) -> ErrorKind { c::ERROR_FILE_NOT_FOUND => return ErrorKind::NotFound, c::ERROR_PATH_NOT_FOUND => return ErrorKind::NotFound, c::ERROR_NO_DATA => return ErrorKind::BrokenPipe, + c::ERROR_INVALID_PARAMETER => return ErrorKind::InvalidInput, c::ERROR_SEM_TIMEOUT | c::WAIT_TIMEOUT | c::ERROR_DRIVER_CANCEL_TIMEOUT From df88972f8ce9ddbebec6d551810f7127fe25d2a3 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas <git@kazlauskas.me> Date: Sat, 27 Jun 2020 01:16:23 +0300 Subject: [PATCH 04/15] Update psm version This new version includes a fix for building on aarch64 windows. --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8cbf6cf6869ea..97b4e79a03c9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2663,9 +2663,9 @@ dependencies = [ [[package]] name = "psm" -version = "0.1.8" +version = "0.1.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "659ecfea2142a458893bb7673134bad50b752fea932349c213d6a23874ce3aa7" +checksum = "092d385624a084892d07374caa7b0994956692cf40650419a1f1a787a8d229cf" dependencies = [ "cc", ] From 765bd47fa0f0e0d5d893283a94c76e2b1009d680 Mon Sep 17 00:00:00 2001 From: Aaron Hill <aa1ronham@gmail.com> Date: Sat, 27 Jun 2020 11:35:12 -0400 Subject: [PATCH 05/15] Recover extra trailing angle brackets in struct definition This commit applies the existing 'extra angle bracket recovery' logic when parsing fields in struct definitions. This allows us to continue parsing the struct's fields, avoiding spurious 'missing field' errors in code that tries to use the struct. --- src/librustc_parse/parser/diagnostics.rs | 19 ++++++++---- src/librustc_parse/parser/expr.rs | 2 +- src/librustc_parse/parser/item.rs | 29 ++++++++++++++++++- src/librustc_parse/parser/path.rs | 2 +- .../recover-field-extra-angle-brackets.rs | 14 +++++++++ .../recover-field-extra-angle-brackets.stderr | 8 +++++ 6 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/parser/recover-field-extra-angle-brackets.rs create mode 100644 src/test/ui/parser/recover-field-extra-angle-brackets.stderr diff --git a/src/librustc_parse/parser/diagnostics.rs b/src/librustc_parse/parser/diagnostics.rs index fc9ffc3092447..4b060bf108f4e 100644 --- a/src/librustc_parse/parser/diagnostics.rs +++ b/src/librustc_parse/parser/diagnostics.rs @@ -376,7 +376,14 @@ impl<'a> Parser<'a> { /// let _ = vec![1, 2, 3].into_iter().collect::<Vec<usize>>>>(); /// ^^ help: remove extra angle brackets /// ``` - pub(super) fn check_trailing_angle_brackets(&mut self, segment: &PathSegment, end: TokenKind) { + /// + /// If `true` is returned, then trailing brackets were recovered, tokens were consumed + /// up until one of the tokens in 'end' was encountered, and an error was emitted. + pub(super) fn check_trailing_angle_brackets( + &mut self, + segment: &PathSegment, + end: &[&TokenKind], + ) -> bool { // This function is intended to be invoked after parsing a path segment where there are two // cases: // @@ -409,7 +416,7 @@ impl<'a> Parser<'a> { parsed_angle_bracket_args, ); if !parsed_angle_bracket_args { - return; + return false; } // Keep the span at the start so we can highlight the sequence of `>` characters to be @@ -447,18 +454,18 @@ impl<'a> Parser<'a> { number_of_gt, number_of_shr, ); if number_of_gt < 1 && number_of_shr < 1 { - return; + return false; } // Finally, double check that we have our end token as otherwise this is the // second case. if self.look_ahead(position, |t| { trace!("check_trailing_angle_brackets: t={:?}", t); - *t == end + end.contains(&&t.kind) }) { // Eat from where we started until the end token so that parsing can continue // as if we didn't have those extra angle brackets. - self.eat_to_tokens(&[&end]); + self.eat_to_tokens(end); let span = lo.until(self.token.span); let total_num_of_gt = number_of_gt + number_of_shr * 2; @@ -473,7 +480,9 @@ impl<'a> Parser<'a> { Applicability::MachineApplicable, ) .emit(); + return true; } + false } /// Check to see if a pair of chained operators looks like an attempt at chained comparison, diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index 2745b18a8cd51..fb38fdc26c782 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -867,7 +867,7 @@ impl<'a> Parser<'a> { let fn_span_lo = self.token.span; let segment = self.parse_path_segment(PathStyle::Expr)?; - self.check_trailing_angle_brackets(&segment, token::OpenDelim(token::Paren)); + self.check_trailing_angle_brackets(&segment, &[&token::OpenDelim(token::Paren)]); if self.check(&token::OpenDelim(token::Paren)) { // Method call `expr.f()` diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 10df16964da08..fa6264c98e4f0 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -9,7 +9,7 @@ use rustc_ast::ast::{AssocItem, AssocItemKind, ForeignItemKind, Item, ItemKind, use rustc_ast::ast::{Async, Const, Defaultness, IsAuto, Mutability, Unsafe, UseTree, UseTreeKind}; use rustc_ast::ast::{BindingMode, Block, FnDecl, FnSig, Param, SelfKind}; use rustc_ast::ast::{EnumDef, Generics, StructField, TraitRef, Ty, TyKind, Variant, VariantData}; -use rustc_ast::ast::{FnHeader, ForeignItem, PathSegment, Visibility, VisibilityKind}; +use rustc_ast::ast::{FnHeader, ForeignItem, Path, PathSegment, Visibility, VisibilityKind}; use rustc_ast::ast::{MacArgs, MacCall, MacDelimiter}; use rustc_ast::ptr::P; use rustc_ast::token::{self, TokenKind}; @@ -1262,6 +1262,25 @@ impl<'a> Parser<'a> { sp, &format!("expected `,`, or `}}`, found {}", super::token_descr(&self.token)), ); + + // Try to recover extra trailing angle brackets + let mut recovered = false; + if let TyKind::Path(_, Path { segments, .. }) = &a_var.ty.kind { + if let Some(last_segment) = segments.last() { + recovered = self.check_trailing_angle_brackets( + last_segment, + &[&token::Comma, &token::CloseDelim(token::Brace)], + ); + if recovered { + // Handle a case like `Vec<u8>>,` where we can continue parsing fields + // after the comma + self.eat(&token::Comma); + // `check_trailing_angle_brackets` already emitted a nicer error + err.cancel(); + } + } + } + if self.token.is_ident() { // This is likely another field; emit the diagnostic and keep going err.span_suggestion( @@ -1271,6 +1290,14 @@ impl<'a> Parser<'a> { Applicability::MachineApplicable, ); err.emit(); + recovered = true; + } + + if recovered { + // Make sure an error was emitted (either by recovering an angle bracket, + // or by finding an identifier as the next token), since we're + // going to continue parsing + assert!(self.sess.span_diagnostic.has_errors()); } else { return Err(err); } diff --git a/src/librustc_parse/parser/path.rs b/src/librustc_parse/parser/path.rs index 5210614548da3..67e9b3af4a8cf 100644 --- a/src/librustc_parse/parser/path.rs +++ b/src/librustc_parse/parser/path.rs @@ -169,7 +169,7 @@ impl<'a> Parser<'a> { // `PathStyle::Expr` is only provided at the root invocation and never in // `parse_path_segment` to recurse and therefore can be checked to maintain // this invariant. - self.check_trailing_angle_brackets(&segment, token::ModSep); + self.check_trailing_angle_brackets(&segment, &[&token::ModSep]); } segments.push(segment); diff --git a/src/test/ui/parser/recover-field-extra-angle-brackets.rs b/src/test/ui/parser/recover-field-extra-angle-brackets.rs new file mode 100644 index 0000000000000..5e0e00bcb5e8d --- /dev/null +++ b/src/test/ui/parser/recover-field-extra-angle-brackets.rs @@ -0,0 +1,14 @@ +// Tests that we recover from extra trailing angle brackets +// in a struct field + +struct BadStruct { + first: Vec<u8>>, //~ ERROR unmatched angle bracket + second: bool +} + +fn bar(val: BadStruct) { + val.first; + val.second; +} + +fn main() {} diff --git a/src/test/ui/parser/recover-field-extra-angle-brackets.stderr b/src/test/ui/parser/recover-field-extra-angle-brackets.stderr new file mode 100644 index 0000000000000..318e55f6e99ac --- /dev/null +++ b/src/test/ui/parser/recover-field-extra-angle-brackets.stderr @@ -0,0 +1,8 @@ +error: unmatched angle bracket + --> $DIR/recover-field-extra-angle-brackets.rs:5:19 + | +LL | first: Vec<u8>>, + | ^ help: remove extra angle bracket + +error: aborting due to previous error + From 3fc5593ea80819f940f6edef3108d15ef2ad7956 Mon Sep 17 00:00:00 2001 From: Alexis Bourget <alexis.bourget@gmail.com> Date: Sat, 27 Jun 2020 18:33:15 +0200 Subject: [PATCH 06/15] Document the type keyword --- src/libstd/keyword_docs.rs | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index d972cf6db18cf..3b493c4244de6 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1463,9 +1463,33 @@ mod true_keyword {} // /// Define an alias for an existing type. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// The syntax is `type Name = ExistingType;`. /// -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 +/// # Examples +/// +/// `type` does **not** create a new type: +/// +/// ```rust +/// type Meters = u32; +/// type Kilograms = u32; +/// +/// let m: Meters = 3; +/// let k: Kilograms = 3; +/// +/// assert_eq!(m, k); +/// ``` +/// +/// In traits, using `type` allows the usage of an associated type without +/// knowing about it when declaring the [`trait`]: +/// +/// ```rust +/// trait Iterator { +/// type Item; +/// fn next(&mut self) -> Option<Self::Item>; +/// } +/// ``` +/// +/// [`trait`]: keyword.trait.html mod type_keyword {} #[doc(keyword = "unsafe")] From 14d03702735ccca48f5b954db6c01e9cedb14f29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= <tomasz.miasko@gmail.com> Date: Sun, 28 Jun 2020 00:00:00 +0000 Subject: [PATCH 07/15] Remove defunct `-Z print-region-graph` --- src/librustc_interface/tests.rs | 1 - src/librustc_session/options.rs | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/librustc_interface/tests.rs b/src/librustc_interface/tests.rs index d861b444c8816..e35dbbc0c2f24 100644 --- a/src/librustc_interface/tests.rs +++ b/src/librustc_interface/tests.rs @@ -500,7 +500,6 @@ fn test_debugging_options_tracking_hash() { untracked!(print_link_args, true); untracked!(print_llvm_passes, true); untracked!(print_mono_items, Some(String::from("abc"))); - untracked!(print_region_graph, true); untracked!(print_type_sizes, true); untracked!(query_dep_graph, true); untracked!(query_stats, true); diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index 9337f241d7022..ed5fd6dc7028b 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -958,9 +958,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "print the LLVM optimization passes being run (default: no)"), print_mono_items: Option<String> = (None, parse_opt_string, [UNTRACKED], "print the result of the monomorphization collection pass"), - print_region_graph: bool = (false, parse_bool, [UNTRACKED], - "prints region inference graph. \ - Use with RUST_REGION_GRAPH=help for more info (default: no)"), print_type_sizes: bool = (false, parse_bool, [UNTRACKED], "print layout information for each type encountered (default: no)"), profile: bool = (false, parse_bool, [TRACKED], From 7231e575461d1246c247b2ac3e97c1adcab49737 Mon Sep 17 00:00:00 2001 From: James Box <james@jbox.dev> Date: Sat, 27 Jun 2020 22:55:42 -0500 Subject: [PATCH 08/15] Fix wording for anonymous parameter name help --- src/librustc_parse/parser/diagnostics.rs | 2 +- src/test/ui/anon-params/anon-params-denied-2018.stderr | 8 ++++---- src/test/ui/parser/inverted-parameters.rs | 2 +- src/test/ui/parser/inverted-parameters.stderr | 2 +- src/test/ui/parser/omitted-arg-in-item-fn.stderr | 2 +- src/test/ui/rfc-2565-param-attrs/param-attrs-2018.stderr | 2 +- src/test/ui/span/issue-34264.stderr | 4 ++-- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_parse/parser/diagnostics.rs b/src/librustc_parse/parser/diagnostics.rs index fc9ffc3092447..e27bbc532cfc4 100644 --- a/src/librustc_parse/parser/diagnostics.rs +++ b/src/librustc_parse/parser/diagnostics.rs @@ -1415,7 +1415,7 @@ impl<'a> Parser<'a> { if self.token != token::Lt { err.span_suggestion( pat.span, - "if this was a parameter name, give it a type", + "if this is a parameter name, give it a type", format!("{}: TypeName", ident), Applicability::HasPlaceholders, ); diff --git a/src/test/ui/anon-params/anon-params-denied-2018.stderr b/src/test/ui/anon-params/anon-params-denied-2018.stderr index e7a806a846820..840294db0830a 100644 --- a/src/test/ui/anon-params/anon-params-denied-2018.stderr +++ b/src/test/ui/anon-params/anon-params-denied-2018.stderr @@ -9,7 +9,7 @@ help: if this is a `self` type, give it a parameter name | LL | fn foo(self: i32); | ^^^^^^^^^ -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn foo(i32: TypeName); | ^^^^^^^^^^^^^ @@ -29,7 +29,7 @@ help: if this is a `self` type, give it a parameter name | LL | fn bar_with_default_impl(self: String, String) {} | ^^^^^^^^^^^^ -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn bar_with_default_impl(String: TypeName, String) {} | ^^^^^^^^^^^^^^^^ @@ -45,7 +45,7 @@ LL | fn bar_with_default_impl(String, String) {} | ^ expected one of `:`, `@`, or `|` | = note: anonymous parameters are removed in the 2018 edition (see RFC 1685) -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn bar_with_default_impl(String, String: TypeName) {} | ^^^^^^^^^^^^^^^^ @@ -61,7 +61,7 @@ LL | fn baz(a:usize, b, c: usize) -> usize { | ^ expected one of `:`, `@`, or `|` | = note: anonymous parameters are removed in the 2018 edition (see RFC 1685) -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn baz(a:usize, b: TypeName, c: usize) -> usize { | ^^^^^^^^^^^ diff --git a/src/test/ui/parser/inverted-parameters.rs b/src/test/ui/parser/inverted-parameters.rs index 6f19ee9c7dc0d..5c4272504e061 100644 --- a/src/test/ui/parser/inverted-parameters.rs +++ b/src/test/ui/parser/inverted-parameters.rs @@ -20,7 +20,7 @@ fn pattern((i32, i32) (a, b)) {} fn fizz(i32) {} //~^ ERROR expected one of `:`, `@` -//~| HELP if this was a parameter name, give it a type +//~| HELP if this is a parameter name, give it a type //~| HELP if this is a `self` type, give it a parameter name //~| HELP if this is a type, explicitly ignore the parameter name diff --git a/src/test/ui/parser/inverted-parameters.stderr b/src/test/ui/parser/inverted-parameters.stderr index 043ff65f74e1a..ae180af93e373 100644 --- a/src/test/ui/parser/inverted-parameters.stderr +++ b/src/test/ui/parser/inverted-parameters.stderr @@ -39,7 +39,7 @@ help: if this is a `self` type, give it a parameter name | LL | fn fizz(self: i32) {} | ^^^^^^^^^ -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn fizz(i32: TypeName) {} | ^^^^^^^^^^^^^ diff --git a/src/test/ui/parser/omitted-arg-in-item-fn.stderr b/src/test/ui/parser/omitted-arg-in-item-fn.stderr index 9f138bf84ce19..bc3329dcbc23d 100644 --- a/src/test/ui/parser/omitted-arg-in-item-fn.stderr +++ b/src/test/ui/parser/omitted-arg-in-item-fn.stderr @@ -9,7 +9,7 @@ help: if this is a `self` type, give it a parameter name | LL | fn foo(self: x) { | ^^^^^^^ -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn foo(x: TypeName) { | ^^^^^^^^^^^ diff --git a/src/test/ui/rfc-2565-param-attrs/param-attrs-2018.stderr b/src/test/ui/rfc-2565-param-attrs/param-attrs-2018.stderr index 1e51567a9b1c4..5516d4a4c1c1c 100644 --- a/src/test/ui/rfc-2565-param-attrs/param-attrs-2018.stderr +++ b/src/test/ui/rfc-2565-param-attrs/param-attrs-2018.stderr @@ -9,7 +9,7 @@ help: if this is a `self` type, give it a parameter name | LL | trait Trait2015 { fn foo(#[allow(C)] self: i32); } | ^^^^^^^^^ -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | trait Trait2015 { fn foo(#[allow(C)] i32: TypeName); } | ^^^^^^^^^^^^^ diff --git a/src/test/ui/span/issue-34264.stderr b/src/test/ui/span/issue-34264.stderr index 116f5ddd5b4b2..40c3219bf27b0 100644 --- a/src/test/ui/span/issue-34264.stderr +++ b/src/test/ui/span/issue-34264.stderr @@ -21,7 +21,7 @@ LL | fn foo(Option<i32>, String) {} | ^ expected one of `:`, `@`, or `|` | = note: anonymous parameters are removed in the 2018 edition (see RFC 1685) -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn foo(Option<i32>, String: TypeName) {} | ^^^^^^^^^^^^^^^^ @@ -41,7 +41,7 @@ help: if this is a `self` type, give it a parameter name | LL | fn bar(self: x, y: usize) {} | ^^^^^^^ -help: if this was a parameter name, give it a type +help: if this is a parameter name, give it a type | LL | fn bar(x: TypeName, y: usize) {} | ^^^^^^^^^^^ From 4595fa8a1baf95b0a19aae3d84e945344d8f8a44 Mon Sep 17 00:00:00 2001 From: pierwill <pierwill@users.noreply.github.com> Date: Sun, 28 Jun 2020 00:06:48 -0700 Subject: [PATCH 09/15] Fix comma in debug_assert! docs --- src/libcore/macros/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs index 3cfdde60135b7..5620d26af40a2 100644 --- a/src/libcore/macros/mod.rs +++ b/src/libcore/macros/mod.rs @@ -151,7 +151,7 @@ macro_rules! assert_ne { /// An unchecked assertion allows a program in an inconsistent state to keep /// running, which might have unexpected consequences but does not introduce /// unsafety as long as this only happens in safe code. The performance cost -/// of assertions, is however, not measurable in general. Replacing [`assert!`] +/// of assertions, however, is not measurable in general. Replacing [`assert!`] /// with `debug_assert!` is thus only encouraged after thorough profiling, and /// more importantly, only in safe code! /// From e611a3fb8423f178e856813fc1a1f2397980bd8a Mon Sep 17 00:00:00 2001 From: Alexis Bourget <alexis.bourget@gmail.com> Date: Sun, 28 Jun 2020 17:20:27 +0200 Subject: [PATCH 10/15] Apply suggestions from code review --- src/libstd/keyword_docs.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index 3b493c4244de6..089056d68f803 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1479,17 +1479,28 @@ mod true_keyword {} /// assert_eq!(m, k); /// ``` /// -/// In traits, using `type` allows the usage of an associated type without -/// knowing about it when declaring the [`trait`]: +/// In traits, `type` is used to declare an [associated type]: /// /// ```rust /// trait Iterator { +/// // associated type declaration /// type Item; /// fn next(&mut self) -> Option<Self::Item>; /// } +/// +/// struct Once<T>(Option<T>); +/// +/// impl<T> Iterator for Once<T> { +/// // associated type definition +/// type Item = T; +/// fn next(&mut self) -> Option<Self::Item> { +/// self.0.take() +/// } +/// } /// ``` /// /// [`trait`]: keyword.trait.html +/// [associated type]: ../reference/items/associated-items.html#associated-types mod type_keyword {} #[doc(keyword = "unsafe")] From dfd454bd3843c4f4dee2e943297bf3d208252dc6 Mon Sep 17 00:00:00 2001 From: Alexis Bourget <alexis.bourget@gmail.com> Date: Sun, 28 Jun 2020 20:20:32 +0200 Subject: [PATCH 11/15] Apply suggestions, reformulating some paragraphs and improving some examples --- src/libstd/keyword_docs.rs | 74 +++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index 746165beab8f5..d84ed1c93dee4 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1030,22 +1030,39 @@ mod self_upper_keyword {} // /// A place that is valid for the duration of a program. /// -/// A `static` item is similar to a [`const`] item in that it lives for the -/// entire duration of the program and need to have its type explicited, with a -/// `static` lifetime, outliving any other lifetime. Added to that, `static` -/// items represent a precise memory location. +/// A static item is a value which is valid for the entire duration of your +/// program (a `'static` lifetime). +/// +/// On the surface, `static` items seem very similar to [`const`]s: both contain +/// a value, both require type annotations and both can only be initialized with +/// constant functions and values. However, `static`s are notably different in +/// that they represent a location in memory. That means that you can have +/// references to `static` items and potentially even modify them, making them +/// essentially global variables. /// /// Static items do not call [`drop`] at the end of the program. /// /// There are two types of `static` items: those declared in association with /// the [`mut`] keyword and those without. /// +/// Items that are both static and owned cannot be moved: +/// +/// ```rust,compile_fail,E0507 +/// static VEC: Vec<u32> = vec![]; +/// +/// fn move_vec(v: Vec<u32>) -> Vec<u32> { +/// v +/// } +/// +/// move_vec(VEC); +/// ``` +/// /// # Simple `static`s /// -/// Non-[`mut`] `static` items that contain a type that is not interior mutable -/// may be placed in read-only memory. All access to a `static` item are -/// considered safe but some restrictions apply. See the [Reference] for more -/// information. +/// Accessing non-[`mut`] `static` items is considered safe, but some +/// restrictions apply. Most notably, the type of a `static` value needs to +/// implement the [`Sync`] trait, ruling out interior mutability containers +/// like [`RefCell`]. See the [Reference] for more information. /// /// ```rust /// static FOO: [i32; 5] = [1, 2, 3, 4, 5]; @@ -1054,43 +1071,22 @@ mod self_upper_keyword {} /// let r2 = &FOO as *const _; /// // With a strictly read-only static, references will have the same adress /// assert_eq!(r1, r2); +/// // A static item is used just like a variable +/// println!("{:?}", FOO); /// ``` /// /// # Mutable `static`s /// /// If a `static` item is declared with the [`mut`] keyword, then it is allowed -/// to be modified by the program. To make concurrency bugs hard to run into, -/// all access to a `static mut` require an [`unsafe`] block. Care should be -/// taken to ensure access (both read and write) are thread-safe. +/// to be modified by the program. However, accessing mutable `static`s can +/// cause undefined behavior in a number of ways, for example due to data races +/// in a multithreaded context. As such, all accesses to mutable `static`s +/// require an [`unsafe`] block. /// -/// Despite their unsafety, mutable `static`s are very useful: they can be used -/// to represent global state shared by the whole program or be used in +/// Despite their unsafety, mutable `static`s are necessary in many contexts: +/// they can be used to represent global state shared by the whole program or in /// [`extern`] blocks to bind to variables from C libraries. /// -/// As global state: -/// -/// ```rust -/// # #![allow(unused_variables)] -/// # fn main() {} -/// # fn atomic_add(_: &mut u32, _: u32) -> u32 { 2 } -/// static mut LEVELS: u32 = 0; -/// -/// // This violates the idea of no shared state, and this doesn't internally -/// // protect against races, so this function is `unsafe` -/// unsafe fn bump_levels_unsafe1() -> u32 { -/// let ret = LEVELS; -/// LEVELS += 1; -/// return ret; -/// } -/// -/// // Assuming that we have an atomic_add function which returns the old value, -/// // this function is "safe" but the meaning of the return value may not be -/// // what callers expect, so it's still marked as `unsafe` -/// unsafe fn bump_levels_unsafe2() -> u32 { -/// return atomic_add(&mut LEVELS, 1); -/// } -/// ``` -/// /// In an [`extern`] block: /// /// ```rust,no_run @@ -1108,7 +1104,9 @@ mod self_upper_keyword {} /// [`mut`]: keyword.mut.html /// [`unsafe`]: keyword.unsafe.html /// [`drop`]: mem/fn.drop.html -/// [Reference]: ../reference/items/static-items.html#static-items +/// [`Sync`]: marker/trait.Sync.html +/// [`RefCell`]: cell/struct.RefCell.html +/// [Reference]: ../reference/items/static-items.html mod static_keyword {} #[doc(keyword = "struct")] From 4224313e2bc3fc08e5eee0519d7b5813c3cad580 Mon Sep 17 00:00:00 2001 From: Alexis Bourget <alexis.bourget@gmail.com> Date: Sun, 28 Jun 2020 20:48:53 +0200 Subject: [PATCH 12/15] Fix small nits --- src/libstd/keyword_docs.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index d84ed1c93dee4..3bae720270eba 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1028,8 +1028,6 @@ mod self_upper_keyword {} #[doc(keyword = "static")] // -/// A place that is valid for the duration of a program. -/// /// A static item is a value which is valid for the entire duration of your /// program (a `'static` lifetime). /// @@ -1045,7 +1043,7 @@ mod self_upper_keyword {} /// There are two types of `static` items: those declared in association with /// the [`mut`] keyword and those without. /// -/// Items that are both static and owned cannot be moved: +/// Static items cannot be moved: /// /// ```rust,compile_fail,E0507 /// static VEC: Vec<u32> = vec![]; @@ -1054,6 +1052,7 @@ mod self_upper_keyword {} /// v /// } /// +/// // This line causes an error /// move_vec(VEC); /// ``` /// @@ -1071,7 +1070,7 @@ mod self_upper_keyword {} /// let r2 = &FOO as *const _; /// // With a strictly read-only static, references will have the same adress /// assert_eq!(r1, r2); -/// // A static item is used just like a variable +/// // A static item can be used just like a variable in many cases /// println!("{:?}", FOO); /// ``` /// From 49c1018d139746f926f1af03d1d4282e0fed7115 Mon Sep 17 00:00:00 2001 From: pierwill <pierwill@users.noreply.github.com> Date: Sun, 28 Jun 2020 13:19:59 -0700 Subject: [PATCH 13/15] Fix markdown rendering in librustc_lexer docs Use back-ticks instead of quotation marks in docs for the block comment variant of TokenKind. --- src/librustc_lexer/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_lexer/src/lib.rs b/src/librustc_lexer/src/lib.rs index 77b3d26463dfe..2f4b1bbd3ba0f 100644 --- a/src/librustc_lexer/src/lib.rs +++ b/src/librustc_lexer/src/lib.rs @@ -51,8 +51,9 @@ pub enum TokenKind { // Multi-char tokens: /// "// comment" LineComment, - /// "/* block comment */" - /// Block comments can be recursive, so the sequence like "/* /* */" + /// `/* block comment */` + /// + /// Block comments can be recursive, so the sequence like `/* /* */` /// will not be considered terminated and will result in a parsing error. BlockComment { terminated: bool }, /// Any whitespace characters sequence. From 5239a68e72d1a0c7cba2cfd219a7da911360fbb7 Mon Sep 17 00:00:00 2001 From: Rich Kadel <richkadel@google.com> Date: Sun, 21 Jun 2020 23:29:08 -0700 Subject: [PATCH 14/15] add spans to injected coverage counters added regions with counter expressions and counters. Added codegen_llvm/coverageinfo mod for upcoming coverage map Move coverage region collection to CodegenCx finalization Moved from `query coverageinfo` (renamed from `query coverage_data`), as discussed in the PR at: https://github.com/rust-lang/rust/pull/73684#issuecomment-649882503 Address merge conflict in MIR instrument_coverage test The MIR test output format changed for int types. moved debug messages out of block.rs This makes the block.rs calls to add coverage mapping data to the CodegenCx much more concise and readable. move coverage intrinsic handling into llvm impl I realized that having half of the coverage intrinsic handling in `rustc_codegen_ssa` and half in `rustc_codegen_llvm` meant that any non-llvm backend would be bound to the same decisions about how the coverage-related MIR terminators should be handled. To fix this, I moved the non-codegen portion of coverage intrinsic handling into its own trait, and implemented it in `rustc_codegen_llvm` alongside `codegen_intrinsic_call`. I also added the (required?) stubs for the new intrinsics to `IntrepretCx::emulate_intrinsic()`, to ensure calls to this function do not fail if called with these new but known intrinsics. address PR Feedback on 28 June 2020 2:48pm PDT --- src/libcore/intrinsics.rs | 35 ++++- src/librustc_codegen_llvm/base.rs | 5 + src/librustc_codegen_llvm/context.rs | 21 ++- src/librustc_codegen_llvm/coverageinfo/mod.rs | 126 +++++++++++++++++ src/librustc_codegen_llvm/intrinsic.rs | 65 ++++++++- src/librustc_codegen_llvm/lib.rs | 1 + src/librustc_codegen_ssa/coverageinfo/map.rs | 83 ++++++++++++ src/librustc_codegen_ssa/coverageinfo/mod.rs | 3 + src/librustc_codegen_ssa/lib.rs | 1 + src/librustc_codegen_ssa/mir/block.rs | 14 +- src/librustc_codegen_ssa/traits/builder.rs | 2 + .../traits/coverageinfo.rs | 35 +++++ src/librustc_codegen_ssa/traits/intrinsic.rs | 11 ++ src/librustc_codegen_ssa/traits/mod.rs | 4 + src/librustc_index/vec.rs | 3 +- src/librustc_middle/mir/coverage/mod.rs | 24 ++++ src/librustc_middle/mir/mod.rs | 28 ++-- src/librustc_middle/mir/query.rs | 14 ++ src/librustc_middle/query/mod.rs | 6 +- src/librustc_mir/interpret/intrinsics.rs | 5 +- src/librustc_mir/transform/inline.rs | 9 +- .../transform/instrument_coverage.rs | 127 ++++++++++-------- src/librustc_session/options.rs | 5 +- src/librustc_span/symbol.rs | 3 + src/librustc_typeck/check/intrinsic.rs | 12 +- .../rustc.bar.InstrumentCoverage.diff | 18 ++- .../rustc.main.InstrumentCoverage.diff | 18 ++- 27 files changed, 585 insertions(+), 93 deletions(-) create mode 100644 src/librustc_codegen_llvm/coverageinfo/mod.rs create mode 100644 src/librustc_codegen_ssa/coverageinfo/map.rs create mode 100644 src/librustc_codegen_ssa/coverageinfo/mod.rs create mode 100644 src/librustc_codegen_ssa/traits/coverageinfo.rs create mode 100644 src/librustc_middle/mir/coverage/mod.rs diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 5d09018759191..80be8c158023d 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1956,7 +1956,40 @@ extern "rust-intrinsic" { /// generation. #[cfg(not(bootstrap))] #[lang = "count_code_region"] - pub fn count_code_region(index: u32); + pub fn count_code_region(index: u32, start_byte_pos: u32, end_byte_pos: u32); + + /// Internal marker for code coverage expressions, injected into the MIR when the + /// "instrument-coverage" option is enabled. This intrinsic is not converted into a + /// backend intrinsic call, but its arguments are extracted during the production of a + /// "coverage map", which is injected into the generated code, as additional data. + /// This marker identifies a code region and two other counters or counter expressions + /// whose sum is the number of times the code region was executed. + #[cfg(not(bootstrap))] + pub fn coverage_counter_add( + index: u32, + left_index: u32, + right_index: u32, + start_byte_pos: u32, + end_byte_pos: u32, + ); + + /// This marker identifies a code region and two other counters or counter expressions + /// whose difference is the number of times the code region was executed. + /// (See `coverage_counter_add` for more information.) + #[cfg(not(bootstrap))] + pub fn coverage_counter_subtract( + index: u32, + left_index: u32, + right_index: u32, + start_byte_pos: u32, + end_byte_pos: u32, + ); + + /// This marker identifies a code region to be added to the "coverage map" to indicate source + /// code that can never be reached. + /// (See `coverage_counter_add` for more information.) + #[cfg(not(bootstrap))] + pub fn coverage_unreachable(start_byte_pos: u32, end_byte_pos: u32); /// See documentation of `<*const T>::guaranteed_eq` for details. #[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")] diff --git a/src/librustc_codegen_llvm/base.rs b/src/librustc_codegen_llvm/base.rs index e99fb8dcae1e5..d5e0d7d36ee7a 100644 --- a/src/librustc_codegen_llvm/base.rs +++ b/src/librustc_codegen_llvm/base.rs @@ -150,6 +150,11 @@ pub fn compile_codegen_unit( cx.create_used_variable() } + // Finalize code coverage by injecting the coverage map + if cx.sess().opts.debugging_opts.instrument_coverage { + cx.coverageinfo_finalize(); + } + // Finalize debuginfo if cx.sess().opts.debuginfo != DebugInfo::None { cx.debuginfo_finalize(); diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 7ff5ac5cbdc10..e9a001bab7540 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -1,5 +1,6 @@ use crate::attributes; use crate::callee::get_fn; +use crate::coverageinfo; use crate::debuginfo; use crate::llvm; use crate::llvm_util; @@ -77,6 +78,7 @@ pub struct CodegenCx<'ll, 'tcx> { pub pointee_infos: RefCell<FxHashMap<(Ty<'tcx>, Size), Option<PointeeInfo>>>, pub isize_ty: &'ll Type, + pub coverage_cx: Option<coverageinfo::CrateCoverageContext<'tcx>>, pub dbg_cx: Option<debuginfo::CrateDebugContext<'ll, 'tcx>>, eh_personality: Cell<Option<&'ll Value>>, @@ -256,6 +258,13 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { let (llcx, llmod) = (&*llvm_module.llcx, llvm_module.llmod()); + let coverage_cx = if tcx.sess.opts.debugging_opts.instrument_coverage { + let covctx = coverageinfo::CrateCoverageContext::new(); + Some(covctx) + } else { + None + }; + let dbg_cx = if tcx.sess.opts.debuginfo != DebugInfo::None { let dctx = debuginfo::CrateDebugContext::new(llmod); debuginfo::metadata::compile_unit_metadata(tcx, &codegen_unit.name().as_str(), &dctx); @@ -285,6 +294,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { scalar_lltypes: Default::default(), pointee_infos: Default::default(), isize_ty, + coverage_cx, dbg_cx, eh_personality: Cell::new(None), rust_try_fn: Cell::new(None), @@ -296,6 +306,11 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { crate fn statics_to_rauw(&self) -> &RefCell<Vec<(&'ll Value, &'ll Value)>> { &self.statics_to_rauw } + + #[inline] + pub fn coverage_context(&'a self) -> &'a coverageinfo::CrateCoverageContext<'tcx> { + self.coverage_cx.as_ref().unwrap() + } } impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> { @@ -749,8 +764,6 @@ impl CodegenCx<'b, 'tcx> { ifn!("llvm.lifetime.start.p0i8", fn(t_i64, i8p) -> void); ifn!("llvm.lifetime.end.p0i8", fn(t_i64, i8p) -> void); - ifn!("llvm.instrprof.increment", fn(i8p, t_i64, t_i32, t_i32) -> void); - ifn!("llvm.expect.i1", fn(i1, i1) -> i1); ifn!("llvm.eh.typeid.for", fn(i8p) -> t_i32); ifn!("llvm.localescape", fn(...) -> void); @@ -765,6 +778,10 @@ impl CodegenCx<'b, 'tcx> { ifn!("llvm.va_end", fn(i8p) -> void); ifn!("llvm.va_copy", fn(i8p, i8p) -> void); + if self.sess().opts.debugging_opts.instrument_coverage { + ifn!("llvm.instrprof.increment", fn(i8p, t_i64, t_i32, t_i32) -> void); + } + if self.sess().opts.debuginfo != DebugInfo::None { ifn!("llvm.dbg.declare", fn(self.type_metadata(), self.type_metadata()) -> void); ifn!("llvm.dbg.value", fn(self.type_metadata(), t_i64, self.type_metadata()) -> void); diff --git a/src/librustc_codegen_llvm/coverageinfo/mod.rs b/src/librustc_codegen_llvm/coverageinfo/mod.rs new file mode 100644 index 0000000000000..ff9f8f7aeaa54 --- /dev/null +++ b/src/librustc_codegen_llvm/coverageinfo/mod.rs @@ -0,0 +1,126 @@ +use crate::builder::Builder; +use crate::common::CodegenCx; +use log::debug; +use rustc_codegen_ssa::coverageinfo::map::*; +use rustc_codegen_ssa::traits::{CoverageInfoBuilderMethods, CoverageInfoMethods}; +use rustc_data_structures::fx::FxHashMap; +use rustc_middle::ty::Instance; + +use std::cell::RefCell; + +/// A context object for maintaining all state needed by the coverageinfo module. +pub struct CrateCoverageContext<'tcx> { + // Coverage region data for each instrumented function identified by DefId. + pub(crate) coverage_regions: RefCell<FxHashMap<Instance<'tcx>, FunctionCoverageRegions>>, +} + +impl<'tcx> CrateCoverageContext<'tcx> { + pub fn new() -> Self { + Self { coverage_regions: Default::default() } + } +} + +/// Generates and exports the Coverage Map. +// FIXME(richkadel): Actually generate and export the coverage map to LLVM. +// The current implementation is actually just debug messages to show the data is available. +pub fn finalize(cx: &CodegenCx<'_, '_>) { + let coverage_regions = &*cx.coverage_context().coverage_regions.borrow(); + for instance in coverage_regions.keys() { + let coverageinfo = cx.tcx.coverageinfo(instance.def_id()); + debug_assert!(coverageinfo.num_counters > 0); + debug!( + "Generate coverage map for: {:?}, hash: {}, num_counters: {}", + instance, coverageinfo.hash, coverageinfo.num_counters + ); + let function_coverage_regions = &coverage_regions[instance]; + for (index, region) in function_coverage_regions.indexed_regions() { + match region.kind { + CoverageKind::Counter => debug!( + " Counter {}, for {}..{}", + index, region.coverage_span.start_byte_pos, region.coverage_span.end_byte_pos + ), + CoverageKind::CounterExpression(lhs, op, rhs) => debug!( + " CounterExpression {} = {} {:?} {}, for {}..{}", + index, + lhs, + op, + rhs, + region.coverage_span.start_byte_pos, + region.coverage_span.end_byte_pos + ), + } + } + for unreachable in function_coverage_regions.unreachable_regions() { + debug!( + " Unreachable code region: {}..{}", + unreachable.start_byte_pos, unreachable.end_byte_pos + ); + } + } +} + +impl CoverageInfoMethods for CodegenCx<'ll, 'tcx> { + fn coverageinfo_finalize(&self) { + finalize(self) + } +} + +impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> { + fn add_counter_region( + &mut self, + instance: Instance<'tcx>, + index: u32, + start_byte_pos: u32, + end_byte_pos: u32, + ) { + debug!( + "adding counter to coverage map: instance={:?}, index={}, byte range {}..{}", + instance, index, start_byte_pos, end_byte_pos, + ); + let mut coverage_regions = self.coverage_context().coverage_regions.borrow_mut(); + coverage_regions.entry(instance).or_default().add_counter( + index, + start_byte_pos, + end_byte_pos, + ); + } + + fn add_counter_expression_region( + &mut self, + instance: Instance<'tcx>, + index: u32, + lhs: u32, + op: CounterOp, + rhs: u32, + start_byte_pos: u32, + end_byte_pos: u32, + ) { + debug!( + "adding counter expression to coverage map: instance={:?}, index={}, {} {:?} {}, byte range {}..{}", + instance, index, lhs, op, rhs, start_byte_pos, end_byte_pos, + ); + let mut coverage_regions = self.coverage_context().coverage_regions.borrow_mut(); + coverage_regions.entry(instance).or_default().add_counter_expression( + index, + lhs, + op, + rhs, + start_byte_pos, + end_byte_pos, + ); + } + + fn add_unreachable_region( + &mut self, + instance: Instance<'tcx>, + start_byte_pos: u32, + end_byte_pos: u32, + ) { + debug!( + "adding unreachable code to coverage map: instance={:?}, byte range {}..{}", + instance, start_byte_pos, end_byte_pos, + ); + let mut coverage_regions = self.coverage_context().coverage_regions.borrow_mut(); + coverage_regions.entry(instance).or_default().add_unreachable(start_byte_pos, end_byte_pos); + } +} diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 130c0cf1877c6..de90ac0bac1d3 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -13,12 +13,15 @@ use rustc_ast::ast; use rustc_codegen_ssa::base::{compare_simd_types, to_immediate, wants_msvc_seh}; use rustc_codegen_ssa::common::span_invalid_monomorphization_error; use rustc_codegen_ssa::common::{IntPredicate, TypeKind}; +use rustc_codegen_ssa::coverageinfo::CounterOp; use rustc_codegen_ssa::glue; use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue}; use rustc_codegen_ssa::mir::place::PlaceRef; use rustc_codegen_ssa::traits::*; use rustc_codegen_ssa::MemFlags; use rustc_hir as hir; +use rustc_middle::mir::coverage; +use rustc_middle::mir::Operand; use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt}; use rustc_middle::ty::{self, Ty}; use rustc_middle::{bug, span_bug}; @@ -81,6 +84,53 @@ fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: &str) -> Option<&'ll Valu } impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { + fn is_codegen_intrinsic( + &mut self, + intrinsic: &str, + args: &Vec<Operand<'tcx>>, + caller_instance: ty::Instance<'tcx>, + ) -> bool { + match intrinsic { + "count_code_region" => { + use coverage::count_code_region_args::*; + self.add_counter_region( + caller_instance, + op_to_u32(&args[COUNTER_INDEX]), + op_to_u32(&args[START_BYTE_POS]), + op_to_u32(&args[END_BYTE_POS]), + ); + true // Also inject the counter increment in the backend + } + "coverage_counter_add" | "coverage_counter_subtract" => { + use coverage::coverage_counter_expression_args::*; + self.add_counter_expression_region( + caller_instance, + op_to_u32(&args[COUNTER_EXPRESSION_INDEX]), + op_to_u32(&args[LEFT_INDEX]), + if intrinsic == "coverage_counter_add" { + CounterOp::Add + } else { + CounterOp::Subtract + }, + op_to_u32(&args[RIGHT_INDEX]), + op_to_u32(&args[START_BYTE_POS]), + op_to_u32(&args[END_BYTE_POS]), + ); + false // Does not inject backend code + } + "coverage_unreachable" => { + use coverage::coverage_unreachable_args::*; + self.add_unreachable_region( + caller_instance, + op_to_u32(&args[START_BYTE_POS]), + op_to_u32(&args[END_BYTE_POS]), + ); + false // Does not inject backend code + } + _ => true, // Unhandled intrinsics should be passed to `codegen_intrinsic_call()` + } + } + fn codegen_intrinsic_call( &mut self, instance: ty::Instance<'tcx>, @@ -143,15 +193,16 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { // FIXME(richkadel): The current implementation assumes the MIR for the given // caller_instance represents a single function. Validate and/or correct if inlining // and/or monomorphization invalidates these assumptions. - let coverage_data = tcx.coverage_data(caller_instance.def_id()); + let coverageinfo = tcx.coverageinfo(caller_instance.def_id()); let mangled_fn = tcx.symbol_name(caller_instance); let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); - let hash = self.const_u64(coverage_data.hash); - let num_counters = self.const_u32(coverage_data.num_counters); - let index = args[0].immediate(); + let hash = self.const_u64(coverageinfo.hash); + let num_counters = self.const_u32(coverageinfo.num_counters); + use coverage::count_code_region_args::*; + let index = args[COUNTER_INDEX].immediate(); debug!( "count_code_region to LLVM intrinsic instrprof.increment(fn_name={}, hash={:?}, num_counters={:?}, index={:?})", - mangled_fn.name, hash, num_counters, index + mangled_fn.name, hash, num_counters, index, ); self.instrprof_increment(mangled_fn_name, hash, num_counters, index) } @@ -2131,3 +2182,7 @@ fn float_type_width(ty: Ty<'_>) -> Option<u64> { _ => None, } } + +fn op_to_u32<'tcx>(op: &Operand<'tcx>) -> u32 { + Operand::scalar_from_const(op).to_u32().expect("Scalar is u32") +} diff --git a/src/librustc_codegen_llvm/lib.rs b/src/librustc_codegen_llvm/lib.rs index 55ee660d9f700..565968f9952e5 100644 --- a/src/librustc_codegen_llvm/lib.rs +++ b/src/librustc_codegen_llvm/lib.rs @@ -55,6 +55,7 @@ mod callee; mod common; mod consts; mod context; +mod coverageinfo; mod debuginfo; mod declare; mod intrinsic; diff --git a/src/librustc_codegen_ssa/coverageinfo/map.rs b/src/librustc_codegen_ssa/coverageinfo/map.rs new file mode 100644 index 0000000000000..3bd262cf2b213 --- /dev/null +++ b/src/librustc_codegen_ssa/coverageinfo/map.rs @@ -0,0 +1,83 @@ +use rustc_data_structures::fx::FxHashMap; +use std::collections::hash_map; +use std::slice; + +#[derive(Copy, Clone, Debug)] +pub enum CounterOp { + Add, + Subtract, +} + +pub enum CoverageKind { + Counter, + CounterExpression(u32, CounterOp, u32), +} + +pub struct CoverageSpan { + pub start_byte_pos: u32, + pub end_byte_pos: u32, +} + +pub struct CoverageRegion { + pub kind: CoverageKind, + pub coverage_span: CoverageSpan, +} + +/// Collects all of the coverage regions associated with (a) injected counters, (b) counter +/// expressions (additions or subtraction), and (c) unreachable regions (always counted as zero), +/// for a given Function. Counters and counter expressions are indexed because they can be operands +/// in an expression. +/// +/// Note, it's important to distinguish the `unreachable` region type from what LLVM's refers to as +/// a "gap region" (or "gap area"). A gap region is a code region within a counted region (either +/// counter or expression), but the line or lines in the gap region are not executable (such as +/// lines with only whitespace or comments). According to LLVM Code Coverage Mapping documentation, +/// "A count for a gap area is only used as the line execution count if there are no other regions +/// on a line." +#[derive(Default)] +pub struct FunctionCoverageRegions { + indexed: FxHashMap<u32, CoverageRegion>, + unreachable: Vec<CoverageSpan>, +} + +impl FunctionCoverageRegions { + pub fn add_counter(&mut self, index: u32, start_byte_pos: u32, end_byte_pos: u32) { + self.indexed.insert( + index, + CoverageRegion { + kind: CoverageKind::Counter, + coverage_span: CoverageSpan { start_byte_pos, end_byte_pos }, + }, + ); + } + + pub fn add_counter_expression( + &mut self, + index: u32, + lhs: u32, + op: CounterOp, + rhs: u32, + start_byte_pos: u32, + end_byte_pos: u32, + ) { + self.indexed.insert( + index, + CoverageRegion { + kind: CoverageKind::CounterExpression(lhs, op, rhs), + coverage_span: CoverageSpan { start_byte_pos, end_byte_pos }, + }, + ); + } + + pub fn add_unreachable(&mut self, start_byte_pos: u32, end_byte_pos: u32) { + self.unreachable.push(CoverageSpan { start_byte_pos, end_byte_pos }); + } + + pub fn indexed_regions(&self) -> hash_map::Iter<'_, u32, CoverageRegion> { + self.indexed.iter() + } + + pub fn unreachable_regions(&self) -> slice::Iter<'_, CoverageSpan> { + self.unreachable.iter() + } +} diff --git a/src/librustc_codegen_ssa/coverageinfo/mod.rs b/src/librustc_codegen_ssa/coverageinfo/mod.rs new file mode 100644 index 0000000000000..304f8e19da4e6 --- /dev/null +++ b/src/librustc_codegen_ssa/coverageinfo/mod.rs @@ -0,0 +1,3 @@ +pub mod map; + +pub use map::CounterOp; diff --git a/src/librustc_codegen_ssa/lib.rs b/src/librustc_codegen_ssa/lib.rs index bd3721850f35f..618df15f5bcbe 100644 --- a/src/librustc_codegen_ssa/lib.rs +++ b/src/librustc_codegen_ssa/lib.rs @@ -34,6 +34,7 @@ use std::path::{Path, PathBuf}; pub mod back; pub mod base; pub mod common; +pub mod coverageinfo; pub mod debuginfo; pub mod glue; pub mod meth; diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 480f9a5032087..7514eb8e889a8 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -651,6 +651,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } if intrinsic.is_some() && intrinsic != Some("drop_in_place") { + let intrinsic = intrinsic.unwrap(); + + // `is_codegen_intrinsic()` allows the backend implementation to perform compile-time + // operations before converting the `args` to backend values. + if !bx.is_codegen_intrinsic(intrinsic, &args, self.instance) { + // If the intrinsic call was fully addressed by the `is_codegen_intrinsic()` call + // (as a compile-time operation), return immediately. This avoids the need to + // convert the arguments, the call to `codegen_intrinsic_call()`, and the return + // value handling. + return; + } + let dest = match ret_dest { _ if fn_abi.ret.is_indirect() => llargs[0], ReturnDest::Nothing => { @@ -670,7 +682,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // third argument must be constant. This is // checked by const-qualification, which also // promotes any complex rvalues to constants. - if i == 2 && intrinsic.unwrap().starts_with("simd_shuffle") { + if i == 2 && intrinsic.starts_with("simd_shuffle") { if let mir::Operand::Constant(constant) = arg { let c = self.eval_mir_constant(constant); let (llval, ty) = self.simd_shuffle_indices( diff --git a/src/librustc_codegen_ssa/traits/builder.rs b/src/librustc_codegen_ssa/traits/builder.rs index 7ffc9f15bffdc..994d8e0395dc7 100644 --- a/src/librustc_codegen_ssa/traits/builder.rs +++ b/src/librustc_codegen_ssa/traits/builder.rs @@ -1,5 +1,6 @@ use super::abi::AbiBuilderMethods; use super::asm::AsmBuilderMethods; +use super::coverageinfo::CoverageInfoBuilderMethods; use super::debuginfo::DebugInfoBuilderMethods; use super::intrinsic::IntrinsicCallMethods; use super::type_::ArgAbiMethods; @@ -29,6 +30,7 @@ pub enum OverflowOp { pub trait BuilderMethods<'a, 'tcx>: HasCodegen<'tcx> + + CoverageInfoBuilderMethods<'tcx> + DebugInfoBuilderMethods + ArgAbiMethods<'tcx> + AbiBuilderMethods<'tcx> diff --git a/src/librustc_codegen_ssa/traits/coverageinfo.rs b/src/librustc_codegen_ssa/traits/coverageinfo.rs new file mode 100644 index 0000000000000..d80f90fa4fa0d --- /dev/null +++ b/src/librustc_codegen_ssa/traits/coverageinfo.rs @@ -0,0 +1,35 @@ +use super::BackendTypes; +use crate::coverageinfo::CounterOp; +use rustc_middle::ty::Instance; + +pub trait CoverageInfoMethods: BackendTypes { + fn coverageinfo_finalize(&self); +} + +pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes { + fn add_counter_region( + &mut self, + instance: Instance<'tcx>, + index: u32, + start_byte_pos: u32, + end_byte_pos: u32, + ); + + fn add_counter_expression_region( + &mut self, + instance: Instance<'tcx>, + index: u32, + lhs: u32, + op: CounterOp, + rhs: u32, + start_byte_pos: u32, + end_byte_pos: u32, + ); + + fn add_unreachable_region( + &mut self, + instance: Instance<'tcx>, + start_byte_pos: u32, + end_byte_pos: u32, + ); +} diff --git a/src/librustc_codegen_ssa/traits/intrinsic.rs b/src/librustc_codegen_ssa/traits/intrinsic.rs index f62019498511c..e713cc948c10d 100644 --- a/src/librustc_codegen_ssa/traits/intrinsic.rs +++ b/src/librustc_codegen_ssa/traits/intrinsic.rs @@ -1,5 +1,6 @@ use super::BackendTypes; use crate::mir::operand::OperandRef; +use rustc_middle::mir::Operand; use rustc_middle::ty::{self, Ty}; use rustc_span::Span; use rustc_target::abi::call::FnAbi; @@ -18,6 +19,16 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes { caller_instance: ty::Instance<'tcx>, ); + /// Intrinsic-specific pre-codegen processing, if any is required. Some intrinsics are handled + /// at compile time and do not generate code. Returns true if codegen is required or false if + /// the intrinsic does not need code generation. + fn is_codegen_intrinsic( + &mut self, + intrinsic: &str, + args: &Vec<Operand<'tcx>>, + caller_instance: ty::Instance<'tcx>, + ) -> bool; + fn abort(&mut self); fn assume(&mut self, val: Self::Value); fn expect(&mut self, cond: Self::Value, expected: bool) -> Self::Value; diff --git a/src/librustc_codegen_ssa/traits/mod.rs b/src/librustc_codegen_ssa/traits/mod.rs index 6b782731d535c..0ac519dd0b17c 100644 --- a/src/librustc_codegen_ssa/traits/mod.rs +++ b/src/librustc_codegen_ssa/traits/mod.rs @@ -19,6 +19,7 @@ mod asm; mod backend; mod builder; mod consts; +mod coverageinfo; mod debuginfo; mod declare; mod intrinsic; @@ -32,6 +33,7 @@ pub use self::asm::{AsmBuilderMethods, AsmMethods, InlineAsmOperandRef}; pub use self::backend::{Backend, BackendTypes, CodegenBackend, ExtraBackendMethods}; pub use self::builder::{BuilderMethods, OverflowOp}; pub use self::consts::ConstMethods; +pub use self::coverageinfo::{CoverageInfoBuilderMethods, CoverageInfoMethods}; pub use self::debuginfo::{DebugInfoBuilderMethods, DebugInfoMethods}; pub use self::declare::{DeclareMethods, PreDefineMethods}; pub use self::intrinsic::IntrinsicCallMethods; @@ -56,6 +58,7 @@ pub trait CodegenMethods<'tcx>: + MiscMethods<'tcx> + ConstMethods<'tcx> + StaticMethods + + CoverageInfoMethods + DebugInfoMethods<'tcx> + DeclareMethods<'tcx> + AsmMethods @@ -72,6 +75,7 @@ impl<'tcx, T> CodegenMethods<'tcx> for T where + MiscMethods<'tcx> + ConstMethods<'tcx> + StaticMethods + + CoverageInfoMethods + DebugInfoMethods<'tcx> + DeclareMethods<'tcx> + AsmMethods diff --git a/src/librustc_index/vec.rs b/src/librustc_index/vec.rs index 4dde33283f575..c5dedab979326 100644 --- a/src/librustc_index/vec.rs +++ b/src/librustc_index/vec.rs @@ -536,7 +536,8 @@ impl<I: Idx, T> IndexVec<I, T> { } /// Create an `IndexVec` with `n` elements, where the value of each - /// element is the result of `func(i)` + /// element is the result of `func(i)`. (The underlying vector will + /// be allocated only once, with a capacity of at least `n`.) #[inline] pub fn from_fn_n(func: impl FnMut(I) -> T, n: usize) -> Self { let indices = (0..n).map(I::new); diff --git a/src/librustc_middle/mir/coverage/mod.rs b/src/librustc_middle/mir/coverage/mod.rs new file mode 100644 index 0000000000000..327f321bd7534 --- /dev/null +++ b/src/librustc_middle/mir/coverage/mod.rs @@ -0,0 +1,24 @@ +//! Metadata from source code coverage analysis and instrumentation. + +/// Positional arguments to `libcore::count_code_region()` +pub mod count_code_region_args { + pub const COUNTER_INDEX: usize = 0; + pub const START_BYTE_POS: usize = 1; + pub const END_BYTE_POS: usize = 2; +} + +/// Positional arguments to `libcore::coverage_counter_add()` and +/// `libcore::coverage_counter_subtract()` +pub mod coverage_counter_expression_args { + pub const COUNTER_EXPRESSION_INDEX: usize = 0; + pub const LEFT_INDEX: usize = 1; + pub const RIGHT_INDEX: usize = 2; + pub const START_BYTE_POS: usize = 3; + pub const END_BYTE_POS: usize = 4; +} + +/// Positional arguments to `libcore::coverage_unreachable()` +pub mod coverage_unreachable_args { + pub const START_BYTE_POS: usize = 0; + pub const END_BYTE_POS: usize = 1; +} diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index 0ed0d9050078c..a3eddd3ff7ba7 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -40,6 +40,7 @@ use std::{iter, mem, option}; use self::predecessors::{PredecessorCache, Predecessors}; pub use self::query::*; +pub mod coverage; pub mod interpret; pub mod mono; mod predecessors; @@ -2307,6 +2308,18 @@ impl<'tcx> Operand<'tcx> { }) } + /// Convenience helper to make a `Scalar` from the given `Operand`, assuming that `Operand` + /// wraps a constant literal value. Panics if this is not the case. + pub fn scalar_from_const(operand: &Operand<'tcx>) -> Scalar { + match operand { + Operand::Constant(constant) => match constant.literal.val.try_to_scalar() { + Some(scalar) => scalar, + _ => panic!("{:?}: Scalar value expected", constant.literal.val), + }, + _ => panic!("{:?}: Constant expected", operand), + } + } + pub fn to_copy(&self) -> Self { match *self { Operand::Copy(_) | Operand::Constant(_) => self.clone(), @@ -2980,18 +2993,3 @@ impl Location { } } } - -/// Coverage data associated with each function (MIR) instrumented with coverage counters, when -/// compiled with `-Zinstrument_coverage`. The query `tcx.coverage_data(DefId)` computes these -/// values on demand (during code generation). This query is only valid after executing the MIR pass -/// `InstrumentCoverage`. -#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] -pub struct CoverageData { - /// A hash value that can be used by the consumer of the coverage profile data to detect - /// changes to the instrumented source of the associated MIR body (typically, for an - /// individual function). - pub hash: u64, - - /// The total number of coverage region counters added to the MIR `Body`. - pub num_counters: u32, -} diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs index 9ad79230a4f6d..8dddaf40c8264 100644 --- a/src/librustc_middle/mir/query.rs +++ b/src/librustc_middle/mir/query.rs @@ -309,3 +309,17 @@ pub struct DestructuredConst<'tcx> { pub variant: Option<VariantIdx>, pub fields: &'tcx [&'tcx ty::Const<'tcx>], } + +/// Coverage information summarized from a MIR if instrumented for source code coverage (see +/// compiler option `-Zinstrument-coverage`). This information is generated by the +/// `InstrumentCoverage` MIR pass and can be retrieved via the `coverageinfo` query. +#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] +pub struct CoverageInfo { + /// A hash value that can be used by the consumer of the coverage profile data to detect + /// changes to the instrumented source of the associated MIR body (typically, for an + /// individual function). + pub hash: u64, + + /// The total number of coverage region counters added to the MIR `Body`. + pub num_counters: u32, +} diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index ba5a8c3ec2052..c9201855462ee 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -231,8 +231,10 @@ rustc_queries! { cache_on_disk_if { key.is_local() } } - query coverage_data(key: DefId) -> mir::CoverageData { - desc { |tcx| "retrieving coverage data from MIR for `{}`", tcx.def_path_str(key) } + /// Returns coverage summary info for a function, after executing the `InstrumentCoverage` + /// MIR pass (assuming the -Zinstrument-coverage option is enabled). + query coverageinfo(key: DefId) -> mir::CoverageInfo { + desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key) } storage(ArenaCacheSelector<'tcx>) cache_on_disk_if { key.is_local() } } diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 88ba28dab82e1..a2bd65e82e24e 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -410,7 +410,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.copy_op(self.operand_index(args[0], index)?, dest)?; } // FIXME(#73156): Handle source code coverage in const eval - sym::count_code_region => (), + sym::count_code_region + | sym::coverage_counter_add + | sym::coverage_counter_subtract + | sym::coverage_unreachable => (), _ => return Ok(false), } diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 068d055fa78f8..c03be2a8fcd69 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -39,7 +39,14 @@ struct CallSite<'tcx> { impl<'tcx> MirPass<'tcx> for Inline { fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut Body<'tcx>) { if tcx.sess.opts.debugging_opts.mir_opt_level >= 2 { - Inliner { tcx, source }.run_pass(body); + if tcx.sess.opts.debugging_opts.instrument_coverage { + // The current implementation of source code coverage injects code region counters + // into the MIR, and assumes a 1-to-1 correspondence between MIR and source-code- + // based function. + debug!("function inlining is disabled when compiling with `instrument_coverage`"); + } else { + Inliner { tcx, source }.run_pass(body); + } } } } diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 06b648ab5a908..25e154f2e9597 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -5,65 +5,71 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::lang_items; use rustc_middle::hir; use rustc_middle::ich::StableHashingContext; -use rustc_middle::mir::interpret::{ConstValue, Scalar}; +use rustc_middle::mir::coverage::*; +use rustc_middle::mir::interpret::Scalar; +use rustc_middle::mir::CoverageInfo; use rustc_middle::mir::{ - self, traversal, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, - StatementKind, Terminator, TerminatorKind, START_BLOCK, + self, traversal, BasicBlock, BasicBlockData, Operand, Place, SourceInfo, StatementKind, + Terminator, TerminatorKind, START_BLOCK, }; use rustc_middle::ty; use rustc_middle::ty::query::Providers; +use rustc_middle::ty::FnDef; use rustc_middle::ty::TyCtxt; -use rustc_middle::ty::{ConstKind, FnDef}; use rustc_span::def_id::DefId; -use rustc_span::Span; +use rustc_span::{Pos, Span}; /// Inserts call to count_code_region() as a placeholder to be replaced during code generation with /// the intrinsic llvm.instrprof.increment. pub struct InstrumentCoverage; -/// The `query` provider for `CoverageData`, requested by `codegen_intrinsic_call()` when +/// The `query` provider for `CoverageInfo`, requested by `codegen_intrinsic_call()` when /// constructing the arguments for `llvm.instrprof.increment`. pub(crate) fn provide(providers: &mut Providers<'_>) { - providers.coverage_data = |tcx, def_id| { - let mir_body = tcx.optimized_mir(def_id); - // FIXME(richkadel): The current implementation assumes the MIR for the given DefId - // represents a single function. Validate and/or correct if inlining and/or monomorphization - // invalidates these assumptions. - let count_code_region_fn = - tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None); - let mut num_counters: u32 = 0; - // The `num_counters` argument to `llvm.instrprof.increment` is the number of injected - // counters, with each counter having an index from `0..num_counters-1`. MIR optimization - // may split and duplicate some BasicBlock sequences. Simply counting the calls may not - // not work; but computing the num_counters by adding `1` to the highest index (for a given - // instrumented function) is valid. - for (_, data) in traversal::preorder(mir_body) { - if let Some(terminator) = &data.terminator { - if let TerminatorKind::Call { func: Operand::Constant(func), args, .. } = - &terminator.kind - { - if let FnDef(called_fn_def_id, _) = func.literal.ty.kind { - if called_fn_def_id == count_code_region_fn { - if let Operand::Constant(constant) = - args.get(0).expect("count_code_region has at least one arg") - { - if let ConstKind::Value(ConstValue::Scalar(value)) = - constant.literal.val - { - let index = value - .to_u32() - .expect("count_code_region index at arg0 is u32"); - num_counters = std::cmp::max(num_counters, index + 1); - } - } - } - } + providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); +} + +fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, mir_def_id: DefId) -> CoverageInfo { + let mir_body = tcx.optimized_mir(mir_def_id); + // FIXME(richkadel): The current implementation assumes the MIR for the given DefId + // represents a single function. Validate and/or correct if inlining (which should be disabled + // if -Zinstrument-coverage is enabled) and/or monomorphization invalidates these assumptions. + let count_code_region_fn = tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None); + + // The `num_counters` argument to `llvm.instrprof.increment` is the number of injected + // counters, with each counter having an index from `0..num_counters-1`. MIR optimization + // may split and duplicate some BasicBlock sequences. Simply counting the calls may not + // not work; but computing the num_counters by adding `1` to the highest index (for a given + // instrumented function) is valid. + let mut num_counters: u32 = 0; + for terminator in traversal::preorder(mir_body) + .map(|(_, data)| (data, count_code_region_fn)) + .filter_map(terminators_that_call_given_fn) + { + if let TerminatorKind::Call { args, .. } = &terminator.kind { + let index_arg = args.get(count_code_region_args::COUNTER_INDEX).expect("arg found"); + let index = + mir::Operand::scalar_from_const(index_arg).to_u32().expect("index arg is u32"); + num_counters = std::cmp::max(num_counters, index + 1); + } + } + let hash = if num_counters > 0 { hash_mir_source(tcx, mir_def_id) } else { 0 }; + CoverageInfo { num_counters, hash } +} + +fn terminators_that_call_given_fn( + (data, fn_def_id): (&'tcx BasicBlockData<'tcx>, DefId), +) -> Option<&'tcx Terminator<'tcx>> { + if let Some(terminator) = &data.terminator { + if let TerminatorKind::Call { func: Operand::Constant(func), .. } = &terminator.kind { + if let FnDef(called_fn_def_id, _) = func.literal.ty.kind { + if called_fn_def_id == fn_def_id { + return Some(&terminator); } } } - let hash = if num_counters > 0 { hash_mir_source(tcx, def_id) } else { 0 }; - CoverageData { num_counters, hash } - }; + } + None } struct Instrumentor<'tcx> { @@ -102,17 +108,16 @@ impl<'tcx> Instrumentor<'tcx> { fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) { // FIXME(richkadel): As a first step, counters are only injected at the top of each // function. The complete solution will inject counters at each conditional code branch. - let top_of_function = START_BLOCK; - let entire_function = mir_body.span; - - self.inject_counter(mir_body, top_of_function, entire_function); + let code_region = mir_body.span; + let next_block = START_BLOCK; + self.inject_counter(mir_body, code_region, next_block); } fn inject_counter( &mut self, mir_body: &mut mir::Body<'tcx>, - next_block: BasicBlock, code_region: Span, + next_block: BasicBlock, ) { let injection_point = code_region.shrink_to_lo(); @@ -121,12 +126,20 @@ impl<'tcx> Instrumentor<'tcx> { self.tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None), injection_point, ); - let counter_index = Operand::const_from_scalar( - self.tcx, - self.tcx.types.u32, - Scalar::from_u32(self.next_counter()), - injection_point, - ); + + let index = self.next_counter(); + + let mut args = Vec::new(); + + use count_code_region_args::*; + debug_assert_eq!(COUNTER_INDEX, args.len()); + args.push(self.const_u32(index, injection_point)); + + debug_assert_eq!(START_BYTE_POS, args.len()); + args.push(self.const_u32(code_region.lo().to_u32(), injection_point)); + + debug_assert_eq!(END_BYTE_POS, args.len()); + args.push(self.const_u32(code_region.hi().to_u32(), injection_point)); let mut patch = MirPatch::new(mir_body); @@ -136,7 +149,7 @@ impl<'tcx> Instrumentor<'tcx> { new_block, TerminatorKind::Call { func: count_code_region_fn, - args: vec![counter_index], + args, // new_block will swapped with the next_block, after applying patch destination: Some((Place::from(temp), new_block)), cleanup: None, @@ -154,6 +167,10 @@ impl<'tcx> Instrumentor<'tcx> { // `next_block`), just swap the indexes, leaving the rest of the graph unchanged. mir_body.basic_blocks_mut().swap(next_block, new_block); } + + fn const_u32(&self, value: u32, span: Span) -> Operand<'tcx> { + Operand::const_from_scalar(self.tcx, self.tcx.types.u32, Scalar::from_u32(value), span) + } } fn function_handle<'tcx>(tcx: TyCtxt<'tcx>, fn_def_id: DefId, span: Span) -> Operand<'tcx> { diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index 9337f241d7022..f65785d15d379 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -881,8 +881,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, (default: no)"), instrument_coverage: bool = (false, parse_bool, [TRACKED], "instrument the generated code with LLVM code region counters to (in the \ - future) generate coverage reports (default: no; note, the compiler build \ - config must include `profiler = true`)"), + future) generate coverage reports; disables/overrides some optimization \ + options (note, the compiler build config must include `profiler = true`) \ + (default: no)"), instrument_mcount: bool = (false, parse_bool, [TRACKED], "insert function instrument code for mcount-based tracing (default: no)"), keep_hygiene_data: bool = (false, parse_bool, [UNTRACKED], diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 857734037afe7..1159269190224 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -242,6 +242,9 @@ symbols! { core, core_intrinsics, count_code_region, + coverage_counter_add, + coverage_counter_subtract, + coverage_unreachable, crate_id, crate_in_paths, crate_local, diff --git a/src/librustc_typeck/check/intrinsic.rs b/src/librustc_typeck/check/intrinsic.rs index 1c0b22ca7370b..6205088adadb7 100644 --- a/src/librustc_typeck/check/intrinsic.rs +++ b/src/librustc_typeck/check/intrinsic.rs @@ -352,7 +352,17 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { return; } - "count_code_region" => (0, vec![tcx.types.u32], tcx.mk_unit()), + "count_code_region" => { + (0, vec![tcx.types.u32, tcx.types.u32, tcx.types.u32], tcx.mk_unit()) + } + + "coverage_counter_add" | "coverage_counter_subtract" => ( + 0, + vec![tcx.types.u32, tcx.types.u32, tcx.types.u32, tcx.types.u32, tcx.types.u32], + tcx.mk_unit(), + ), + + "coverage_unreachable" => (0, vec![tcx.types.u32, tcx.types.u32], tcx.mk_unit()), ref other => { struct_span_err!( diff --git a/src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff b/src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff index 8a079f0da4b01..af2899c887a94 100644 --- a/src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff +++ b/src/test/mir-opt/instrument_coverage/rustc.bar.InstrumentCoverage.diff @@ -7,19 +7,31 @@ bb0: { + StorageLive(_1); // scope 0 at $DIR/instrument_coverage.rs:18:1: 20:2 -+ _1 = const std::intrinsics::count_code_region(const 0_u32) -> bb2; // scope 0 at $DIR/instrument_coverage.rs:18:1: 20:2 ++ _1 = const std::intrinsics::count_code_region(const 0_u32, const 484_u32, const 513_u32) -> bb2; // scope 0 at $DIR/instrument_coverage.rs:18:1: 20:2 + // ty::Const -+ // + ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region} ++ // + ty: unsafe extern "rust-intrinsic" fn(u32, u32, u32) {std::intrinsics::count_code_region} + // + val: Value(Scalar(<ZST>)) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:18:1: 18:1 -+ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region}, val: Value(Scalar(<ZST>)) } ++ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u32, u32, u32) {std::intrinsics::count_code_region}, val: Value(Scalar(<ZST>)) } + // ty::Const + // + ty: u32 + // + val: Value(Scalar(0x00000000)) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:18:1: 18:1 + // + literal: Const { ty: u32, val: Value(Scalar(0x00000000)) } ++ // ty::Const ++ // + ty: u32 ++ // + val: Value(Scalar(0x000001e4)) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:18:1: 18:1 ++ // + literal: Const { ty: u32, val: Value(Scalar(0x000001e4)) } ++ // ty::Const ++ // + ty: u32 ++ // + val: Value(Scalar(0x00000201)) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:18:1: 18:1 ++ // + literal: Const { ty: u32, val: Value(Scalar(0x00000201)) } + } + + bb1 (cleanup): { diff --git a/src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff b/src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff index 3c2ec1dc06b70..4a300230f8a97 100644 --- a/src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff +++ b/src/test/mir-opt/instrument_coverage/rustc.main.InstrumentCoverage.diff @@ -11,19 +11,31 @@ bb0: { - falseUnwind -> [real: bb1, cleanup: bb2]; // scope 0 at $DIR/instrument_coverage.rs:10:5: 14:6 + StorageLive(_4); // scope 0 at $DIR/instrument_coverage.rs:9:1: 15:2 -+ _4 = const std::intrinsics::count_code_region(const 0_u32) -> bb7; // scope 0 at $DIR/instrument_coverage.rs:9:1: 15:2 ++ _4 = const std::intrinsics::count_code_region(const 0_u32, const 387_u32, const 465_u32) -> bb7; // scope 0 at $DIR/instrument_coverage.rs:9:1: 15:2 + // ty::Const -+ // + ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region} ++ // + ty: unsafe extern "rust-intrinsic" fn(u32, u32, u32) {std::intrinsics::count_code_region} + // + val: Value(Scalar(<ZST>)) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:9:1: 9:1 -+ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u32) {std::intrinsics::count_code_region}, val: Value(Scalar(<ZST>)) } ++ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u32, u32, u32) {std::intrinsics::count_code_region}, val: Value(Scalar(<ZST>)) } + // ty::Const + // + ty: u32 + // + val: Value(Scalar(0x00000000)) + // mir::Constant + // + span: $DIR/instrument_coverage.rs:9:1: 9:1 + // + literal: Const { ty: u32, val: Value(Scalar(0x00000000)) } ++ // ty::Const ++ // + ty: u32 ++ // + val: Value(Scalar(0x00000183)) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:9:1: 9:1 ++ // + literal: Const { ty: u32, val: Value(Scalar(0x00000183)) } ++ // ty::Const ++ // + ty: u32 ++ // + val: Value(Scalar(0x000001d1)) ++ // mir::Constant ++ // + span: $DIR/instrument_coverage.rs:9:1: 9:1 ++ // + literal: Const { ty: u32, val: Value(Scalar(0x000001d1)) } } bb1: { From 135ca385782be5dbf79f5688f32b030dbde74a41 Mon Sep 17 00:00:00 2001 From: Adam Perry <lol@anp.lol> Date: Thu, 21 May 2020 16:43:39 -0700 Subject: [PATCH 15/15] Stabilize `#[track_caller]`. Does not yet make its constness stable, though. Use of `Location::caller` in const contexts is still gated by `#![feature(const_caller_location)]`. --- .../src/language-features/track-caller.md | 5 ----- src/libcore/lib.rs | 2 +- src/libcore/macros/mod.rs | 2 +- src/libcore/panic.rs | 14 ++++---------- src/librustc_error_codes/error_codes/E0736.md | 2 -- src/librustc_error_codes/error_codes/E0737.md | 2 -- src/librustc_error_codes/error_codes/E0739.md | 1 - src/librustc_errors/lib.rs | 2 +- src/librustc_feature/accepted.rs | 3 +++ src/librustc_feature/active.rs | 4 ---- src/librustc_feature/builtin_attrs.rs | 2 +- src/librustc_middle/lib.rs | 2 +- src/libstd/lib.rs | 2 +- .../ui/feature-gates/feature-gate-track_caller.rs | 5 ----- .../feature-gates/feature-gate-track_caller.stderr | 12 ------------ src/test/ui/macros/issue-68060.rs | 2 -- src/test/ui/macros/issue-68060.stderr | 6 +++--- .../numbers-arithmetic/saturating-float-casts.rs | 1 - src/test/ui/rfc-2091-track-caller/call-chain.rs | 2 -- .../caller-location-fnptr-rt-ctfe-equiv.rs | 2 +- .../caller-location-intrinsic.rs | 6 ++---- .../rfc-2091-track-caller/const-caller-location.rs | 2 +- .../diverging-caller-location.rs | 2 -- .../ui/rfc-2091-track-caller/error-odd-syntax.rs | 2 -- .../rfc-2091-track-caller/error-odd-syntax.stderr | 2 +- .../error-with-invalid-abi.rs | 2 -- .../error-with-invalid-abi.stderr | 4 ++-- .../ui/rfc-2091-track-caller/error-with-naked.rs | 2 +- .../ui/rfc-2091-track-caller/intrinsic-wrapper.rs | 6 ++---- src/test/ui/rfc-2091-track-caller/only-for-fns.rs | 2 -- .../ui/rfc-2091-track-caller/only-for-fns.stderr | 2 +- src/test/ui/rfc-2091-track-caller/pass.rs | 2 -- .../track-caller-attribute.rs | 10 ++++------ .../ui/rfc-2091-track-caller/track-caller-ffi.rs | 10 ++++------ .../tracked-fn-ptr-with-arg.rs | 2 -- .../ui/rfc-2091-track-caller/tracked-fn-ptr.rs | 2 -- .../rfc-2091-track-caller/tracked-trait-impls.rs | 2 -- 37 files changed, 35 insertions(+), 98 deletions(-) delete mode 100644 src/doc/unstable-book/src/language-features/track-caller.md delete mode 100644 src/test/ui/feature-gates/feature-gate-track_caller.rs delete mode 100644 src/test/ui/feature-gates/feature-gate-track_caller.stderr diff --git a/src/doc/unstable-book/src/language-features/track-caller.md b/src/doc/unstable-book/src/language-features/track-caller.md deleted file mode 100644 index afc11a2b9492c..0000000000000 --- a/src/doc/unstable-book/src/language-features/track-caller.md +++ /dev/null @@ -1,5 +0,0 @@ -# `track_caller` - -The tracking issue for this feature is: [#47809](https://github.com/rust-lang/rust/issues/47809). - ------------------------- diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index aeb52bffbf24c..b732dacae1c5b 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -118,7 +118,7 @@ #![feature(staged_api)] #![feature(std_internals)] #![feature(stmt_expr_attributes)] -#![feature(track_caller)] +#![cfg_attr(bootstrap, feature(track_caller))] #![feature(transparent_unions)] #![feature(unboxed_closures)] #![feature(unsized_locals)] diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs index 3cfdde60135b7..13c0e8daf740f 100644 --- a/src/libcore/macros/mod.rs +++ b/src/libcore/macros/mod.rs @@ -1,6 +1,6 @@ #[doc(include = "panic.md")] #[macro_export] -#[allow_internal_unstable(core_panic, track_caller)] +#[allow_internal_unstable(core_panic, const_caller_location)] #[stable(feature = "core", since = "1.6.0")] macro_rules! panic { () => ( diff --git a/src/libcore/panic.rs b/src/libcore/panic.rs index c7009b76e8148..543aa969330ae 100644 --- a/src/libcore/panic.rs +++ b/src/libcore/panic.rs @@ -190,7 +190,6 @@ impl<'a> Location<'a> { /// # Examples /// /// ``` - /// #![feature(track_caller)] /// use core::panic::Location; /// /// /// Returns the [`Location`] at which it is called. @@ -206,7 +205,7 @@ impl<'a> Location<'a> { /// /// let fixed_location = get_just_one_location(); /// assert_eq!(fixed_location.file(), file!()); - /// assert_eq!(fixed_location.line(), 15); + /// assert_eq!(fixed_location.line(), 14); /// assert_eq!(fixed_location.column(), 5); /// /// // running the same untracked function in a different location gives us the same result @@ -217,7 +216,7 @@ impl<'a> Location<'a> { /// /// let this_location = get_caller_location(); /// assert_eq!(this_location.file(), file!()); - /// assert_eq!(this_location.line(), 29); + /// assert_eq!(this_location.line(), 28); /// assert_eq!(this_location.column(), 21); /// /// // running the tracked function in a different location produces a different value @@ -226,13 +225,8 @@ impl<'a> Location<'a> { /// assert_ne!(this_location.line(), another_location.line()); /// assert_ne!(this_location.column(), another_location.column()); /// ``` - // FIXME: When stabilizing this method, please also update the documentation - // of `intrinsics::caller_location`. - #[unstable( - feature = "track_caller", - reason = "uses #[track_caller] which is not yet stable", - issue = "47809" - )] + #[stable(feature = "track_caller", since = "1.46.0")] + #[rustc_const_unstable(feature = "const_caller_location", issue = "47809")] #[track_caller] pub const fn caller() -> &'static Location<'static> { crate::intrinsics::caller_location() diff --git a/src/librustc_error_codes/error_codes/E0736.md b/src/librustc_error_codes/error_codes/E0736.md index 8a60dc320599b..0f3d41ba66dc4 100644 --- a/src/librustc_error_codes/error_codes/E0736.md +++ b/src/librustc_error_codes/error_codes/E0736.md @@ -3,8 +3,6 @@ Erroneous code example: ```compile_fail,E0736 -#![feature(track_caller)] - #[naked] #[track_caller] fn foo() {} diff --git a/src/librustc_error_codes/error_codes/E0737.md b/src/librustc_error_codes/error_codes/E0737.md index c6553e97b7e91..ab5e60692b4da 100644 --- a/src/librustc_error_codes/error_codes/E0737.md +++ b/src/librustc_error_codes/error_codes/E0737.md @@ -5,8 +5,6 @@ restrictions. Erroneous code example: ```compile_fail,E0737 -#![feature(track_caller)] - #[track_caller] extern "C" fn foo() {} ``` diff --git a/src/librustc_error_codes/error_codes/E0739.md b/src/librustc_error_codes/error_codes/E0739.md index 707751066edbc..8d9039bef93f6 100644 --- a/src/librustc_error_codes/error_codes/E0739.md +++ b/src/librustc_error_codes/error_codes/E0739.md @@ -3,7 +3,6 @@ Erroneous code example: ```compile_fail,E0739 -#![feature(track_caller)] #[track_caller] struct Bar { a: u8, diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index 0c1418d3cad27..362913ceadf18 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -5,7 +5,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/")] #![feature(crate_visibility_modifier)] #![feature(nll)] -#![feature(track_caller)] +#![cfg_attr(bootstrap, feature(track_caller))] pub use emitter::ColorConfig; diff --git a/src/librustc_feature/accepted.rs b/src/librustc_feature/accepted.rs index b164b21913d6e..d93c17b05b498 100644 --- a/src/librustc_feature/accepted.rs +++ b/src/librustc_feature/accepted.rs @@ -265,6 +265,9 @@ declare_features! ( (accepted, const_if_match, "1.45.0", Some(49146), None), /// Allows the use of `loop` and `while` in constants. (accepted, const_loop, "1.45.0", Some(52000), None), + /// Allows `#[track_caller]` to be used which provides + /// accurate caller location reporting during panic (RFC 2091). + (accepted, track_caller, "1.46.0", Some(47809), None), // ------------------------------------------------------------------------- // feature-group-end: accepted features diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index 8660d6a8d6410..b9a55377949f2 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -494,10 +494,6 @@ declare_features! ( /// Allows the use of raw-dylibs (RFC 2627). (active, raw_dylib, "1.40.0", Some(58713), None), - /// Allows `#[track_caller]` to be used which provides - /// accurate caller location reporting during panic (RFC 2091). - (active, track_caller, "1.40.0", Some(47809), None), - /// Allows making `dyn Trait` well-formed even if `Trait` is not object safe. /// In that case, `dyn Trait: Trait` does not hold. Moreover, coercions and /// casts in safe Rust to `dyn Trait` for such a `Trait` is also forbidden. diff --git a/src/librustc_feature/builtin_attrs.rs b/src/librustc_feature/builtin_attrs.rs index 524a357971029..c9a34f033758b 100644 --- a/src/librustc_feature/builtin_attrs.rs +++ b/src/librustc_feature/builtin_attrs.rs @@ -260,6 +260,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ungated!(cold, Whitelisted, template!(Word)), ungated!(no_builtins, Whitelisted, template!(Word)), ungated!(target_feature, Whitelisted, template!(List: r#"enable = "name""#)), + ungated!(track_caller, Whitelisted, template!(Word)), gated!( no_sanitize, Whitelisted, template!(List: "address, memory, thread"), @@ -333,7 +334,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ gated!(ffi_returns_twice, Whitelisted, template!(Word), experimental!(ffi_returns_twice)), gated!(ffi_pure, Whitelisted, template!(Word), experimental!(ffi_pure)), gated!(ffi_const, Whitelisted, template!(Word), experimental!(ffi_const)), - gated!(track_caller, Whitelisted, template!(Word), experimental!(track_caller)), gated!( register_attr, CrateLevel, template!(List: "attr1, attr2, ..."), experimental!(register_attr), diff --git a/src/librustc_middle/lib.rs b/src/librustc_middle/lib.rs index 676346fbebdd1..96b8ca27183ed 100644 --- a/src/librustc_middle/lib.rs +++ b/src/librustc_middle/lib.rs @@ -42,7 +42,7 @@ #![feature(or_patterns)] #![feature(range_is_empty)] #![feature(min_specialization)] -#![feature(track_caller)] +#![cfg_attr(bootstrap, feature(track_caller))] #![feature(trusted_len)] #![feature(stmt_expr_attributes)] #![feature(test)] diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index ef699ede2a140..372038df54f2e 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -316,7 +316,7 @@ #![feature(toowned_clone_into)] #![feature(total_cmp)] #![feature(trace_macros)] -#![feature(track_caller)] +#![cfg_attr(bootstrap, feature(track_caller))] #![feature(try_reserve)] #![feature(unboxed_closures)] #![feature(untagged_unions)] diff --git a/src/test/ui/feature-gates/feature-gate-track_caller.rs b/src/test/ui/feature-gates/feature-gate-track_caller.rs deleted file mode 100644 index 5865cf0a4f754..0000000000000 --- a/src/test/ui/feature-gates/feature-gate-track_caller.rs +++ /dev/null @@ -1,5 +0,0 @@ -#[track_caller] -fn f() {} -//~^^ ERROR the `#[track_caller]` attribute is an experimental feature - -fn main() {} diff --git a/src/test/ui/feature-gates/feature-gate-track_caller.stderr b/src/test/ui/feature-gates/feature-gate-track_caller.stderr deleted file mode 100644 index 8ceab501617ee..0000000000000 --- a/src/test/ui/feature-gates/feature-gate-track_caller.stderr +++ /dev/null @@ -1,12 +0,0 @@ -error[E0658]: the `#[track_caller]` attribute is an experimental feature - --> $DIR/feature-gate-track_caller.rs:1:1 - | -LL | #[track_caller] - | ^^^^^^^^^^^^^^^ - | - = note: see issue #47809 <https://github.com/rust-lang/rust/issues/47809> for more information - = help: add `#![feature(track_caller)]` to the crate attributes to enable - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/macros/issue-68060.rs b/src/test/ui/macros/issue-68060.rs index bc70f8ffec2b2..8772e98b6e9bb 100644 --- a/src/test/ui/macros/issue-68060.rs +++ b/src/test/ui/macros/issue-68060.rs @@ -1,5 +1,3 @@ -#![feature(track_caller)] - fn main() { (0..) .map( diff --git a/src/test/ui/macros/issue-68060.stderr b/src/test/ui/macros/issue-68060.stderr index 22187c4a4098a..b9b2f946c5967 100644 --- a/src/test/ui/macros/issue-68060.stderr +++ b/src/test/ui/macros/issue-68060.stderr @@ -1,5 +1,5 @@ error[E0658]: `#[target_feature(..)]` can only be applied to `unsafe` functions - --> $DIR/issue-68060.rs:6:13 + --> $DIR/issue-68060.rs:4:13 | LL | #[target_feature(enable = "")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -11,13 +11,13 @@ LL | |_| (), = help: add `#![feature(target_feature_11)]` to the crate attributes to enable error: the feature named `` is not valid for this target - --> $DIR/issue-68060.rs:6:30 + --> $DIR/issue-68060.rs:4:30 | LL | #[target_feature(enable = "")] | ^^^^^^^^^^^ `` is not valid for this target error[E0737]: `#[track_caller]` requires Rust ABI - --> $DIR/issue-68060.rs:9:13 + --> $DIR/issue-68060.rs:7:13 | LL | #[track_caller] | ^^^^^^^^^^^^^^^ diff --git a/src/test/ui/numbers-arithmetic/saturating-float-casts.rs b/src/test/ui/numbers-arithmetic/saturating-float-casts.rs index e6d0c94a02fac..825a11972aeeb 100644 --- a/src/test/ui/numbers-arithmetic/saturating-float-casts.rs +++ b/src/test/ui/numbers-arithmetic/saturating-float-casts.rs @@ -9,7 +9,6 @@ // merged. #![feature(test, stmt_expr_attributes)] -#![feature(track_caller)] #![deny(overflowing_literals)] extern crate test; diff --git a/src/test/ui/rfc-2091-track-caller/call-chain.rs b/src/test/ui/rfc-2091-track-caller/call-chain.rs index 3f1c8f7abe8b8..fefb84de729fe 100644 --- a/src/test/ui/rfc-2091-track-caller/call-chain.rs +++ b/src/test/ui/rfc-2091-track-caller/call-chain.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - use std::panic::Location; struct Foo; diff --git a/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs b/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs index edf9ba2c41a15..05240908917bc 100644 --- a/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs +++ b/src/test/ui/rfc-2091-track-caller/caller-location-fnptr-rt-ctfe-equiv.rs @@ -6,7 +6,7 @@ // run-pass // compile-flags: -Z unleash-the-miri-inside-of-you -#![feature(core_intrinsics, const_caller_location, track_caller, const_fn)] +#![feature(core_intrinsics, const_caller_location, const_fn)] type L = &'static std::panic::Location<'static>; diff --git a/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs b/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs index 090e912c1d0ba..f244b74e391ff 100644 --- a/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs +++ b/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - #[inline(never)] #[track_caller] fn codegen_caller_loc() -> &'static core::panic::Location<'static> { @@ -15,13 +13,13 @@ macro_rules! caller_location_from_macro { fn main() { let loc = codegen_caller_loc(); assert_eq!(loc.file(), file!()); - assert_eq!(loc.line(), 16); + assert_eq!(loc.line(), 14); assert_eq!(loc.column(), 15); // `Location::caller()` in a macro should behave similarly to `file!` and `line!`, // i.e. point to where the macro was invoked, instead of the macro itself. let loc2 = caller_location_from_macro!(); assert_eq!(loc2.file(), file!()); - assert_eq!(loc2.line(), 23); + assert_eq!(loc2.line(), 21); assert_eq!(loc2.column(), 16); } diff --git a/src/test/ui/rfc-2091-track-caller/const-caller-location.rs b/src/test/ui/rfc-2091-track-caller/const-caller-location.rs index 0614c52c66036..8030a4d967a67 100644 --- a/src/test/ui/rfc-2091-track-caller/const-caller-location.rs +++ b/src/test/ui/rfc-2091-track-caller/const-caller-location.rs @@ -1,6 +1,6 @@ // run-pass -#![feature(const_fn, track_caller)] +#![feature(const_caller_location, const_fn)] use std::panic::Location; diff --git a/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs b/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs index 1fb75ef35ffc1..6681119557d79 100644 --- a/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs +++ b/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs @@ -6,8 +6,6 @@ //! we don't inspect the location returned -- it would be difficult to distinguish between the //! explicit panic and a failed assertion. That it compiles and runs is enough for this one. -#![feature(track_caller)] - #[track_caller] fn doesnt_return() -> ! { let _location = core::panic::Location::caller(); diff --git a/src/test/ui/rfc-2091-track-caller/error-odd-syntax.rs b/src/test/ui/rfc-2091-track-caller/error-odd-syntax.rs index d6560231871c9..6f4290e2a5ee9 100644 --- a/src/test/ui/rfc-2091-track-caller/error-odd-syntax.rs +++ b/src/test/ui/rfc-2091-track-caller/error-odd-syntax.rs @@ -1,5 +1,3 @@ -#![feature(track_caller)] - #[track_caller(1)] fn f() {} //~^^ ERROR malformed `track_caller` attribute input diff --git a/src/test/ui/rfc-2091-track-caller/error-odd-syntax.stderr b/src/test/ui/rfc-2091-track-caller/error-odd-syntax.stderr index 8906fa59506a7..e7ddf8df4ab53 100644 --- a/src/test/ui/rfc-2091-track-caller/error-odd-syntax.stderr +++ b/src/test/ui/rfc-2091-track-caller/error-odd-syntax.stderr @@ -1,5 +1,5 @@ error: malformed `track_caller` attribute input - --> $DIR/error-odd-syntax.rs:3:1 + --> $DIR/error-odd-syntax.rs:1:1 | LL | #[track_caller(1)] | ^^^^^^^^^^^^^^^^^^ help: must be of the form: `#[track_caller]` diff --git a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs index 1145f7786c78b..074e1ceb791ce 100644 --- a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs +++ b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.rs @@ -1,5 +1,3 @@ -#![feature(track_caller)] - #[track_caller] extern "C" fn f() {} //~^^ ERROR `#[track_caller]` requires Rust ABI diff --git a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr index e08c52fabd6b7..bcc0c8170e655 100644 --- a/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr +++ b/src/test/ui/rfc-2091-track-caller/error-with-invalid-abi.stderr @@ -1,11 +1,11 @@ error[E0737]: `#[track_caller]` requires Rust ABI - --> $DIR/error-with-invalid-abi.rs:3:1 + --> $DIR/error-with-invalid-abi.rs:1:1 | LL | #[track_caller] | ^^^^^^^^^^^^^^^ error[E0737]: `#[track_caller]` requires Rust ABI - --> $DIR/error-with-invalid-abi.rs:8:5 + --> $DIR/error-with-invalid-abi.rs:6:5 | LL | #[track_caller] | ^^^^^^^^^^^^^^^ diff --git a/src/test/ui/rfc-2091-track-caller/error-with-naked.rs b/src/test/ui/rfc-2091-track-caller/error-with-naked.rs index f457384833335..2b110c9a32515 100644 --- a/src/test/ui/rfc-2091-track-caller/error-with-naked.rs +++ b/src/test/ui/rfc-2091-track-caller/error-with-naked.rs @@ -1,4 +1,4 @@ -#![feature(naked_functions, track_caller)] +#![feature(naked_functions)] #[track_caller] //~ ERROR cannot use `#[track_caller]` with `#[naked]` #[naked] diff --git a/src/test/ui/rfc-2091-track-caller/intrinsic-wrapper.rs b/src/test/ui/rfc-2091-track-caller/intrinsic-wrapper.rs index 76e62b89ab818..74217f47084a3 100644 --- a/src/test/ui/rfc-2091-track-caller/intrinsic-wrapper.rs +++ b/src/test/ui/rfc-2091-track-caller/intrinsic-wrapper.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - macro_rules! caller_location_from_macro { () => (core::panic::Location::caller()); } @@ -9,13 +7,13 @@ macro_rules! caller_location_from_macro { fn main() { let loc = core::panic::Location::caller(); assert_eq!(loc.file(), file!()); - assert_eq!(loc.line(), 10); + assert_eq!(loc.line(), 8); assert_eq!(loc.column(), 15); // `Location::caller()` in a macro should behave similarly to `file!` and `line!`, // i.e. point to where the macro was invoked, instead of the macro itself. let loc2 = caller_location_from_macro!(); assert_eq!(loc2.file(), file!()); - assert_eq!(loc2.line(), 17); + assert_eq!(loc2.line(), 15); assert_eq!(loc2.column(), 16); } diff --git a/src/test/ui/rfc-2091-track-caller/only-for-fns.rs b/src/test/ui/rfc-2091-track-caller/only-for-fns.rs index 0fd59b4bf4918..bc0ca9552806f 100644 --- a/src/test/ui/rfc-2091-track-caller/only-for-fns.rs +++ b/src/test/ui/rfc-2091-track-caller/only-for-fns.rs @@ -1,5 +1,3 @@ -#![feature(track_caller)] - #[track_caller] struct S; //~^^ ERROR attribute should be applied to function diff --git a/src/test/ui/rfc-2091-track-caller/only-for-fns.stderr b/src/test/ui/rfc-2091-track-caller/only-for-fns.stderr index c2fb8fa1eb6ca..6666dcfa6e599 100644 --- a/src/test/ui/rfc-2091-track-caller/only-for-fns.stderr +++ b/src/test/ui/rfc-2091-track-caller/only-for-fns.stderr @@ -1,5 +1,5 @@ error[E0739]: attribute should be applied to function - --> $DIR/only-for-fns.rs:3:1 + --> $DIR/only-for-fns.rs:1:1 | LL | #[track_caller] | ^^^^^^^^^^^^^^^ diff --git a/src/test/ui/rfc-2091-track-caller/pass.rs b/src/test/ui/rfc-2091-track-caller/pass.rs index eef83b3d68f97..ada150b25cf2c 100644 --- a/src/test/ui/rfc-2091-track-caller/pass.rs +++ b/src/test/ui/rfc-2091-track-caller/pass.rs @@ -1,6 +1,4 @@ // run-pass -#![feature(track_caller)] - #[track_caller] fn f() {} diff --git a/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs b/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs index 76a380ed3e30d..efcc1f6942de1 100644 --- a/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs +++ b/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - use std::panic::Location; #[track_caller] @@ -20,21 +18,21 @@ fn nested_tracked() -> &'static Location<'static> { fn main() { let location = Location::caller(); assert_eq!(location.file(), file!()); - assert_eq!(location.line(), 21); + assert_eq!(location.line(), 19); assert_eq!(location.column(), 20); let tracked = tracked(); assert_eq!(tracked.file(), file!()); - assert_eq!(tracked.line(), 26); + assert_eq!(tracked.line(), 24); assert_eq!(tracked.column(), 19); let nested = nested_intrinsic(); assert_eq!(nested.file(), file!()); - assert_eq!(nested.line(), 13); + assert_eq!(nested.line(), 11); assert_eq!(nested.column(), 5); let contained = nested_tracked(); assert_eq!(contained.file(), file!()); - assert_eq!(contained.line(), 17); + assert_eq!(contained.line(), 15); assert_eq!(contained.column(), 5); } diff --git a/src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs b/src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs index 23c17d743c43c..5115f687c2632 100644 --- a/src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs +++ b/src/test/ui/rfc-2091-track-caller/track-caller-ffi.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - use std::panic::Location; extern "Rust" { @@ -30,21 +28,21 @@ mod provides { fn main() { let location = Location::caller(); assert_eq!(location.file(), file!()); - assert_eq!(location.line(), 31); + assert_eq!(location.line(), 29); assert_eq!(location.column(), 20); let tracked = unsafe { rust_track_caller_ffi_test_tracked() }; assert_eq!(tracked.file(), file!()); - assert_eq!(tracked.line(), 36); + assert_eq!(tracked.line(), 34); assert_eq!(tracked.column(), 28); let untracked = unsafe { rust_track_caller_ffi_test_untracked() }; assert_eq!(untracked.file(), file!()); - assert_eq!(untracked.line(), 26); + assert_eq!(untracked.line(), 24); assert_eq!(untracked.column(), 9); let contained = rust_track_caller_ffi_test_nested_tracked(); assert_eq!(contained.file(), file!()); - assert_eq!(contained.line(), 14); + assert_eq!(contained.line(), 12); assert_eq!(contained.column(), 14); } diff --git a/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs b/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs index b17c1efb3d38c..5fcfea96d547a 100644 --- a/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs +++ b/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr-with-arg.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - fn pass_to_ptr_call<T>(f: fn(T), x: T) { f(x); } diff --git a/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr.rs b/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr.rs index 8ee4d4fa16871..4415d850c241c 100644 --- a/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr.rs +++ b/src/test/ui/rfc-2091-track-caller/tracked-fn-ptr.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - fn ptr_call(f: fn()) { f(); } diff --git a/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs b/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs index 0a5f92bb635e5..4db4c29e53d58 100644 --- a/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs +++ b/src/test/ui/rfc-2091-track-caller/tracked-trait-impls.rs @@ -1,7 +1,5 @@ // run-pass -#![feature(track_caller)] - macro_rules! assert_expansion_site_is_tracked { () => {{ let location = std::panic::Location::caller();