From 9bcc133bb7f818958c2882d54400ddafd8aea8f3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 10 Jun 2024 21:16:54 -0500 Subject: [PATCH 01/19] docs(change): Don't mention a Cargo 2024 edition change for 1.79 --- RELEASES.md | 1 - 1 file changed, 1 deletion(-) diff --git a/RELEASES.md b/RELEASES.md index c1311ab14c534..d63ebd0a4a90d 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -97,7 +97,6 @@ Cargo - [Prevent dashes in `lib.name`, always normalizing to `_`.](https://github.com/rust-lang/cargo/pull/12783/) - [Stabilize MSRV-aware version requirement selection in `cargo add`.](https://github.com/rust-lang/cargo/pull/13608/) - [Switch to using `gitoxide` by default for listing files.](https://github.com/rust-lang/cargo/pull/13696/) -- [Error on `[project]` in Edition 2024; `cargo fix --edition` will change it to `[package]`.](https://github.com/rust-lang/cargo/pull/13747/) From c462328382b9fc1c39ffca3f519f7559717b2ccc Mon Sep 17 00:00:00 2001 From: Stefan Lankes Date: Wed, 12 Jun 2024 20:18:33 +0200 Subject: [PATCH 02/19] export std::os::fd module on HermitOS The HermitOS' IO interface is similiar to Unix. Consequently, this PR synchronize the FD implementation between both. --- library/std/src/os/hermit/io/mod.rs | 15 +++------------ library/std/src/os/mod.rs | 2 +- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/library/std/src/os/hermit/io/mod.rs b/library/std/src/os/hermit/io/mod.rs index 524dfae0d63ae..df93f63a003cf 100644 --- a/library/std/src/os/hermit/io/mod.rs +++ b/library/std/src/os/hermit/io/mod.rs @@ -1,13 +1,4 @@ -#![stable(feature = "os_fd", since = "1.66.0")] +#![stable(feature = "rust1", since = "1.0.0")] -mod net; -#[path = "../../fd/owned.rs"] -mod owned; -#[path = "../../fd/raw.rs"] -mod raw; - -// Export the types and traits for the public API. -#[stable(feature = "os_fd", since = "1.66.0")] -pub use owned::*; -#[stable(feature = "os_fd", since = "1.66.0")] -pub use raw::*; +#[stable(feature = "rust1", since = "1.0.0")] +pub use crate::os::fd::*; diff --git a/library/std/src/os/mod.rs b/library/std/src/os/mod.rs index ca3584e82f918..d2a7b316b8131 100644 --- a/library/std/src/os/mod.rs +++ b/library/std/src/os/mod.rs @@ -160,7 +160,7 @@ pub(crate) mod watchos; #[cfg(target_os = "xous")] pub mod xous; -#[cfg(any(unix, target_os = "wasi", doc))] +#[cfg(any(unix, target_os = "hermit", target_os = "wasi", doc))] pub mod fd; #[cfg(any(target_os = "linux", target_os = "android", doc))] From 9e851041d7b7631217857067f80b461543844c24 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 14 Jun 2024 11:18:32 +0200 Subject: [PATCH 03/19] div_euclid, rem_euclid: clarify/extend documentation --- library/core/src/num/int_macros.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/library/core/src/num/int_macros.rs b/library/core/src/num/int_macros.rs index 55bb6166f1017..4488948496398 100644 --- a/library/core/src/num/int_macros.rs +++ b/library/core/src/num/int_macros.rs @@ -2784,8 +2784,10 @@ macro_rules! int_impl { /// /// In other words, the result is `self / rhs` rounded to the integer `q` /// such that `self >= q * rhs`. - /// If `self > 0`, this is equal to round towards zero (the default in Rust); - /// if `self < 0`, this is equal to round towards +/- infinity. + /// If `self > 0`, this is equal to rounding towards zero (the default in Rust); + /// if `self < 0`, this is equal to rounding away from zero (towards +/- infinity). + /// If `rhs > 0`, this is equal to rounding towards -infinity; + /// if `rhs < 0`, this is equal to rounding towards +infinity. /// /// # Panics /// @@ -2823,8 +2825,8 @@ macro_rules! int_impl { /// Calculates the least nonnegative remainder of `self (mod rhs)`. /// /// This is done as if by the Euclidean division algorithm -- given - /// `r = self.rem_euclid(rhs)`, `self = rhs * self.div_euclid(rhs) + r`, and - /// `0 <= r < abs(rhs)`. + /// `r = self.rem_euclid(rhs)`, the result satisfies + /// `self = rhs * self.div_euclid(rhs) + r` and `0 <= r < abs(rhs)`. /// /// # Panics /// From 4f97ab54c452400b47e3518e17d18ada7675f07d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 18 May 2024 16:56:08 -0400 Subject: [PATCH 04/19] Resolve elided lifetimes in assoc const to static if no other lifetimes are in scope --- compiler/rustc_lint/messages.ftl | 1 + .../rustc_lint/src/context/diagnostics.rs | 15 ++- compiler/rustc_lint/src/lints.rs | 2 + compiler/rustc_lint_defs/src/builtin.rs | 8 +- compiler/rustc_lint_defs/src/lib.rs | 1 + compiler/rustc_resolve/src/late.rs | 104 +++++++++++------- tests/ui/associated-consts/double-elided.rs | 8 +- .../ui/associated-consts/double-elided.stderr | 47 -------- ...nfer-placeholder-in-non-suggestable-pos.rs | 2 - ...-placeholder-in-non-suggestable-pos.stderr | 16 +-- .../consts/assoc-const-elided-lifetime.stderr | 12 ++ 11 files changed, 101 insertions(+), 115 deletions(-) delete mode 100644 tests/ui/associated-consts/double-elided.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 733c73bc3d078..ec22da407fa9b 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -14,6 +14,7 @@ lint_associated_const_elided_lifetime = {$elided -> *[false] `'_` cannot be used here } .suggestion = use the `'static` lifetime + .note = cannot automatically infer `'static` because of other lifetimes in scope lint_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified .note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future` diff --git a/compiler/rustc_lint/src/context/diagnostics.rs b/compiler/rustc_lint/src/context/diagnostics.rs index 83640d7210fa7..290bb5173dbe7 100644 --- a/compiler/rustc_lint/src/context/diagnostics.rs +++ b/compiler/rustc_lint/src/context/diagnostics.rs @@ -319,11 +319,20 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: & BuiltinLintDiag::UnusedQualifications { removal_span } => { lints::UnusedQualifications { removal_span }.decorate_lint(diag); } - BuiltinLintDiag::AssociatedConstElidedLifetime { elided, span: lt_span } => { + BuiltinLintDiag::AssociatedConstElidedLifetime { + elided, + span: lt_span, + lifetimes_in_scope, + } => { let lt_span = if elided { lt_span.shrink_to_hi() } else { lt_span }; let code = if elided { "'static " } else { "'static" }; - lints::AssociatedConstElidedLifetime { span: lt_span, code, elided } - .decorate_lint(diag); + lints::AssociatedConstElidedLifetime { + span: lt_span, + code, + elided, + lifetimes_in_scope, + } + .decorate_lint(diag); } BuiltinLintDiag::RedundantImportVisibility { max_vis, span: vis_span, import_vis } => { lints::RedundantImportVisibility { span: vis_span, help: (), max_vis, import_vis } diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index b377da31a581b..698317ebf7b96 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -2865,6 +2865,8 @@ pub struct AssociatedConstElidedLifetime { pub code: &'static str, pub elided: bool, + #[note] + pub lifetimes_in_scope: MultiSpan, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index a12c76037e77e..1913b9d6a1c31 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -4593,16 +4593,18 @@ declare_lint! { declare_lint! { /// The `elided_lifetimes_in_associated_constant` lint detects elided lifetimes - /// that were erroneously allowed in associated constants. + /// in associated constants when there are other lifetimes in scope. This was + /// accidentally supported, and this lint was later relaxed to allow eliding + /// lifetimes to `'static` when there are no lifetimes in scope. /// /// ### Example /// /// ```rust,compile_fail /// #![deny(elided_lifetimes_in_associated_constant)] /// - /// struct Foo; + /// struct Foo<'a>(&'a ()); /// - /// impl Foo { + /// impl<'a> Foo<'a> { /// const STR: &str = "hello, world"; /// } /// ``` diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 1ce95df34041c..70330c4457766 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -696,6 +696,7 @@ pub enum BuiltinLintDiag { AssociatedConstElidedLifetime { elided: bool, span: Span, + lifetimes_in_scope: MultiSpan, }, RedundantImportVisibility { span: Span, diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index b0adc3775f667..20926366ba5e8 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -310,9 +310,10 @@ enum LifetimeRibKind { /// error on default object bounds (e.g., `Box`). AnonymousReportError, - /// Resolves elided lifetimes to `'static`, but gives a warning that this behavior - /// is a bug and will be reverted soon. - AnonymousWarn(NodeId), + /// Resolves elided lifetimes to `'static` if there are no other lifetimes in scope, + /// otherwise give a warning that the previous behavior of introducing a new early-bound + /// lifetime is a bug and will be removed. + StaticIfNoLifetimeInScope(NodeId), /// Signal we cannot find which should be the anonymous lifetime. ElisionFailure, @@ -1212,7 +1213,7 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast, } LifetimeRibKind::AnonymousCreateParameter { .. } | LifetimeRibKind::AnonymousReportError - | LifetimeRibKind::AnonymousWarn(_) + | LifetimeRibKind::StaticIfNoLifetimeInScope(_) | LifetimeRibKind::Elided(_) | LifetimeRibKind::ElisionFailure | LifetimeRibKind::ConcreteAnonConst(_) @@ -1580,7 +1581,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // lifetime would be illegal. LifetimeRibKind::Item | LifetimeRibKind::AnonymousReportError - | LifetimeRibKind::AnonymousWarn(_) + | LifetimeRibKind::StaticIfNoLifetimeInScope(_) | LifetimeRibKind::ElisionFailure => Some(LifetimeUseSet::Many), // An anonymous lifetime is legal here, and bound to the right // place, go ahead. @@ -1643,7 +1644,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { | LifetimeRibKind::Generics { .. } | LifetimeRibKind::ElisionFailure | LifetimeRibKind::AnonymousReportError - | LifetimeRibKind::AnonymousWarn(_) => {} + | LifetimeRibKind::StaticIfNoLifetimeInScope(_) => {} } } @@ -1677,16 +1678,36 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { self.record_lifetime_res(lifetime.id, res, elision_candidate); return; } - LifetimeRibKind::AnonymousWarn(node_id) => { - self.r.lint_buffer.buffer_lint( - lint::builtin::ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT, - node_id, - lifetime.ident.span, - lint::BuiltinLintDiag::AssociatedConstElidedLifetime { - elided, - span: lifetime.ident.span, - }, - ); + LifetimeRibKind::StaticIfNoLifetimeInScope(node_id) => { + let mut lifetimes_in_scope = vec![]; + for rib in &self.lifetime_ribs[..i] { + lifetimes_in_scope.extend(rib.bindings.iter().map(|(ident, _)| ident.span)); + // Consider any anonymous lifetimes, too + if let LifetimeRibKind::AnonymousCreateParameter { binder, .. } = rib.kind + && let Some(extra) = self.r.extra_lifetime_params_map.get(&binder) + { + lifetimes_in_scope.extend(extra.iter().map(|(ident, _, _)| ident.span)); + } + } + if lifetimes_in_scope.is_empty() { + self.record_lifetime_res( + lifetime.id, + LifetimeRes::Static, + elision_candidate, + ); + return; + } else { + self.r.lint_buffer.buffer_lint( + lint::builtin::ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT, + node_id, + lifetime.ident.span, + lint::BuiltinLintDiag::AssociatedConstElidedLifetime { + elided, + span: lifetime.ident.span, + lifetimes_in_scope: lifetimes_in_scope.into(), + }, + ); + } } LifetimeRibKind::AnonymousReportError => { if elided { @@ -1904,7 +1925,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // impl Foo for std::cell::Ref // note lack of '_ // async fn foo(_: std::cell::Ref) { ... } LifetimeRibKind::AnonymousCreateParameter { report_in_path: true, .. } - | LifetimeRibKind::AnonymousWarn(_) => { + | LifetimeRibKind::StaticIfNoLifetimeInScope(_) => { let sess = self.r.tcx.sess; let subdiag = rustc_errors::elided_lifetime_in_path_suggestion( sess.source_map(), @@ -3030,30 +3051,33 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { kind: LifetimeBinderKind::ConstItem, }, |this| { - this.with_lifetime_rib(LifetimeRibKind::AnonymousWarn(item.id), |this| { - // If this is a trait impl, ensure the const - // exists in trait - this.check_trait_item( - item.id, - item.ident, - &item.kind, - ValueNS, - item.span, - seen_trait_items, - |i, s, c| ConstNotMemberOfTrait(i, s, c), - ); + this.with_lifetime_rib( + LifetimeRibKind::StaticIfNoLifetimeInScope(item.id), + |this| { + // If this is a trait impl, ensure the const + // exists in trait + this.check_trait_item( + item.id, + item.ident, + &item.kind, + ValueNS, + item.span, + seen_trait_items, + |i, s, c| ConstNotMemberOfTrait(i, s, c), + ); - this.visit_generics(generics); - this.visit_ty(ty); - if let Some(expr) = expr { - // We allow arbitrary const expressions inside of associated consts, - // even if they are potentially not const evaluatable. - // - // Type parameters can already be used and as associated consts are - // not used as part of the type system, this is far less surprising. - this.resolve_const_body(expr, None); - } - }); + this.visit_generics(generics); + this.visit_ty(ty); + if let Some(expr) = expr { + // We allow arbitrary const expressions inside of associated consts, + // even if they are potentially not const evaluatable. + // + // Type parameters can already be used and as associated consts are + // not used as part of the type system, this is far less surprising. + this.resolve_const_body(expr, None); + } + }, + ); }, ); } diff --git a/tests/ui/associated-consts/double-elided.rs b/tests/ui/associated-consts/double-elided.rs index fd0317781bb15..6831b599374ad 100644 --- a/tests/ui/associated-consts/double-elided.rs +++ b/tests/ui/associated-consts/double-elided.rs @@ -1,12 +1,10 @@ +//@ check-pass + struct S; impl S { const C: &&str = &""; - //~^ WARN `&` without an explicit lifetime name cannot be used here - //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - //~| WARN `&` without an explicit lifetime name cannot be used here - //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - //~| ERROR in type `&&str`, reference has a longer lifetime than the data it references + // Now resolves to `&'static &'static str`. } fn main() {} diff --git a/tests/ui/associated-consts/double-elided.stderr b/tests/ui/associated-consts/double-elided.stderr deleted file mode 100644 index 67834788ccd4a..0000000000000 --- a/tests/ui/associated-consts/double-elided.stderr +++ /dev/null @@ -1,47 +0,0 @@ -warning: `&` without an explicit lifetime name cannot be used here - --> $DIR/double-elided.rs:4:14 - | -LL | const C: &&str = &""; - | ^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #115010 - = note: `#[warn(elided_lifetimes_in_associated_constant)]` on by default -help: use the `'static` lifetime - | -LL | const C: &'static &str = &""; - | +++++++ - -warning: `&` without an explicit lifetime name cannot be used here - --> $DIR/double-elided.rs:4:15 - | -LL | const C: &&str = &""; - | ^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #115010 -help: use the `'static` lifetime - | -LL | const C: &&'static str = &""; - | +++++++ - -error[E0491]: in type `&&str`, reference has a longer lifetime than the data it references - --> $DIR/double-elided.rs:4:5 - | -LL | const C: &&str = &""; - | ^^^^^^^^^^^^^^^^^^^^^ - | -note: the pointer is valid for the anonymous lifetime as defined here - --> $DIR/double-elided.rs:4:14 - | -LL | const C: &&str = &""; - | ^ -note: but the referenced data is only valid for the anonymous lifetime as defined here - --> $DIR/double-elided.rs:4:14 - | -LL | const C: &&str = &""; - | ^ - -error: aborting due to 1 previous error; 2 warnings emitted - -For more information about this error, try `rustc --explain E0491`. diff --git a/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.rs b/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.rs index 1e0b77b0d3bd3..40896c32e1113 100644 --- a/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.rs +++ b/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.rs @@ -5,8 +5,6 @@ trait Trait { impl Trait for () { const ASSOC: &dyn Fn(_) = 1i32; //~^ ERROR the placeholder `_` is not allowed within types on item signatures for associated constants - //~| WARN `&` without an explicit lifetime name cannot be used here - //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! } fn main() {} diff --git a/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.stderr b/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.stderr index e946491a08482..6b8ba9af7a072 100644 --- a/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.stderr +++ b/tests/ui/associated-consts/infer-placeholder-in-non-suggestable-pos.stderr @@ -1,23 +1,9 @@ -warning: `&` without an explicit lifetime name cannot be used here - --> $DIR/infer-placeholder-in-non-suggestable-pos.rs:6:18 - | -LL | const ASSOC: &dyn Fn(_) = 1i32; - | ^ - | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #115010 - = note: `#[warn(elided_lifetimes_in_associated_constant)]` on by default -help: use the `'static` lifetime - | -LL | const ASSOC: &'static dyn Fn(_) = 1i32; - | +++++++ - error[E0121]: the placeholder `_` is not allowed within types on item signatures for associated constants --> $DIR/infer-placeholder-in-non-suggestable-pos.rs:6:26 | LL | const ASSOC: &dyn Fn(_) = 1i32; | ^ not allowed in type signatures -error: aborting due to 1 previous error; 1 warning emitted +error: aborting due to 1 previous error For more information about this error, try `rustc --explain E0121`. diff --git a/tests/ui/consts/assoc-const-elided-lifetime.stderr b/tests/ui/consts/assoc-const-elided-lifetime.stderr index a1eeaff4ba87b..3e847298c356a 100644 --- a/tests/ui/consts/assoc-const-elided-lifetime.stderr +++ b/tests/ui/consts/assoc-const-elided-lifetime.stderr @@ -6,6 +6,11 @@ LL | const FOO: Foo<'_> = Foo { x: PhantomData::<&()> }; | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #115010 +note: cannot automatically infer `'static` because of other lifetimes in scope + --> $DIR/assoc-const-elided-lifetime.rs:9:6 + | +LL | impl<'a> Foo<'a> { + | ^^ note: the lint level is defined here --> $DIR/assoc-const-elided-lifetime.rs:1:9 | @@ -24,6 +29,13 @@ LL | const BAR: &() = &(); | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #115010 +note: cannot automatically infer `'static` because of other lifetimes in scope + --> $DIR/assoc-const-elided-lifetime.rs:9:6 + | +LL | impl<'a> Foo<'a> { + | ^^ +LL | const FOO: Foo<'_> = Foo { x: PhantomData::<&()> }; + | ^^ help: use the `'static` lifetime | LL | const BAR: &'static () = &(); From 805397c16f7629f1357d59c676f3a6d4c9e96244 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 13 Jun 2024 20:33:43 -0400 Subject: [PATCH 05/19] Add more tests --- .../elided-lifetime.rs | 23 ++++++++ .../elided-lifetime.stderr | 59 +++++++++++++++++++ .../generic-associated-const.rs | 20 +++++++ .../generic-associated-const.stderr | 57 ++++++++++++++++++ .../static-default-lifetime/inner-item.rs | 21 +++++++ .../static-trait-impl.rs | 20 +++++++ .../static-trait-impl.stderr | 45 ++++++++++++++ 7 files changed, 245 insertions(+) create mode 100644 tests/ui/consts/static-default-lifetime/elided-lifetime.rs create mode 100644 tests/ui/consts/static-default-lifetime/elided-lifetime.stderr create mode 100644 tests/ui/consts/static-default-lifetime/generic-associated-const.rs create mode 100644 tests/ui/consts/static-default-lifetime/generic-associated-const.stderr create mode 100644 tests/ui/consts/static-default-lifetime/inner-item.rs create mode 100644 tests/ui/consts/static-default-lifetime/static-trait-impl.rs create mode 100644 tests/ui/consts/static-default-lifetime/static-trait-impl.stderr diff --git a/tests/ui/consts/static-default-lifetime/elided-lifetime.rs b/tests/ui/consts/static-default-lifetime/elided-lifetime.rs new file mode 100644 index 0000000000000..e1d9791625b34 --- /dev/null +++ b/tests/ui/consts/static-default-lifetime/elided-lifetime.rs @@ -0,0 +1,23 @@ +#![deny(elided_lifetimes_in_associated_constant)] + +struct Foo<'a>(&'a ()); + +impl Foo<'_> { + const STATIC: &str = ""; + //~^ ERROR `&` without an explicit lifetime name cannot be used here + //~| WARN this was previously accepted by the compiler but is being phased out +} + +trait Bar { + const STATIC: &'static str; + // TODO^ +} + +impl Bar for Foo<'_> { + const STATIC: &str = ""; + //~^ ERROR `&` without an explicit lifetime name cannot be used here + //~| WARN this was previously accepted by the compiler but is being phased out + //~| ERROR const not compatible with trait +} + +fn main() {} diff --git a/tests/ui/consts/static-default-lifetime/elided-lifetime.stderr b/tests/ui/consts/static-default-lifetime/elided-lifetime.stderr new file mode 100644 index 0000000000000..8be225e2a3ce9 --- /dev/null +++ b/tests/ui/consts/static-default-lifetime/elided-lifetime.stderr @@ -0,0 +1,59 @@ +error: `&` without an explicit lifetime name cannot be used here + --> $DIR/elided-lifetime.rs:6:19 + | +LL | const STATIC: &str = ""; + | ^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #115010 +note: cannot automatically infer `'static` because of other lifetimes in scope + --> $DIR/elided-lifetime.rs:5:10 + | +LL | impl Foo<'_> { + | ^^ +note: the lint level is defined here + --> $DIR/elided-lifetime.rs:1:9 + | +LL | #![deny(elided_lifetimes_in_associated_constant)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: use the `'static` lifetime + | +LL | const STATIC: &'static str = ""; + | +++++++ + +error: `&` without an explicit lifetime name cannot be used here + --> $DIR/elided-lifetime.rs:17:19 + | +LL | const STATIC: &str = ""; + | ^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #115010 +note: cannot automatically infer `'static` because of other lifetimes in scope + --> $DIR/elided-lifetime.rs:16:18 + | +LL | impl Bar for Foo<'_> { + | ^^ +help: use the `'static` lifetime + | +LL | const STATIC: &'static str = ""; + | +++++++ + +error[E0308]: const not compatible with trait + --> $DIR/elided-lifetime.rs:17:5 + | +LL | const STATIC: &str = ""; + | ^^^^^^^^^^^^^^^^^^ lifetime mismatch + | + = note: expected reference `&'static _` + found reference `&_` +note: the anonymous lifetime as defined here... + --> $DIR/elided-lifetime.rs:16:18 + | +LL | impl Bar for Foo<'_> { + | ^^ + = note: ...does not necessarily outlive the static lifetime + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/consts/static-default-lifetime/generic-associated-const.rs b/tests/ui/consts/static-default-lifetime/generic-associated-const.rs new file mode 100644 index 0000000000000..50eb9ab166955 --- /dev/null +++ b/tests/ui/consts/static-default-lifetime/generic-associated-const.rs @@ -0,0 +1,20 @@ +#![deny(elided_lifetimes_in_associated_constant)] +#![feature(generic_const_items)] +//~^ WARN the feature `generic_const_items` is incomplete + +struct A; +impl A { + const GAC_TYPE: &str = ""; + const GAC_LIFETIME<'a>: &str = ""; + //~^ ERROR `&` without an explicit lifetime name cannot be used here + //~| WARN this was previously accepted by the compiler but is being phased out +} + +trait Trait { + const GAC_TYPE: &str = ""; + //~^ ERROR missing lifetime specifier + const GAC_LIFETIME<'a>: &str = ""; + //~^ ERROR missing lifetime specifier +} + +fn main() {} diff --git a/tests/ui/consts/static-default-lifetime/generic-associated-const.stderr b/tests/ui/consts/static-default-lifetime/generic-associated-const.stderr new file mode 100644 index 0000000000000..d6e49c7ac5886 --- /dev/null +++ b/tests/ui/consts/static-default-lifetime/generic-associated-const.stderr @@ -0,0 +1,57 @@ +error[E0106]: missing lifetime specifier + --> $DIR/generic-associated-const.rs:14:24 + | +LL | const GAC_TYPE: &str = ""; + | ^ expected named lifetime parameter + | +help: consider introducing a named lifetime parameter + | +LL | const GAC_TYPE<'a, T>: &'a str = ""; + | +++ ++ + +error[E0106]: missing lifetime specifier + --> $DIR/generic-associated-const.rs:16:29 + | +LL | const GAC_LIFETIME<'a>: &str = ""; + | ^ expected named lifetime parameter + | +help: consider using the `'a` lifetime + | +LL | const GAC_LIFETIME<'a>: &'a str = ""; + | ++ + +warning: the feature `generic_const_items` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/generic-associated-const.rs:2:12 + | +LL | #![feature(generic_const_items)] + | ^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #113521 for more information + = note: `#[warn(incomplete_features)]` on by default + +error: `&` without an explicit lifetime name cannot be used here + --> $DIR/generic-associated-const.rs:8:29 + | +LL | const GAC_LIFETIME<'a>: &str = ""; + | ^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #115010 +note: cannot automatically infer `'static` because of other lifetimes in scope + --> $DIR/generic-associated-const.rs:8:24 + | +LL | const GAC_LIFETIME<'a>: &str = ""; + | ^^ +note: the lint level is defined here + --> $DIR/generic-associated-const.rs:1:9 + | +LL | #![deny(elided_lifetimes_in_associated_constant)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: use the `'static` lifetime + | +LL | const GAC_LIFETIME<'a>: &'static str = ""; + | +++++++ + +error: aborting due to 3 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0106`. diff --git a/tests/ui/consts/static-default-lifetime/inner-item.rs b/tests/ui/consts/static-default-lifetime/inner-item.rs new file mode 100644 index 0000000000000..061b240a40d8b --- /dev/null +++ b/tests/ui/consts/static-default-lifetime/inner-item.rs @@ -0,0 +1,21 @@ +//@ check-pass + +struct Foo<'a>(&'a ()); + +impl<'a> Foo<'a> { + fn hello(self) { + const INNER: &str = ""; + } +} + +impl Foo<'_> { + fn implicit(self) { + const INNER: &str = ""; + } + + fn fn_lifetime(&self) { + const INNER: &str = ""; + } +} + +fn main() {} diff --git a/tests/ui/consts/static-default-lifetime/static-trait-impl.rs b/tests/ui/consts/static-default-lifetime/static-trait-impl.rs new file mode 100644 index 0000000000000..025fda4df5811 --- /dev/null +++ b/tests/ui/consts/static-default-lifetime/static-trait-impl.rs @@ -0,0 +1,20 @@ +#![deny(elided_lifetimes_in_associated_constant)] + +trait Bar<'a> { + const STATIC: &'a str; +} + +struct A; +impl Bar<'_> for A { + const STATIC: &str = ""; + //~^ ERROR `&` without an explicit lifetime name cannot be used here + //~| WARN this was previously accepted by the compiler but is being phased out + //~| ERROR const not compatible with trait +} + +struct B; +impl Bar<'static> for B { + const STATIC: &str = ""; +} + +fn main() {} diff --git a/tests/ui/consts/static-default-lifetime/static-trait-impl.stderr b/tests/ui/consts/static-default-lifetime/static-trait-impl.stderr new file mode 100644 index 0000000000000..8e4c27875ab3c --- /dev/null +++ b/tests/ui/consts/static-default-lifetime/static-trait-impl.stderr @@ -0,0 +1,45 @@ +error: `&` without an explicit lifetime name cannot be used here + --> $DIR/static-trait-impl.rs:9:19 + | +LL | const STATIC: &str = ""; + | ^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #115010 +note: cannot automatically infer `'static` because of other lifetimes in scope + --> $DIR/static-trait-impl.rs:8:10 + | +LL | impl Bar<'_> for A { + | ^^ +note: the lint level is defined here + --> $DIR/static-trait-impl.rs:1:9 + | +LL | #![deny(elided_lifetimes_in_associated_constant)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: use the `'static` lifetime + | +LL | const STATIC: &'static str = ""; + | +++++++ + +error[E0308]: const not compatible with trait + --> $DIR/static-trait-impl.rs:9:5 + | +LL | const STATIC: &str = ""; + | ^^^^^^^^^^^^^^^^^^ lifetime mismatch + | + = note: expected reference `&_` + found reference `&_` +note: the anonymous lifetime as defined here... + --> $DIR/static-trait-impl.rs:8:10 + | +LL | impl Bar<'_> for A { + | ^^ +note: ...does not necessarily outlive the anonymous lifetime as defined here + --> $DIR/static-trait-impl.rs:8:10 + | +LL | impl Bar<'_> for A { + | ^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 5f3357c3c68e555c655c71bb8672db7908622c5b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 13 Jun 2024 20:44:15 -0400 Subject: [PATCH 06/19] Resolve const lifetimes to static in trait too --- compiler/rustc_resolve/src/late.rs | 56 +++++++++++-------- .../elided-lifetime.rs | 3 +- .../elided-lifetime.stderr | 8 +-- .../generic-associated-const.rs | 1 - .../generic-associated-const.stderr | 15 +---- ...ifetime-in-assoc-const-type.default.stderr | 56 ++++--------------- ...ssoc-const-type.generic_const_items.stderr | 46 ++++----------- .../missing-lifetime-in-assoc-const-type.rs | 8 +-- 8 files changed, 69 insertions(+), 124 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 20926366ba5e8..f3c27ca857f05 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -312,8 +312,8 @@ enum LifetimeRibKind { /// Resolves elided lifetimes to `'static` if there are no other lifetimes in scope, /// otherwise give a warning that the previous behavior of introducing a new early-bound - /// lifetime is a bug and will be removed. - StaticIfNoLifetimeInScope(NodeId), + /// lifetime is a bug and will be removed (if `emit_lint` is enabled). + StaticIfNoLifetimeInScope { lint_id: NodeId, emit_lint: bool }, /// Signal we cannot find which should be the anonymous lifetime. ElisionFailure, @@ -1213,7 +1213,7 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast, } LifetimeRibKind::AnonymousCreateParameter { .. } | LifetimeRibKind::AnonymousReportError - | LifetimeRibKind::StaticIfNoLifetimeInScope(_) + | LifetimeRibKind::StaticIfNoLifetimeInScope { .. } | LifetimeRibKind::Elided(_) | LifetimeRibKind::ElisionFailure | LifetimeRibKind::ConcreteAnonConst(_) @@ -1581,7 +1581,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // lifetime would be illegal. LifetimeRibKind::Item | LifetimeRibKind::AnonymousReportError - | LifetimeRibKind::StaticIfNoLifetimeInScope(_) + | LifetimeRibKind::StaticIfNoLifetimeInScope { .. } | LifetimeRibKind::ElisionFailure => Some(LifetimeUseSet::Many), // An anonymous lifetime is legal here, and bound to the right // place, go ahead. @@ -1644,7 +1644,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { | LifetimeRibKind::Generics { .. } | LifetimeRibKind::ElisionFailure | LifetimeRibKind::AnonymousReportError - | LifetimeRibKind::StaticIfNoLifetimeInScope(_) => {} + | LifetimeRibKind::StaticIfNoLifetimeInScope { .. } => {} } } @@ -1678,7 +1678,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { self.record_lifetime_res(lifetime.id, res, elision_candidate); return; } - LifetimeRibKind::StaticIfNoLifetimeInScope(node_id) => { + LifetimeRibKind::StaticIfNoLifetimeInScope { lint_id: node_id, emit_lint } => { let mut lifetimes_in_scope = vec![]; for rib in &self.lifetime_ribs[..i] { lifetimes_in_scope.extend(rib.bindings.iter().map(|(ident, _)| ident.span)); @@ -1696,7 +1696,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { elision_candidate, ); return; - } else { + } else if emit_lint { self.r.lint_buffer.buffer_lint( lint::builtin::ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT, node_id, @@ -1925,7 +1925,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { // impl Foo for std::cell::Ref // note lack of '_ // async fn foo(_: std::cell::Ref) { ... } LifetimeRibKind::AnonymousCreateParameter { report_in_path: true, .. } - | LifetimeRibKind::StaticIfNoLifetimeInScope(_) => { + | LifetimeRibKind::StaticIfNoLifetimeInScope { .. } => { let sess = self.r.tcx.sess; let subdiag = rustc_errors::elided_lifetime_in_path_suggestion( sess.source_map(), @@ -2859,19 +2859,27 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { kind: LifetimeBinderKind::ConstItem, }, |this| { - this.visit_generics(generics); - this.visit_ty(ty); - - // Only impose the restrictions of `ConstRibKind` for an - // actual constant expression in a provided default. - if let Some(expr) = expr { - // We allow arbitrary const expressions inside of associated consts, - // even if they are potentially not const evaluatable. - // - // Type parameters can already be used and as associated consts are - // not used as part of the type system, this is far less surprising. - this.resolve_const_body(expr, None); - } + this.with_lifetime_rib( + LifetimeRibKind::StaticIfNoLifetimeInScope { + lint_id: item.id, + emit_lint: false, + }, + |this| { + this.visit_generics(generics); + this.visit_ty(ty); + + // Only impose the restrictions of `ConstRibKind` for an + // actual constant expression in a provided default. + if let Some(expr) = expr { + // We allow arbitrary const expressions inside of associated consts, + // even if they are potentially not const evaluatable. + // + // Type parameters can already be used and as associated consts are + // not used as part of the type system, this is far less surprising. + this.resolve_const_body(expr, None); + } + }, + ) }, ); } @@ -3052,7 +3060,11 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { }, |this| { this.with_lifetime_rib( - LifetimeRibKind::StaticIfNoLifetimeInScope(item.id), + LifetimeRibKind::StaticIfNoLifetimeInScope { + lint_id: item.id, + // In impls, it's not a hard error yet due to backcompat. + emit_lint: true, + }, |this| { // If this is a trait impl, ensure the const // exists in trait diff --git a/tests/ui/consts/static-default-lifetime/elided-lifetime.rs b/tests/ui/consts/static-default-lifetime/elided-lifetime.rs index e1d9791625b34..95d59f9b894e4 100644 --- a/tests/ui/consts/static-default-lifetime/elided-lifetime.rs +++ b/tests/ui/consts/static-default-lifetime/elided-lifetime.rs @@ -9,8 +9,7 @@ impl Foo<'_> { } trait Bar { - const STATIC: &'static str; - // TODO^ + const STATIC: &str; } impl Bar for Foo<'_> { diff --git a/tests/ui/consts/static-default-lifetime/elided-lifetime.stderr b/tests/ui/consts/static-default-lifetime/elided-lifetime.stderr index 8be225e2a3ce9..d4fc171753894 100644 --- a/tests/ui/consts/static-default-lifetime/elided-lifetime.stderr +++ b/tests/ui/consts/static-default-lifetime/elided-lifetime.stderr @@ -22,7 +22,7 @@ LL | const STATIC: &'static str = ""; | +++++++ error: `&` without an explicit lifetime name cannot be used here - --> $DIR/elided-lifetime.rs:17:19 + --> $DIR/elided-lifetime.rs:16:19 | LL | const STATIC: &str = ""; | ^ @@ -30,7 +30,7 @@ LL | const STATIC: &str = ""; = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #115010 note: cannot automatically infer `'static` because of other lifetimes in scope - --> $DIR/elided-lifetime.rs:16:18 + --> $DIR/elided-lifetime.rs:15:18 | LL | impl Bar for Foo<'_> { | ^^ @@ -40,7 +40,7 @@ LL | const STATIC: &'static str = ""; | +++++++ error[E0308]: const not compatible with trait - --> $DIR/elided-lifetime.rs:17:5 + --> $DIR/elided-lifetime.rs:16:5 | LL | const STATIC: &str = ""; | ^^^^^^^^^^^^^^^^^^ lifetime mismatch @@ -48,7 +48,7 @@ LL | const STATIC: &str = ""; = note: expected reference `&'static _` found reference `&_` note: the anonymous lifetime as defined here... - --> $DIR/elided-lifetime.rs:16:18 + --> $DIR/elided-lifetime.rs:15:18 | LL | impl Bar for Foo<'_> { | ^^ diff --git a/tests/ui/consts/static-default-lifetime/generic-associated-const.rs b/tests/ui/consts/static-default-lifetime/generic-associated-const.rs index 50eb9ab166955..721920bd810cb 100644 --- a/tests/ui/consts/static-default-lifetime/generic-associated-const.rs +++ b/tests/ui/consts/static-default-lifetime/generic-associated-const.rs @@ -12,7 +12,6 @@ impl A { trait Trait { const GAC_TYPE: &str = ""; - //~^ ERROR missing lifetime specifier const GAC_LIFETIME<'a>: &str = ""; //~^ ERROR missing lifetime specifier } diff --git a/tests/ui/consts/static-default-lifetime/generic-associated-const.stderr b/tests/ui/consts/static-default-lifetime/generic-associated-const.stderr index d6e49c7ac5886..2ecab3442a96e 100644 --- a/tests/ui/consts/static-default-lifetime/generic-associated-const.stderr +++ b/tests/ui/consts/static-default-lifetime/generic-associated-const.stderr @@ -1,16 +1,5 @@ error[E0106]: missing lifetime specifier - --> $DIR/generic-associated-const.rs:14:24 - | -LL | const GAC_TYPE: &str = ""; - | ^ expected named lifetime parameter - | -help: consider introducing a named lifetime parameter - | -LL | const GAC_TYPE<'a, T>: &'a str = ""; - | +++ ++ - -error[E0106]: missing lifetime specifier - --> $DIR/generic-associated-const.rs:16:29 + --> $DIR/generic-associated-const.rs:15:29 | LL | const GAC_LIFETIME<'a>: &str = ""; | ^ expected named lifetime parameter @@ -52,6 +41,6 @@ help: use the `'static` lifetime LL | const GAC_LIFETIME<'a>: &'static str = ""; | +++++++ -error: aborting due to 3 previous errors; 1 warning emitted +error: aborting due to 2 previous errors; 1 warning emitted For more information about this error, try `rustc --explain E0106`. diff --git a/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.default.stderr b/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.default.stderr index 24e2e0a0f7a25..71ac55bf04483 100644 --- a/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.default.stderr +++ b/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.default.stderr @@ -1,57 +1,25 @@ -error[E0106]: missing lifetime specifier - --> $DIR/missing-lifetime-in-assoc-const-type.rs:6:14 - | -LL | const A: &str = ""; - | ^ expected named lifetime parameter - | -help: consider introducing a named lifetime parameter - | -LL ~ trait ZstAssert<'a>: Sized { -LL ~ const A: &'a str = ""; - | - -error[E0106]: missing lifetime specifier +error[E0726]: implicit elided lifetime not allowed here --> $DIR/missing-lifetime-in-assoc-const-type.rs:7:14 | LL | const B: S = S { s: &() }; - | ^ expected named lifetime parameter - | -help: consider introducing a named lifetime parameter - | -LL ~ trait ZstAssert<'a>: Sized { -LL | const A: &str = ""; -LL ~ const B: S<'a> = S { s: &() }; + | ^ expected lifetime parameter | - -error[E0106]: missing lifetime specifier - --> $DIR/missing-lifetime-in-assoc-const-type.rs:8:15 - | -LL | const C: &'_ str = ""; - | ^^ expected named lifetime parameter - | -help: consider introducing a named lifetime parameter - | -LL ~ trait ZstAssert<'a>: Sized { -LL | const A: &str = ""; -LL | const B: S = S { s: &() }; -LL ~ const C: &'a str = ""; +help: indicate the anonymous lifetime | +LL | const B: S<'_> = S { s: &() }; + | ++++ -error[E0106]: missing lifetime specifiers +error[E0726]: implicit elided lifetime not allowed here --> $DIR/missing-lifetime-in-assoc-const-type.rs:9:14 | LL | const D: T = T { a: &(), b: &() }; - | ^ expected 2 lifetime parameters - | -help: consider introducing a named lifetime parameter + | ^ expected lifetime parameters | -LL ~ trait ZstAssert<'a>: Sized { -LL | const A: &str = ""; -LL | const B: S = S { s: &() }; -LL | const C: &'_ str = ""; -LL ~ const D: T<'a, 'a> = T { a: &(), b: &() }; +help: indicate the anonymous lifetimes | +LL | const D: T<'_, '_> = T { a: &(), b: &() }; + | ++++++++ -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0106`. +For more information about this error, try `rustc --explain E0726`. diff --git a/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.generic_const_items.stderr b/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.generic_const_items.stderr index a97ffe7da79eb..71ac55bf04483 100644 --- a/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.generic_const_items.stderr +++ b/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.generic_const_items.stderr @@ -1,47 +1,25 @@ -error[E0106]: missing lifetime specifier - --> $DIR/missing-lifetime-in-assoc-const-type.rs:6:14 - | -LL | const A: &str = ""; - | ^ expected named lifetime parameter - | -help: consider introducing a named lifetime parameter - | -LL | const A<'a>: &'a str = ""; - | ++++ ++ - -error[E0106]: missing lifetime specifier +error[E0726]: implicit elided lifetime not allowed here --> $DIR/missing-lifetime-in-assoc-const-type.rs:7:14 | LL | const B: S = S { s: &() }; - | ^ expected named lifetime parameter - | -help: consider introducing a named lifetime parameter - | -LL | const B<'a>: S<'a> = S { s: &() }; - | ++++ ++++ - -error[E0106]: missing lifetime specifier - --> $DIR/missing-lifetime-in-assoc-const-type.rs:8:15 - | -LL | const C: &'_ str = ""; - | ^^ expected named lifetime parameter + | ^ expected lifetime parameter | -help: consider introducing a named lifetime parameter +help: indicate the anonymous lifetime | -LL | const C<'a>: &'a str = ""; - | ++++ ~~ +LL | const B: S<'_> = S { s: &() }; + | ++++ -error[E0106]: missing lifetime specifiers +error[E0726]: implicit elided lifetime not allowed here --> $DIR/missing-lifetime-in-assoc-const-type.rs:9:14 | LL | const D: T = T { a: &(), b: &() }; - | ^ expected 2 lifetime parameters + | ^ expected lifetime parameters | -help: consider introducing a named lifetime parameter +help: indicate the anonymous lifetimes | -LL | const D<'a>: T<'a, 'a> = T { a: &(), b: &() }; - | ++++ ++++++++ +LL | const D: T<'_, '_> = T { a: &(), b: &() }; + | ++++++++ -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0106`. +For more information about this error, try `rustc --explain E0726`. diff --git a/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.rs b/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.rs index f36a6567e423e..a60f0b94587f8 100644 --- a/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.rs +++ b/tests/ui/suggestions/missing-lifetime-in-assoc-const-type.rs @@ -3,10 +3,10 @@ #![cfg_attr(generic_const_items, feature(generic_const_items), allow(incomplete_features))] trait ZstAssert: Sized { - const A: &str = ""; //~ ERROR missing lifetime specifier - const B: S = S { s: &() }; //~ ERROR missing lifetime specifier - const C: &'_ str = ""; //~ ERROR missing lifetime specifier - const D: T = T { a: &(), b: &() }; //~ ERROR missing lifetime specifier + const A: &str = ""; + const B: S = S { s: &() }; //~ ERROR implicit elided lifetime not allowed here + const C: &'_ str = ""; + const D: T = T { a: &(), b: &() }; //~ ERROR implicit elided lifetime not allowed here } struct S<'a> { From 6cce48838b230ed429cb8e69d207993ebff00dd7 Mon Sep 17 00:00:00 2001 From: Slanterns Date: Sun, 16 Jun 2024 06:31:37 +0800 Subject: [PATCH 07/19] update comment --- library/core/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/error.rs b/library/core/src/error.rs index 042a8c9925f38..150e4f3f31857 100644 --- a/library/core/src/error.rs +++ b/library/core/src/error.rs @@ -928,7 +928,7 @@ pub(crate) mod tags { /// An `Option` with a type tag `I`. /// /// Since this struct implements `Erased`, the type can be erased to make a dynamically typed -/// option. The type can be checked dynamically using `Erased::tag_id` and since this is statically +/// option. The type can be checked dynamically using `Tagged::tag_id` and since this is statically /// checked for the concrete type, there is some degree of type safety. #[repr(transparent)] pub(crate) struct TaggedOption<'a, I: tags::Type<'a>>(pub Option); From 240478383bfa73e165bb677e6d5ecafe48523f45 Mon Sep 17 00:00:00 2001 From: Slanterns Date: Sun, 16 Jun 2024 07:43:08 +0800 Subject: [PATCH 08/19] add codegen test for `Error::provide` --- tests/codegen/error-provide.rs | 48 ++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 tests/codegen/error-provide.rs diff --git a/tests/codegen/error-provide.rs b/tests/codegen/error-provide.rs new file mode 100644 index 0000000000000..e80905eee1101 --- /dev/null +++ b/tests/codegen/error-provide.rs @@ -0,0 +1,48 @@ +// Codegen test for #126242 + +//@ compile-flags: -O +#![crate_type = "lib"] +#![feature(error_generic_member_access)] +use std::error::Request; +use std::fmt; + +#[derive(Debug)] +struct MyBacktrace1 {} + +#[derive(Debug)] +struct MyBacktrace2 {} + +#[derive(Debug)] +struct MyBacktrace3 {} + +#[derive(Debug)] +struct MyError { + backtrace1: MyBacktrace1, + backtrace2: MyBacktrace2, + backtrace3: MyBacktrace3, + other: MyBacktrace3, +} + +impl fmt::Display for MyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Example Error") + } +} + +impl std::error::Error for MyError { + // CHECK-LABEL: @provide + #[no_mangle] + fn provide<'a>(&'a self, request: &mut Request<'a>) { + // LLVM should be able to optimize multiple .provide_* calls into a switch table + // and eliminate redundant ones, rather than compare one-by-one. + + // CHECK: switch i64 %{{.*}}, label %{{.*}} [ + // CHECK-COUNT-3: i64 {{.*}}, label %{{.*}} + // CHECK-NEXT: ] + request + .provide_ref::(&self.backtrace1) + .provide_ref::(&self.other) + .provide_ref::(&self.backtrace2) + .provide_ref::(&self.backtrace3); + } +} From e102d2dbd6ce19f1d63d6f88903b0412cd9bb102 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 01:04:40 +1000 Subject: [PATCH 09/19] coverage: More consistent variable names for span processing --- .../rustc_mir_transform/src/coverage/spans.rs | 8 ++-- .../src/coverage/spans/from_mir.rs | 42 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index bb6a666ff73b1..6e3a27ca26145 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -19,11 +19,11 @@ pub(super) fn extract_refined_covspans( basic_coverage_blocks: &CoverageGraph, code_mappings: &mut impl Extend, ) { - let sorted_span_buckets = + let buckets = from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks); - for bucket in sorted_span_buckets { - let refined_spans = refine_sorted_spans(bucket); - code_mappings.extend(refined_spans.into_iter().map(|RefinedCovspan { span, bcb }| { + for covspans in buckets { + let covspans = refine_sorted_spans(covspans); + code_mappings.extend(covspans.into_iter().map(|RefinedCovspan { span, bcb }| { // Each span produced by the refiner represents an ordinary code region. mappings::CodeMapping { span, bcb } })); diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index b1f71035ddede..be2e078031642 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -31,7 +31,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( ) -> Vec> { let &ExtractedHirInfo { body_span, .. } = hir_info; - let mut initial_spans = vec![]; + let mut covspans = vec![]; let mut holes = vec![]; for (bcb, bcb_data) in basic_coverage_blocks.iter_enumerated() { @@ -40,36 +40,36 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( body_span, bcb, bcb_data, - &mut initial_spans, + &mut covspans, &mut holes, ); } // Only add the signature span if we found at least one span in the body. - if !initial_spans.is_empty() || !holes.is_empty() { + if !covspans.is_empty() || !holes.is_empty() { // If there is no usable signature span, add a fake one (before refinement) // to avoid an ugly gap between the body start and the first real span. // FIXME: Find a more principled way to solve this problem. let fn_sig_span = hir_info.fn_sig_span_extended.unwrap_or_else(|| body_span.shrink_to_lo()); - initial_spans.push(SpanFromMir::for_fn_sig(fn_sig_span)); + covspans.push(SpanFromMir::for_fn_sig(fn_sig_span)); } - initial_spans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); - remove_unwanted_macro_spans(&mut initial_spans); - split_visible_macro_spans(&mut initial_spans); + covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); + remove_unwanted_macro_spans(&mut covspans); + split_visible_macro_spans(&mut covspans); let compare_covspans = |a: &SpanFromMir, b: &SpanFromMir| { compare_spans(a.span, b.span) // After deduplication, we want to keep only the most-dominated BCB. .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) }; - initial_spans.sort_by(compare_covspans); + covspans.sort_by(compare_covspans); // Among covspans with the same span, keep only one, // preferring the one with the most-dominated BCB. // (Ideally we should try to preserve _all_ non-dominating BCBs, but that // requires a lot more complexity in the span refiner, for little benefit.) - initial_spans.dedup_by(|b, a| a.span.source_equal(b.span)); + covspans.dedup_by(|b, a| a.span.source_equal(b.span)); // Sort the holes, and merge overlapping/adjacent holes. holes.sort_by(|a, b| compare_spans(a.span, b.span)); @@ -78,7 +78,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( // Now we're ready to start carving holes out of the initial coverage spans, // and grouping them in buckets separated by the holes. - let mut initial_spans = VecDeque::from(initial_spans); + let mut input_covspans = VecDeque::from(covspans); let mut fragments: Vec = vec![]; // For each hole: @@ -93,10 +93,10 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( // Only inspect spans that precede or overlap this hole, // leaving the rest to be inspected by later holes. // (This relies on the spans and holes both being sorted.) - let relevant_initial_spans = - drain_front_while(&mut initial_spans, |c| c.span.lo() < hole.span.hi()); + let relevant_input_covspans = + drain_front_while(&mut input_covspans, |c| c.span.lo() < hole.span.hi()); - for covspan in fragments_from_prev.into_iter().chain(relevant_initial_spans) { + for covspan in fragments_from_prev.into_iter().chain(relevant_input_covspans) { let (before, after) = covspan.split_around_hole_span(hole.span); bucket.extend(before); fragments.extend(after); @@ -106,12 +106,12 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( // After finding the spans before each hole, any remaining fragments/spans // form their own final bucket, after the final hole. // (If there were no holes, this will just be all of the initial spans.) - fragments.extend(initial_spans); + fragments.extend(input_covspans); buckets.push(fragments); // Make sure each individual bucket is still internally sorted. - for bucket in &mut buckets { - bucket.sort_by(compare_covspans); + for covspans in &mut buckets { + covspans.sort_by(compare_covspans); } buckets } @@ -143,9 +143,9 @@ fn drain_front_while<'a, T>( /// /// (The input spans should be sorted in BCB dominator order, so that the /// retained "first" span is likely to dominate the others.) -fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { +fn remove_unwanted_macro_spans(covspans: &mut Vec) { let mut seen_macro_spans = FxHashSet::default(); - initial_spans.retain(|covspan| { + covspans.retain(|covspan| { // Ignore (retain) non-macro-expansion spans. if covspan.visible_macro.is_none() { return true; @@ -160,10 +160,10 @@ fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { /// function body, split it into two parts. The first part covers just the /// macro name plus `!`, and the second part covers the rest of the macro /// invocation. This seems to give better results for code that uses macros. -fn split_visible_macro_spans(initial_spans: &mut Vec) { +fn split_visible_macro_spans(covspans: &mut Vec) { let mut extra_spans = vec![]; - initial_spans.retain(|covspan| { + covspans.retain(|covspan| { let Some(visible_macro) = covspan.visible_macro else { return true }; let split_len = visible_macro.as_str().len() as u32 + 1; @@ -183,7 +183,7 @@ fn split_visible_macro_spans(initial_spans: &mut Vec) { // The newly-split spans are added at the end, so any previous sorting // is not preserved. - initial_spans.extend(extra_spans); + covspans.extend(extra_spans); } // Generate a set of coverage spans from the filtered set of `Statement`s and `Terminator`s of From bf74fb1d2f39afa6948e8be39a6a2e72fcc2e83f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 01:11:32 +1000 Subject: [PATCH 10/19] coverage: Move most span processing back into `coverage::spans` --- .../rustc_mir_transform/src/coverage/spans.rs | 144 ++++++++++++++- .../src/coverage/spans/from_mir.rs | 168 ++---------------- 2 files changed, 157 insertions(+), 155 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 6e3a27ca26145..11fc297911ead 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -1,9 +1,15 @@ +use std::collections::VecDeque; + +use rustc_data_structures::captures::Captures; +use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir; use rustc_span::Span; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; use crate::coverage::mappings; -use crate::coverage::spans::from_mir::SpanFromMir; +use crate::coverage::spans::from_mir::{ + extract_covspans_and_holes_from_mir, ExtractedCovspans, SpanFromMir, +}; use crate::coverage::ExtractedHirInfo; mod from_mir; @@ -19,9 +25,68 @@ pub(super) fn extract_refined_covspans( basic_coverage_blocks: &CoverageGraph, code_mappings: &mut impl Extend, ) { - let buckets = - from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks); - for covspans in buckets { + let ExtractedCovspans { mut covspans, mut holes } = + extract_covspans_and_holes_from_mir(mir_body, hir_info, basic_coverage_blocks); + + covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); + remove_unwanted_macro_spans(&mut covspans); + split_visible_macro_spans(&mut covspans); + + let compare_covspans = |a: &SpanFromMir, b: &SpanFromMir| { + compare_spans(a.span, b.span) + // After deduplication, we want to keep only the most-dominated BCB. + .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) + }; + covspans.sort_by(compare_covspans); + + // Among covspans with the same span, keep only one, + // preferring the one with the most-dominated BCB. + // (Ideally we should try to preserve _all_ non-dominating BCBs, but that + // requires a lot more complexity in the span refiner, for little benefit.) + covspans.dedup_by(|b, a| a.span.source_equal(b.span)); + + // Sort the holes, and merge overlapping/adjacent holes. + holes.sort_by(|a, b| compare_spans(a.span, b.span)); + holes.dedup_by(|b, a| a.merge_if_overlapping_or_adjacent(b)); + + // Now we're ready to start carving holes out of the initial coverage spans, + // and grouping them in buckets separated by the holes. + + let mut input_covspans = VecDeque::from(covspans); + let mut fragments: Vec = vec![]; + + // For each hole: + // - Identify the spans that are entirely or partly before the hole. + // - Put those spans in a corresponding bucket, truncated to the start of the hole. + // - If one of those spans also extends after the hole, put the rest of it + // in a "fragments" vector that is processed by the next hole. + let mut buckets = (0..holes.len()).map(|_| vec![]).collect::>(); + for (hole, bucket) in holes.iter().zip(&mut buckets) { + let fragments_from_prev = std::mem::take(&mut fragments); + + // Only inspect spans that precede or overlap this hole, + // leaving the rest to be inspected by later holes. + // (This relies on the spans and holes both being sorted.) + let relevant_input_covspans = + drain_front_while(&mut input_covspans, |c| c.span.lo() < hole.span.hi()); + + for covspan in fragments_from_prev.into_iter().chain(relevant_input_covspans) { + let (before, after) = covspan.split_around_hole_span(hole.span); + bucket.extend(before); + fragments.extend(after); + } + } + + // After finding the spans before each hole, any remaining fragments/spans + // form their own final bucket, after the final hole. + // (If there were no holes, this will just be all of the initial spans.) + fragments.extend(input_covspans); + buckets.push(fragments); + + for mut covspans in buckets { + // Make sure each individual bucket is internally sorted. + covspans.sort_by(compare_covspans); + let covspans = refine_sorted_spans(covspans); code_mappings.extend(covspans.into_iter().map(|RefinedCovspan { span, bcb }| { // Each span produced by the refiner represents an ordinary code region. @@ -30,6 +95,56 @@ pub(super) fn extract_refined_covspans( } } +/// Macros that expand into branches (e.g. `assert!`, `trace!`) tend to generate +/// multiple condition/consequent blocks that have the span of the whole macro +/// invocation, which is unhelpful. Keeping only the first such span seems to +/// give better mappings, so remove the others. +/// +/// (The input spans should be sorted in BCB dominator order, so that the +/// retained "first" span is likely to dominate the others.) +fn remove_unwanted_macro_spans(covspans: &mut Vec) { + let mut seen_macro_spans = FxHashSet::default(); + covspans.retain(|covspan| { + // Ignore (retain) non-macro-expansion spans. + if covspan.visible_macro.is_none() { + return true; + } + + // Retain only the first macro-expanded covspan with this span. + seen_macro_spans.insert(covspan.span) + }); +} + +/// When a span corresponds to a macro invocation that is visible from the +/// function body, split it into two parts. The first part covers just the +/// macro name plus `!`, and the second part covers the rest of the macro +/// invocation. This seems to give better results for code that uses macros. +fn split_visible_macro_spans(covspans: &mut Vec) { + let mut extra_spans = vec![]; + + covspans.retain(|covspan| { + let Some(visible_macro) = covspan.visible_macro else { return true }; + + let split_len = visible_macro.as_str().len() as u32 + 1; + let (before, after) = covspan.span.split_at(split_len); + if !covspan.span.contains(before) || !covspan.span.contains(after) { + // Something is unexpectedly wrong with the split point. + // The debug assertion in `split_at` will have already caught this, + // but in release builds it's safer to do nothing and maybe get a + // bug report for unexpected coverage, rather than risk an ICE. + return true; + } + + extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb)); + extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb)); + false // Discard the original covspan that we just split. + }); + + // The newly-split spans are added at the end, so any previous sorting + // is not preserved. + covspans.extend(extra_spans); +} + #[derive(Debug)] struct RefinedCovspan { span: Span, @@ -47,6 +162,15 @@ impl RefinedCovspan { } } +/// Similar to `.drain(..)`, but stops just before it would remove an item not +/// satisfying the predicate. +fn drain_front_while<'a, T>( + queue: &'a mut VecDeque, + mut pred_fn: impl FnMut(&T) -> bool, +) -> impl Iterator + Captures<'a> { + std::iter::from_fn(move || if pred_fn(queue.front()?) { queue.pop_front() } else { None }) +} + /// Takes one of the buckets of (sorted) spans extracted from MIR, and "refines" /// those spans by removing spans that overlap in unwanted ways, and by merging /// compatible adjacent spans. @@ -94,3 +218,15 @@ fn refine_sorted_spans(sorted_spans: Vec) -> Vec { refined } + +/// Compares two spans in (lo ascending, hi descending) order. +fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering { + // First sort by span start. + Ord::cmp(&a.lo(), &b.lo()) + // If span starts are the same, sort by span end in reverse order. + // This ensures that if spans A and B are adjacent in the list, + // and they overlap but are not equal, then either: + // - Span A extends further left, or + // - Both have the same start and span A extends further right + .then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse()) +} diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index be2e078031642..1508a893b5826 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -1,7 +1,3 @@ -use std::collections::VecDeque; - -use rustc_data_structures::captures::Captures; -use rustc_data_structures::fx::FxHashSet; use rustc_middle::bug; use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::{ @@ -15,20 +11,19 @@ use crate::coverage::graph::{ }; use crate::coverage::ExtractedHirInfo; +pub(crate) struct ExtractedCovspans { + pub(crate) covspans: Vec, + pub(crate) holes: Vec, +} + /// Traverses the MIR body to produce an initial collection of coverage-relevant /// spans, each associated with a node in the coverage graph (BCB) and possibly /// other metadata. -/// -/// The returned spans are divided into one or more buckets, such that: -/// - The spans in each bucket are strictly after all spans in previous buckets, -/// and strictly before all spans in subsequent buckets. -/// - The contents of each bucket are also sorted, in a specific order that is -/// expected by the subsequent span-refinement step. -pub(super) fn mir_to_initial_sorted_coverage_spans( +pub(crate) fn extract_covspans_and_holes_from_mir( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, basic_coverage_blocks: &CoverageGraph, -) -> Vec> { +) -> ExtractedCovspans { let &ExtractedHirInfo { body_span, .. } = hir_info; let mut covspans = vec![]; @@ -54,136 +49,7 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( covspans.push(SpanFromMir::for_fn_sig(fn_sig_span)); } - covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); - remove_unwanted_macro_spans(&mut covspans); - split_visible_macro_spans(&mut covspans); - - let compare_covspans = |a: &SpanFromMir, b: &SpanFromMir| { - compare_spans(a.span, b.span) - // After deduplication, we want to keep only the most-dominated BCB. - .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) - }; - covspans.sort_by(compare_covspans); - - // Among covspans with the same span, keep only one, - // preferring the one with the most-dominated BCB. - // (Ideally we should try to preserve _all_ non-dominating BCBs, but that - // requires a lot more complexity in the span refiner, for little benefit.) - covspans.dedup_by(|b, a| a.span.source_equal(b.span)); - - // Sort the holes, and merge overlapping/adjacent holes. - holes.sort_by(|a, b| compare_spans(a.span, b.span)); - holes.dedup_by(|b, a| a.merge_if_overlapping_or_adjacent(b)); - - // Now we're ready to start carving holes out of the initial coverage spans, - // and grouping them in buckets separated by the holes. - - let mut input_covspans = VecDeque::from(covspans); - let mut fragments: Vec = vec![]; - - // For each hole: - // - Identify the spans that are entirely or partly before the hole. - // - Put those spans in a corresponding bucket, truncated to the start of the hole. - // - If one of those spans also extends after the hole, put the rest of it - // in a "fragments" vector that is processed by the next hole. - let mut buckets = (0..holes.len()).map(|_| vec![]).collect::>(); - for (hole, bucket) in holes.iter().zip(&mut buckets) { - let fragments_from_prev = std::mem::take(&mut fragments); - - // Only inspect spans that precede or overlap this hole, - // leaving the rest to be inspected by later holes. - // (This relies on the spans and holes both being sorted.) - let relevant_input_covspans = - drain_front_while(&mut input_covspans, |c| c.span.lo() < hole.span.hi()); - - for covspan in fragments_from_prev.into_iter().chain(relevant_input_covspans) { - let (before, after) = covspan.split_around_hole_span(hole.span); - bucket.extend(before); - fragments.extend(after); - } - } - - // After finding the spans before each hole, any remaining fragments/spans - // form their own final bucket, after the final hole. - // (If there were no holes, this will just be all of the initial spans.) - fragments.extend(input_covspans); - buckets.push(fragments); - - // Make sure each individual bucket is still internally sorted. - for covspans in &mut buckets { - covspans.sort_by(compare_covspans); - } - buckets -} - -fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering { - // First sort by span start. - Ord::cmp(&a.lo(), &b.lo()) - // If span starts are the same, sort by span end in reverse order. - // This ensures that if spans A and B are adjacent in the list, - // and they overlap but are not equal, then either: - // - Span A extends further left, or - // - Both have the same start and span A extends further right - .then_with(|| Ord::cmp(&a.hi(), &b.hi()).reverse()) -} - -/// Similar to `.drain(..)`, but stops just before it would remove an item not -/// satisfying the predicate. -fn drain_front_while<'a, T>( - queue: &'a mut VecDeque, - mut pred_fn: impl FnMut(&T) -> bool, -) -> impl Iterator + Captures<'a> { - std::iter::from_fn(move || if pred_fn(queue.front()?) { queue.pop_front() } else { None }) -} - -/// Macros that expand into branches (e.g. `assert!`, `trace!`) tend to generate -/// multiple condition/consequent blocks that have the span of the whole macro -/// invocation, which is unhelpful. Keeping only the first such span seems to -/// give better mappings, so remove the others. -/// -/// (The input spans should be sorted in BCB dominator order, so that the -/// retained "first" span is likely to dominate the others.) -fn remove_unwanted_macro_spans(covspans: &mut Vec) { - let mut seen_macro_spans = FxHashSet::default(); - covspans.retain(|covspan| { - // Ignore (retain) non-macro-expansion spans. - if covspan.visible_macro.is_none() { - return true; - } - - // Retain only the first macro-expanded covspan with this span. - seen_macro_spans.insert(covspan.span) - }); -} - -/// When a span corresponds to a macro invocation that is visible from the -/// function body, split it into two parts. The first part covers just the -/// macro name plus `!`, and the second part covers the rest of the macro -/// invocation. This seems to give better results for code that uses macros. -fn split_visible_macro_spans(covspans: &mut Vec) { - let mut extra_spans = vec![]; - - covspans.retain(|covspan| { - let Some(visible_macro) = covspan.visible_macro else { return true }; - - let split_len = visible_macro.as_str().len() as u32 + 1; - let (before, after) = covspan.span.split_at(split_len); - if !covspan.span.contains(before) || !covspan.span.contains(after) { - // Something is unexpectedly wrong with the split point. - // The debug assertion in `split_at` will have already caught this, - // but in release builds it's safer to do nothing and maybe get a - // bug report for unexpected coverage, rather than risk an ICE. - return true; - } - - extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb)); - extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb)); - false // Discard the original covspan that we just split. - }); - - // The newly-split spans are added at the end, so any previous sorting - // is not preserved. - covspans.extend(extra_spans); + ExtractedCovspans { covspans, holes } } // Generate a set of coverage spans from the filtered set of `Statement`s and `Terminator`s of @@ -402,12 +268,12 @@ fn unexpand_into_body_span_with_prev( } #[derive(Debug)] -struct Hole { - span: Span, +pub(crate) struct Hole { + pub(crate) span: Span, } impl Hole { - fn merge_if_overlapping_or_adjacent(&mut self, other: &mut Self) -> bool { + pub(crate) fn merge_if_overlapping_or_adjacent(&mut self, other: &mut Self) -> bool { if !self.span.overlaps_or_adjacent(other.span) { return false; } @@ -418,7 +284,7 @@ impl Hole { } #[derive(Debug)] -pub(super) struct SpanFromMir { +pub(crate) struct SpanFromMir { /// A span that has been extracted from MIR and then "un-expanded" back to /// within the current function's `body_span`. After various intermediate /// processing steps, this span is emitted as part of the final coverage @@ -426,9 +292,9 @@ pub(super) struct SpanFromMir { /// /// With the exception of `fn_sig_span`, this should always be contained /// within `body_span`. - pub(super) span: Span, - visible_macro: Option, - pub(super) bcb: BasicCoverageBlock, + pub(crate) span: Span, + pub(crate) visible_macro: Option, + pub(crate) bcb: BasicCoverageBlock, } impl SpanFromMir { @@ -436,14 +302,14 @@ impl SpanFromMir { Self::new(fn_sig_span, None, START_BCB) } - fn new(span: Span, visible_macro: Option, bcb: BasicCoverageBlock) -> Self { + pub(crate) fn new(span: Span, visible_macro: Option, bcb: BasicCoverageBlock) -> Self { Self { span, visible_macro, bcb } } /// Splits this span into 0-2 parts: /// - The part that is strictly before the hole span, if any. /// - The part that is strictly after the hole span, if any. - fn split_around_hole_span(&self, hole_span: Span) -> (Option, Option) { + pub(crate) fn split_around_hole_span(&self, hole_span: Span) -> (Option, Option) { let before = try { let span = self.span.trim_end(hole_span)?; Self { span, ..*self } From 88ade9c740c0f7fad269c51a7ad15baa5cb07115 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 01:25:05 +1000 Subject: [PATCH 11/19] coverage: Eagerly convert coverage spans to a simpler form --- .../rustc_mir_transform/src/coverage/spans.rs | 86 +++++++++++-------- .../src/coverage/spans/from_mir.rs | 18 +--- 2 files changed, 55 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 11fc297911ead..122a7957a1fde 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -28,11 +28,15 @@ pub(super) fn extract_refined_covspans( let ExtractedCovspans { mut covspans, mut holes } = extract_covspans_and_holes_from_mir(mir_body, hir_info, basic_coverage_blocks); + // First, perform the passes that need macro information. covspans.sort_by(|a, b| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb)); remove_unwanted_macro_spans(&mut covspans); split_visible_macro_spans(&mut covspans); - let compare_covspans = |a: &SpanFromMir, b: &SpanFromMir| { + // We no longer need the extra information in `SpanFromMir`, so convert to `Covspan`. + let mut covspans = covspans.into_iter().map(SpanFromMir::into_covspan).collect::>(); + + let compare_covspans = |a: &Covspan, b: &Covspan| { compare_spans(a.span, b.span) // After deduplication, we want to keep only the most-dominated BCB. .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) @@ -53,7 +57,7 @@ pub(super) fn extract_refined_covspans( // and grouping them in buckets separated by the holes. let mut input_covspans = VecDeque::from(covspans); - let mut fragments: Vec = vec![]; + let mut fragments = vec![]; // For each hole: // - Identify the spans that are entirely or partly before the hole. @@ -88,7 +92,7 @@ pub(super) fn extract_refined_covspans( covspans.sort_by(compare_covspans); let covspans = refine_sorted_spans(covspans); - code_mappings.extend(covspans.into_iter().map(|RefinedCovspan { span, bcb }| { + code_mappings.extend(covspans.into_iter().map(|Covspan { span, bcb }| { // Each span produced by the refiner represents an ordinary code region. mappings::CodeMapping { span, bcb } })); @@ -145,23 +149,6 @@ fn split_visible_macro_spans(covspans: &mut Vec) { covspans.extend(extra_spans); } -#[derive(Debug)] -struct RefinedCovspan { - span: Span, - bcb: BasicCoverageBlock, -} - -impl RefinedCovspan { - fn is_mergeable(&self, other: &Self) -> bool { - self.bcb == other.bcb - } - - fn merge_from(&mut self, other: &Self) { - debug_assert!(self.is_mergeable(other)); - self.span = self.span.to(other.span); - } -} - /// Similar to `.drain(..)`, but stops just before it would remove an item not /// satisfying the predicate. fn drain_front_while<'a, T>( @@ -175,18 +162,18 @@ fn drain_front_while<'a, T>( /// those spans by removing spans that overlap in unwanted ways, and by merging /// compatible adjacent spans. #[instrument(level = "debug")] -fn refine_sorted_spans(sorted_spans: Vec) -> Vec { +fn refine_sorted_spans(sorted_spans: Vec) -> Vec { // Holds spans that have been read from the input vector, but haven't yet // been committed to the output vector. let mut pending = vec![]; let mut refined = vec![]; for curr in sorted_spans { - pending.retain(|prev: &SpanFromMir| { + pending.retain(|prev: &Covspan| { if prev.span.hi() <= curr.span.lo() { // There's no overlap between the previous/current covspans, // so move the previous one into the refined list. - refined.push(RefinedCovspan { span: prev.span, bcb: prev.bcb }); + refined.push(prev.clone()); false } else { // Otherwise, retain the previous covspan only if it has the @@ -199,26 +186,55 @@ fn refine_sorted_spans(sorted_spans: Vec) -> Vec { } // Drain the rest of the pending list into the refined list. - for prev in pending { - refined.push(RefinedCovspan { span: prev.span, bcb: prev.bcb }); - } + refined.extend(pending); // Do one last merge pass, to simplify the output. debug!(?refined, "before merge"); - refined.dedup_by(|b, a| { - if a.is_mergeable(b) { - debug!(?a, ?b, "merging list-adjacent refined spans"); - a.merge_from(b); - true - } else { - false - } - }); + refined.dedup_by(|b, a| a.merge_if_eligible(b)); debug!(?refined, "after merge"); refined } +#[derive(Clone, Debug)] +struct Covspan { + span: Span, + bcb: BasicCoverageBlock, +} + +impl Covspan { + /// Splits this covspan into 0-2 parts: + /// - The part that is strictly before the hole span, if any. + /// - The part that is strictly after the hole span, if any. + fn split_around_hole_span(&self, hole_span: Span) -> (Option, Option) { + let before = try { + let span = self.span.trim_end(hole_span)?; + Self { span, ..*self } + }; + let after = try { + let span = self.span.trim_start(hole_span)?; + Self { span, ..*self } + }; + + (before, after) + } + + /// If `self` and `other` can be merged (i.e. they have the same BCB), + /// mutates `self.span` to also include `other.span` and returns true. + /// + /// Note that compatible covspans can be merged even if their underlying + /// spans are not overlapping/adjacent; any space between them will also be + /// part of the merged covspan. + fn merge_if_eligible(&mut self, other: &Self) -> bool { + if self.bcb != other.bcb { + return false; + } + + self.span = self.span.to(other.span); + true + } +} + /// Compares two spans in (lo ascending, hi descending) order. fn compare_spans(a: Span, b: Span) -> std::cmp::Ordering { // First sort by span start. diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 1508a893b5826..09deb7534bfde 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -9,6 +9,7 @@ use rustc_span::{ExpnKind, MacroKind, Span, Symbol}; use crate::coverage::graph::{ BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB, }; +use crate::coverage::spans::Covspan; use crate::coverage::ExtractedHirInfo; pub(crate) struct ExtractedCovspans { @@ -306,19 +307,8 @@ impl SpanFromMir { Self { span, visible_macro, bcb } } - /// Splits this span into 0-2 parts: - /// - The part that is strictly before the hole span, if any. - /// - The part that is strictly after the hole span, if any. - pub(crate) fn split_around_hole_span(&self, hole_span: Span) -> (Option, Option) { - let before = try { - let span = self.span.trim_end(hole_span)?; - Self { span, ..*self } - }; - let after = try { - let span = self.span.trim_start(hole_span)?; - Self { span, ..*self } - }; - - (before, after) + pub(crate) fn into_covspan(self) -> Covspan { + let Self { span, visible_macro: _, bcb } = self; + Covspan { span, bcb } } } From 118f66c237acee3f4a1fd4a773d4b2ccabd93851 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 01:28:22 +1000 Subject: [PATCH 12/19] coverage: Split out a function for dividing coverage spans into buckets --- .../rustc_mir_transform/src/coverage/spans.rs | 86 +++++++++++-------- 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 122a7957a1fde..dbc6718636fa3 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -8,7 +8,7 @@ use rustc_span::Span; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph}; use crate::coverage::mappings; use crate::coverage::spans::from_mir::{ - extract_covspans_and_holes_from_mir, ExtractedCovspans, SpanFromMir, + extract_covspans_and_holes_from_mir, ExtractedCovspans, Hole, SpanFromMir, }; use crate::coverage::ExtractedHirInfo; @@ -53,39 +53,8 @@ pub(super) fn extract_refined_covspans( holes.sort_by(|a, b| compare_spans(a.span, b.span)); holes.dedup_by(|b, a| a.merge_if_overlapping_or_adjacent(b)); - // Now we're ready to start carving holes out of the initial coverage spans, - // and grouping them in buckets separated by the holes. - - let mut input_covspans = VecDeque::from(covspans); - let mut fragments = vec![]; - - // For each hole: - // - Identify the spans that are entirely or partly before the hole. - // - Put those spans in a corresponding bucket, truncated to the start of the hole. - // - If one of those spans also extends after the hole, put the rest of it - // in a "fragments" vector that is processed by the next hole. - let mut buckets = (0..holes.len()).map(|_| vec![]).collect::>(); - for (hole, bucket) in holes.iter().zip(&mut buckets) { - let fragments_from_prev = std::mem::take(&mut fragments); - - // Only inspect spans that precede or overlap this hole, - // leaving the rest to be inspected by later holes. - // (This relies on the spans and holes both being sorted.) - let relevant_input_covspans = - drain_front_while(&mut input_covspans, |c| c.span.lo() < hole.span.hi()); - - for covspan in fragments_from_prev.into_iter().chain(relevant_input_covspans) { - let (before, after) = covspan.split_around_hole_span(hole.span); - bucket.extend(before); - fragments.extend(after); - } - } - - // After finding the spans before each hole, any remaining fragments/spans - // form their own final bucket, after the final hole. - // (If there were no holes, this will just be all of the initial spans.) - fragments.extend(input_covspans); - buckets.push(fragments); + // Split the covspans into separate buckets that don't overlap any holes. + let buckets = divide_spans_into_buckets(covspans, &holes); for mut covspans in buckets { // Make sure each individual bucket is internally sorted. @@ -149,6 +118,55 @@ fn split_visible_macro_spans(covspans: &mut Vec) { covspans.extend(extra_spans); } +/// Uses the holes to divide the given covspans into buckets, such that: +/// - No span in any hole overlaps a bucket (truncating the spans if necessary). +/// - The spans in each bucket are strictly after all spans in previous buckets, +/// and strictly before all spans in subsequent buckets. +/// +/// The resulting buckets are sorted relative to each other, but might not be +/// internally sorted. +#[instrument(level = "debug")] +fn divide_spans_into_buckets(input_covspans: Vec, holes: &[Hole]) -> Vec> { + debug_assert!(input_covspans.is_sorted_by(|a, b| compare_spans(a.span, b.span).is_le())); + debug_assert!(holes.is_sorted_by(|a, b| compare_spans(a.span, b.span).is_le())); + + // Now we're ready to start carving holes out of the initial coverage spans, + // and grouping them in buckets separated by the holes. + + let mut input_covspans = VecDeque::from(input_covspans); + let mut fragments = vec![]; + + // For each hole: + // - Identify the spans that are entirely or partly before the hole. + // - Put those spans in a corresponding bucket, truncated to the start of the hole. + // - If one of those spans also extends after the hole, put the rest of it + // in a "fragments" vector that is processed by the next hole. + let mut buckets = (0..holes.len()).map(|_| vec![]).collect::>(); + for (hole, bucket) in holes.iter().zip(&mut buckets) { + let fragments_from_prev = std::mem::take(&mut fragments); + + // Only inspect spans that precede or overlap this hole, + // leaving the rest to be inspected by later holes. + // (This relies on the spans and holes both being sorted.) + let relevant_input_covspans = + drain_front_while(&mut input_covspans, |c| c.span.lo() < hole.span.hi()); + + for covspan in fragments_from_prev.into_iter().chain(relevant_input_covspans) { + let (before, after) = covspan.split_around_hole_span(hole.span); + bucket.extend(before); + fragments.extend(after); + } + } + + // After finding the spans before each hole, any remaining fragments/spans + // form their own final bucket, after the final hole. + // (If there were no holes, this will just be all of the initial spans.) + fragments.extend(input_covspans); + buckets.push(fragments); + + buckets +} + /// Similar to `.drain(..)`, but stops just before it would remove an item not /// satisfying the predicate. fn drain_front_while<'a, T>( From 2d3e6c88048fd19921346c9e945ccdf64fe97939 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 15 Jun 2024 23:39:04 +1000 Subject: [PATCH 13/19] coverage: Split span refinement into two separate steps --- .../rustc_mir_transform/src/coverage/spans.rs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index dbc6718636fa3..84a70d1f02d75 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -59,8 +59,15 @@ pub(super) fn extract_refined_covspans( for mut covspans in buckets { // Make sure each individual bucket is internally sorted. covspans.sort_by(compare_covspans); + let _span = debug_span!("processing bucket", ?covspans).entered(); + + let mut covspans = remove_unwanted_overlapping_spans(covspans); + debug!(?covspans, "after removing overlaps"); + + // Do one last merge pass, to simplify the output. + covspans.dedup_by(|b, a| a.merge_if_eligible(b)); + debug!(?covspans, "after merge"); - let covspans = refine_sorted_spans(covspans); code_mappings.extend(covspans.into_iter().map(|Covspan { span, bcb }| { // Each span produced by the refiner represents an ordinary code region. mappings::CodeMapping { span, bcb } @@ -177,10 +184,11 @@ fn drain_front_while<'a, T>( } /// Takes one of the buckets of (sorted) spans extracted from MIR, and "refines" -/// those spans by removing spans that overlap in unwanted ways, and by merging -/// compatible adjacent spans. +/// those spans by removing spans that overlap in unwanted ways. #[instrument(level = "debug")] -fn refine_sorted_spans(sorted_spans: Vec) -> Vec { +fn remove_unwanted_overlapping_spans(sorted_spans: Vec) -> Vec { + debug_assert!(sorted_spans.is_sorted_by(|a, b| compare_spans(a.span, b.span).is_le())); + // Holds spans that have been read from the input vector, but haven't yet // been committed to the output vector. let mut pending = vec![]; @@ -205,12 +213,6 @@ fn refine_sorted_spans(sorted_spans: Vec) -> Vec { // Drain the rest of the pending list into the refined list. refined.extend(pending); - - // Do one last merge pass, to simplify the output. - debug!(?refined, "before merge"); - refined.dedup_by(|b, a| a.merge_if_eligible(b)); - debug!(?refined, "after merge"); - refined } From 5eb30f06993ea1dab0926d81f6d407d894b593e8 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 14:44:28 +1000 Subject: [PATCH 14/19] coverage: Remove some old low-value unit tests for graph traversal These tests might have originally been useful as an implementation aid, but now they don't provide enough value to justify the burden of updating them as the underlying code changes. The code they test is still exercised by the main end-to-end coverage tests. --- .../src/coverage/counters.rs | 5 - .../rustc_mir_transform/src/coverage/tests.rs | 106 ------------------ 2 files changed, 111 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index b5968517d772d..a8b0f4a8d6df4 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -168,11 +168,6 @@ impl CoverageCounters { self.counter_increment_sites.len() } - #[cfg(test)] - pub(super) fn num_expressions(&self) -> usize { - self.expressions.len() - } - fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> BcbCounter { if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) { bug!( diff --git a/compiler/rustc_mir_transform/src/coverage/tests.rs b/compiler/rustc_mir_transform/src/coverage/tests.rs index ca64688e6b87b..048547dc9f543 100644 --- a/compiler/rustc_mir_transform/src/coverage/tests.rs +++ b/compiler/rustc_mir_transform/src/coverage/tests.rs @@ -24,7 +24,6 @@ //! globals is comparatively simpler. The easiest way is to wrap the test in a closure argument //! to: `rustc_span::create_default_session_globals_then(|| { test_here(); })`. -use super::counters; use super::graph::{self, BasicCoverageBlock}; use itertools::Itertools; @@ -551,108 +550,3 @@ fn test_covgraph_switchint_loop_then_inner_loop_else_break() { assert_successors(&basic_coverage_blocks, bcb(5), &[bcb(1)]); assert_successors(&basic_coverage_blocks, bcb(6), &[bcb(4)]); } - -#[test] -fn test_find_loop_backedges_none() { - let mir_body = goto_switchint(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - if false { - eprintln!( - "basic_coverage_blocks = {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); - eprintln!("successors = {:?}", basic_coverage_blocks.successors); - } - let backedges = graph::find_loop_backedges(&basic_coverage_blocks); - assert_eq!( - backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), - 0, - "backedges: {:?}", - backedges - ); -} - -#[test] -fn test_find_loop_backedges_one() { - let mir_body = switchint_then_loop_else_return(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - let backedges = graph::find_loop_backedges(&basic_coverage_blocks); - assert_eq!( - backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), - 1, - "backedges: {:?}", - backedges - ); - - assert_eq!(backedges[bcb(1)], &[bcb(3)]); -} - -#[test] -fn test_find_loop_backedges_two() { - let mir_body = switchint_loop_then_inner_loop_else_break(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - let backedges = graph::find_loop_backedges(&basic_coverage_blocks); - assert_eq!( - backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), - 2, - "backedges: {:?}", - backedges - ); - - assert_eq!(backedges[bcb(1)], &[bcb(5)]); - assert_eq!(backedges[bcb(4)], &[bcb(6)]); -} - -#[test] -fn test_traverse_coverage_with_loops() { - let mir_body = switchint_loop_then_inner_loop_else_break(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - let mut traversed_in_order = Vec::new(); - let mut traversal = graph::TraverseCoverageGraphWithLoops::new(&basic_coverage_blocks); - while let Some(bcb) = traversal.next() { - traversed_in_order.push(bcb); - } - - // bcb0 is visited first. Then bcb1 starts the first loop, and all remaining nodes, *except* - // bcb6 are inside the first loop. - assert_eq!( - *traversed_in_order.last().expect("should have elements"), - bcb(6), - "bcb6 should not be visited until all nodes inside the first loop have been visited" - ); -} - -#[test] -fn test_make_bcb_counters() { - rustc_span::create_default_session_globals_then(|| { - let mir_body = goto_switchint(); - let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); - // Historically this test would use `spans` internals to set up fake - // coverage spans for BCBs 1 and 2. Now we skip that step and just tell - // BCB counter construction that those BCBs have spans. - let bcb_has_coverage_spans = |bcb: BasicCoverageBlock| (1..=2).contains(&bcb.as_usize()); - let coverage_counters = counters::CoverageCounters::make_bcb_counters( - &basic_coverage_blocks, - bcb_has_coverage_spans, - ); - assert_eq!(coverage_counters.num_expressions(), 0); - - assert_eq!( - 0, // bcb1 has a `Counter` with id = 0 - match coverage_counters.bcb_counter(bcb(1)).expect("should have a counter") { - counters::BcbCounter::Counter { id, .. } => id, - _ => panic!("expected a Counter"), - } - .as_u32() - ); - - assert_eq!( - 1, // bcb2 has a `Counter` with id = 1 - match coverage_counters.bcb_counter(bcb(2)).expect("should have a counter") { - counters::BcbCounter::Counter { id, .. } => id, - _ => panic!("expected a Counter"), - } - .as_u32() - ); - }); -} From 917b455a872050b90474043f31ef657b338526ba Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 14:04:36 +1000 Subject: [PATCH 15/19] coverage: Reduce/simplify visibility in `coverage::graph` Using `pub(super)` makes it harder to move code between modules, and doesn't provide much privacy benefit over `pub(crate)`. --- .../rustc_mir_transform/src/coverage/graph.rs | 52 ++++++++++--------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index fd74a2a97e2c2..544195a80fca7 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -14,16 +14,16 @@ use std::ops::{Index, IndexMut}; /// A coverage-specific simplification of the MIR control flow graph (CFG). The `CoverageGraph`s /// nodes are `BasicCoverageBlock`s, which encompass one or more MIR `BasicBlock`s. #[derive(Debug)] -pub(super) struct CoverageGraph { +pub(crate) struct CoverageGraph { bcbs: IndexVec, bb_to_bcb: IndexVec>, - pub successors: IndexVec>, - pub predecessors: IndexVec>, + pub(crate) successors: IndexVec>, + pub(crate) predecessors: IndexVec>, dominators: Option>, } impl CoverageGraph { - pub fn from_mir(mir_body: &mir::Body<'_>) -> Self { + pub(crate) fn from_mir(mir_body: &mir::Body<'_>) -> Self { let (bcbs, bb_to_bcb) = Self::compute_basic_coverage_blocks(mir_body); // Pre-transform MIR `BasicBlock` successors and predecessors into the BasicCoverageBlock @@ -135,24 +135,28 @@ impl CoverageGraph { } #[inline(always)] - pub fn iter_enumerated( + pub(crate) fn iter_enumerated( &self, ) -> impl Iterator { self.bcbs.iter_enumerated() } #[inline(always)] - pub fn bcb_from_bb(&self, bb: BasicBlock) -> Option { + pub(crate) fn bcb_from_bb(&self, bb: BasicBlock) -> Option { if bb.index() < self.bb_to_bcb.len() { self.bb_to_bcb[bb] } else { None } } #[inline(always)] - pub fn dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool { + pub(crate) fn dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool { self.dominators.as_ref().unwrap().dominates(dom, node) } #[inline(always)] - pub fn cmp_in_dominator_order(&self, a: BasicCoverageBlock, b: BasicCoverageBlock) -> Ordering { + pub(crate) fn cmp_in_dominator_order( + &self, + a: BasicCoverageBlock, + b: BasicCoverageBlock, + ) -> Ordering { self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b) } @@ -166,7 +170,7 @@ impl CoverageGraph { /// /// FIXME: That assumption might not be true for [`TerminatorKind::Yield`]? #[inline(always)] - pub(super) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool { + pub(crate) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool { // Even though bcb0 conceptually has an extra virtual in-edge due to // being the entry point, we've already asserted that it has no _other_ // in-edges, so there's no possibility of it having _multiple_ in-edges. @@ -227,7 +231,7 @@ rustc_index::newtype_index! { /// A node in the control-flow graph of CoverageGraph. #[orderable] #[debug_format = "bcb{}"] - pub(super) struct BasicCoverageBlock { + pub(crate) struct BasicCoverageBlock { const START_BCB = 0; } } @@ -259,23 +263,23 @@ rustc_index::newtype_index! { /// queries (`dominates()`, `predecessors`, `successors`, etc.) have branch (control flow) /// significance. #[derive(Debug, Clone)] -pub(super) struct BasicCoverageBlockData { - pub basic_blocks: Vec, +pub(crate) struct BasicCoverageBlockData { + pub(crate) basic_blocks: Vec, } impl BasicCoverageBlockData { - pub fn from(basic_blocks: Vec) -> Self { + fn from(basic_blocks: Vec) -> Self { assert!(basic_blocks.len() > 0); Self { basic_blocks } } #[inline(always)] - pub fn leader_bb(&self) -> BasicBlock { + pub(crate) fn leader_bb(&self) -> BasicBlock { self.basic_blocks[0] } #[inline(always)] - pub fn last_bb(&self) -> BasicBlock { + pub(crate) fn last_bb(&self) -> BasicBlock { *self.basic_blocks.last().unwrap() } } @@ -364,7 +368,7 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera /// CoverageGraph outside all loops. This supports traversing the BCB CFG in a way that /// ensures a loop is completely traversed before processing Blocks after the end of the loop. #[derive(Debug)] -pub(super) struct TraversalContext { +struct TraversalContext { /// BCB with one or more incoming loop backedges, indicating which loop /// this context is for. /// @@ -375,7 +379,7 @@ pub(super) struct TraversalContext { worklist: VecDeque, } -pub(super) struct TraverseCoverageGraphWithLoops<'a> { +pub(crate) struct TraverseCoverageGraphWithLoops<'a> { basic_coverage_blocks: &'a CoverageGraph, backedges: IndexVec>, @@ -384,7 +388,7 @@ pub(super) struct TraverseCoverageGraphWithLoops<'a> { } impl<'a> TraverseCoverageGraphWithLoops<'a> { - pub(super) fn new(basic_coverage_blocks: &'a CoverageGraph) -> Self { + pub(crate) fn new(basic_coverage_blocks: &'a CoverageGraph) -> Self { let backedges = find_loop_backedges(basic_coverage_blocks); let worklist = VecDeque::from([basic_coverage_blocks.start_node()]); @@ -400,7 +404,7 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { /// For each loop on the loop context stack (top-down), yields a list of BCBs /// within that loop that have an outgoing edge back to the loop header. - pub(super) fn reloop_bcbs_per_loop(&self) -> impl Iterator { + pub(crate) fn reloop_bcbs_per_loop(&self) -> impl Iterator { self.context_stack .iter() .rev() @@ -408,7 +412,7 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { .map(|header_bcb| self.backedges[header_bcb].as_slice()) } - pub(super) fn next(&mut self) -> Option { + pub(crate) fn next(&mut self) -> Option { debug!( "TraverseCoverageGraphWithLoops::next - context_stack: {:?}", self.context_stack.iter().rev().collect::>() @@ -440,7 +444,7 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { None } - pub fn add_successors_to_worklists(&mut self, bcb: BasicCoverageBlock) { + fn add_successors_to_worklists(&mut self, bcb: BasicCoverageBlock) { let successors = &self.basic_coverage_blocks.successors[bcb]; debug!("{:?} has {} successors:", bcb, successors.len()); @@ -494,11 +498,11 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { } } - pub fn is_complete(&self) -> bool { + pub(crate) fn is_complete(&self) -> bool { self.visited.count() == self.visited.domain_size() } - pub fn unvisited(&self) -> Vec { + pub(crate) fn unvisited(&self) -> Vec { let mut unvisited_set: BitSet = BitSet::new_filled(self.visited.domain_size()); unvisited_set.subtract(&self.visited); @@ -506,7 +510,7 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { } } -pub(super) fn find_loop_backedges( +fn find_loop_backedges( basic_coverage_blocks: &CoverageGraph, ) -> IndexVec> { let num_bcbs = basic_coverage_blocks.num_nodes(); From dca6b5eeade5f369c4cb021f1f405b453db35870 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 13:01:41 +1000 Subject: [PATCH 16/19] coverage: Flatten some graph code with let-else --- .../rustc_mir_transform/src/coverage/graph.rs | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 544195a80fca7..607cb14f4c3a0 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -419,26 +419,25 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> { ); while let Some(context) = self.context_stack.last_mut() { - if let Some(bcb) = context.worklist.pop_front() { - if !self.visited.insert(bcb) { - debug!("Already visited: {bcb:?}"); - continue; - } - debug!("Visiting {bcb:?}"); - - if self.backedges[bcb].len() > 0 { - debug!("{bcb:?} is a loop header! Start a new TraversalContext..."); - self.context_stack.push(TraversalContext { - loop_header: Some(bcb), - worklist: VecDeque::new(), - }); - } - self.add_successors_to_worklists(bcb); - return Some(bcb); - } else { - // Strip contexts with empty worklists from the top of the stack + let Some(bcb) = context.worklist.pop_front() else { + // This stack level is exhausted; pop it and try the next one. self.context_stack.pop(); + continue; + }; + + if !self.visited.insert(bcb) { + debug!("Already visited: {bcb:?}"); + continue; + } + debug!("Visiting {bcb:?}"); + + if self.backedges[bcb].len() > 0 { + debug!("{bcb:?} is a loop header! Start a new TraversalContext..."); + self.context_stack + .push(TraversalContext { loop_header: Some(bcb), worklist: VecDeque::new() }); } + self.add_successors_to_worklists(bcb); + return Some(bcb); } None From e5b43c33d8c729022218fe4e8515aea5799f3660 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 16 Jun 2024 15:51:00 +1000 Subject: [PATCH 17/19] coverage: Prefer `Iterator::copied` --- compiler/rustc_mir_transform/src/coverage/graph.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index 607cb14f4c3a0..360dccb240dc6 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -216,7 +216,7 @@ impl graph::StartNode for CoverageGraph { impl graph::Successors for CoverageGraph { #[inline] fn successors(&self, node: Self::Node) -> impl Iterator { - self.successors[node].iter().cloned() + self.successors[node].iter().copied() } } From 51d95464169751604a3cf70b1c8c524450258427 Mon Sep 17 00:00:00 2001 From: Slanterns Date: Sun, 16 Jun 2024 15:56:13 +0800 Subject: [PATCH 18/19] Apply suggestion. Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com> --- tests/codegen/error-provide.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/codegen/error-provide.rs b/tests/codegen/error-provide.rs index e80905eee1101..68dd383e5cce0 100644 --- a/tests/codegen/error-provide.rs +++ b/tests/codegen/error-provide.rs @@ -36,7 +36,9 @@ impl std::error::Error for MyError { // LLVM should be able to optimize multiple .provide_* calls into a switch table // and eliminate redundant ones, rather than compare one-by-one. - // CHECK: switch i64 %{{.*}}, label %{{.*}} [ + // CHECK-NEXT: start: + // CHECK-NEXT: %[[SCRUTINEE:[^ ]+]] = load i64, ptr + // CHECK-NEXT: switch i64 %[[SCRUTINEE]], label %{{.*}} [ // CHECK-COUNT-3: i64 {{.*}}, label %{{.*}} // CHECK-NEXT: ] request From fe9154c64ee0e226a7f0009796b98f9fbec3862d Mon Sep 17 00:00:00 2001 From: Rayyan Khan <163682431+x4exr@users.noreply.github.com> Date: Tue, 11 Jun 2024 19:35:29 -0400 Subject: [PATCH 19/19] doc: Added commas where needed --- library/core/src/ptr/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 6661f6ee78b42..a8a47b69632f7 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -237,7 +237,7 @@ //! pointer. For code which *does* cast a usize to a pointer, the scope of the change depends //! on exactly what you're doing. //! -//! In general you just need to make sure that if you want to convert a usize address to a +//! In general, you just need to make sure that if you want to convert a usize address to a //! pointer and then use that pointer to read/write memory, you need to keep around a pointer //! that has sufficient provenance to perform that read/write itself. In this way all of your //! casts from an address to a pointer are essentially just applying offsets/indexing. @@ -309,7 +309,7 @@ //! i.e. the usual "ZSTs are fake, do what you want" rules apply *but* this only applies //! for actual forgery (integers cast to pointers). If you borrow some struct's field //! that *happens* to be zero-sized, the resulting pointer will have provenance tied to -//! that allocation and it will still get invalidated if the allocation gets deallocated. +//! that allocation, and it will still get invalidated if the allocation gets deallocated. //! In the future we may introduce an API to make such a forged allocation explicit. //! //! * [`wrapping_offset`][] a pointer outside its provenance. This includes pointers @@ -698,7 +698,7 @@ pub const fn dangling_mut() -> *mut T { /// /// If there is no 'exposed' provenance that justifies the way this pointer will be used, /// the program has undefined behavior. In particular, the aliasing rules still apply: pointers -/// and references that have been invalidated due to aliasing accesses cannot be used any more, +/// and references that have been invalidated due to aliasing accesses cannot be used anymore, /// even if they have been exposed! /// /// Note that there is no algorithm that decides which provenance will be used. You can think of this @@ -1097,7 +1097,7 @@ const unsafe fn swap_nonoverlapping_simple_untyped(x: *mut T, y: *mut T, coun // If we end up here, it's because we're using a simple type -- like // a small power-of-two-sized thing -- or a special type with particularly // large alignment, particularly SIMD types. - // Thus we're fine just reading-and-writing it, as either it's small + // Thus, we're fine just reading-and-writing it, as either it's small // and that works well anyway or it's special and the type's author // presumably wanted things to be done in the larger chunk. @@ -1290,7 +1290,7 @@ pub const unsafe fn read(src: *const T) -> T { // provides enough information to know that this is a typed operation. // However, as of March 2023 the compiler was not capable of taking advantage - // of that information. Thus the implementation here switched to an intrinsic, + // of that information. Thus, the implementation here switched to an intrinsic, // which lowers to `_0 = *src` in MIR, to address a few issues: // // - Using `MaybeUninit::assume_init` after a `copy_nonoverlapping` was not @@ -1570,7 +1570,7 @@ pub const unsafe fn write(dst: *mut T, src: T) { /// As a result, using `&packed.unaligned as *const FieldType` causes immediate /// *undefined behavior* in your program. /// -/// Instead you must use the [`ptr::addr_of_mut!`](addr_of_mut) +/// Instead, you must use the [`ptr::addr_of_mut!`](addr_of_mut) /// macro to create the pointer. You may use that returned pointer together with /// this function. ///