Skip to content

Commit

Permalink
Rollup merge of rust-lang#115011 - compiler-errors:warn-on-elided-ass…
Browse files Browse the repository at this point in the history
…oc-ct-lt, r=cjgillot

Warn on elided lifetimes in associated constants (`ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT`)

Elided lifetimes in associated constants (in impls) erroneously resolve to fresh lifetime parameters on the impl since rust-lang#97313. This is not correct behavior (see rust-lang#38831).

I originally opened rust-lang#114716 to fix this, but given the time that has passed, the crater results seem pretty bad: rust-lang#114716 (comment)

This PR alternatively implements a lint against this behavior, and I'm hoping to bump this to deny in a few versions.
  • Loading branch information
matthiaskrgr authored Aug 21, 2023
2 parents fe5f591 + fad7d22 commit 3e4d2ad
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 25 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,14 @@ pub trait LintContext: Sized {
Applicability::MachineApplicable
);
}
BuiltinLintDiagnostics::AssociatedConstElidedLifetime { elided, span } => {
db.span_suggestion_verbose(
if elided { span.shrink_to_hi() } else { span },
"use the `'static` lifetime",
if elided { "'static " } else { "'static" },
Applicability::MachineApplicable
);
}
}
// Rewrap `db`, and pass control to the user.
decorate(db)
Expand Down
42 changes: 42 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3376,6 +3376,7 @@ declare_lint_pass! {
DEPRECATED_IN_FUTURE,
DEPRECATED_WHERE_CLAUSE_LOCATION,
DUPLICATE_MACRO_ATTRIBUTES,
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
ELIDED_LIFETIMES_IN_PATHS,
EXPORTED_PRIVATE_DEPENDENCIES,
FFI_UNWIND_CALLS,
Expand Down Expand Up @@ -4527,3 +4528,44 @@ declare_lint! {
reference: "issue #114095 <https://github.com/rust-lang/rust/issues/114095>",
};
}

declare_lint! {
/// The `elided_lifetimes_in_associated_constant` lint detects elided lifetimes
/// that were erroneously allowed in associated constants.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(elided_lifetimes_in_associated_constant)]
///
/// struct Foo;
///
/// impl Foo {
/// const STR: &str = "hello, world";
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Previous version of Rust
///
/// Implicit static-in-const behavior was decided [against] for associated
/// constants because of ambiguity. This, however, regressed and the compiler
/// erroneously treats elided lifetimes in associated constants as lifetime
/// parameters on the impl.
///
/// This is a [future-incompatible] lint to transition this to a
/// hard error in the future.
///
/// [against]: https://github.com/rust-lang/rust/issues/38831
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
Warn,
"elided lifetimes cannot be used in associated constants in impls",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseError,
reference: "issue #115010 <https://github.com/rust-lang/rust/issues/115010>",
};
}
4 changes: 4 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,10 @@ pub enum BuiltinLintDiagnostics {
/// The span of the unnecessarily-qualified path to remove.
removal_span: Span,
},
AssociatedConstElidedLifetime {
elided: bool,
span: Span,
},
}

/// Lints that are buffered up early on in the `Session` before the
Expand Down
79 changes: 55 additions & 24 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,10 @@ enum LifetimeRibKind {
/// error on default object bounds (e.g., `Box<dyn Foo>`).
AnonymousReportError,

/// Resolves elided lifetimes to `'static`, but gives a warning that this behavior
/// is a bug and will be reverted soon.
AnonymousWarnToStatic(NodeId),

/// Signal we cannot find which should be the anonymous lifetime.
ElisionFailure,

Expand Down Expand Up @@ -1148,6 +1152,7 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
}
LifetimeRibKind::AnonymousCreateParameter { .. }
| LifetimeRibKind::AnonymousReportError
| LifetimeRibKind::AnonymousWarnToStatic(_)
| LifetimeRibKind::Elided(_)
| LifetimeRibKind::ElisionFailure
| LifetimeRibKind::ConcreteAnonConst(_)
Expand Down Expand Up @@ -1515,6 +1520,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
// lifetime would be illegal.
LifetimeRibKind::Item
| LifetimeRibKind::AnonymousReportError
| LifetimeRibKind::AnonymousWarnToStatic(_)
| LifetimeRibKind::ElisionFailure => Some(LifetimeUseSet::Many),
// An anonymous lifetime is legal here, and bound to the right
// place, go ahead.
Expand Down Expand Up @@ -1576,7 +1582,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
| LifetimeRibKind::Elided(_)
| LifetimeRibKind::Generics { .. }
| LifetimeRibKind::ElisionFailure
| LifetimeRibKind::AnonymousReportError => {}
| LifetimeRibKind::AnonymousReportError
| LifetimeRibKind::AnonymousWarnToStatic(_) => {}
}
}

Expand Down Expand Up @@ -1616,6 +1623,25 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
self.record_lifetime_res(lifetime.id, res, elision_candidate);
return;
}
LifetimeRibKind::AnonymousWarnToStatic(node_id) => {
self.record_lifetime_res(lifetime.id, LifetimeRes::Static, elision_candidate);
let msg = if elided {
"`&` without an explicit lifetime name cannot be used here"
} else {
"`'_` cannot be used here"
};
self.r.lint_buffer.buffer_lint_with_diagnostic(
lint::builtin::ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
node_id,
lifetime.ident.span,
msg,
lint::BuiltinLintDiagnostics::AssociatedConstElidedLifetime {
elided,
span: lifetime.ident.span,
},
);
return;
}
LifetimeRibKind::AnonymousReportError => {
let (msg, note) = if elided {
(
Expand Down Expand Up @@ -1811,7 +1837,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
//
// impl Foo for std::cell::Ref<u32> // note lack of '_
// async fn foo(_: std::cell::Ref<u32>) { ... }
LifetimeRibKind::AnonymousCreateParameter { report_in_path: true, .. } => {
LifetimeRibKind::AnonymousCreateParameter { report_in_path: true, .. }
| LifetimeRibKind::AnonymousWarnToStatic(_) => {
let sess = self.r.tcx.sess;
let mut err = rustc_errors::struct_span_err!(
sess,
Expand Down Expand Up @@ -2898,7 +2925,6 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
match &item.kind {
AssocItemKind::Const(box ast::ConstItem { generics, ty, expr, .. }) => {
debug!("resolve_implementation AssocItemKind::Const");

self.with_generic_param_rib(
&generics.params,
RibKind::AssocItem,
Expand All @@ -2908,28 +2934,33 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
kind: LifetimeBinderKind::ConstItem,
},
|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::AnonymousWarnToStatic(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);
}
},
);
},
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ 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() {}
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
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 <https://github.com/rust-lang/rust/issues/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 previous error
error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0121`.
19 changes: 19 additions & 0 deletions tests/ui/consts/assoc-const-elided-lifetime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![deny(elided_lifetimes_in_associated_constant)]

use std::marker::PhantomData;

struct Foo<'a> {
x: PhantomData<&'a ()>,
}

impl<'a> Foo<'a> {
const FOO: Foo<'_> = Foo { x: PhantomData::<&()> };
//~^ ERROR `'_` 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!

const BAR: &() = &();
//~^ ERROR `&` 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() {}
33 changes: 33 additions & 0 deletions tests/ui/consts/assoc-const-elided-lifetime.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error: `'_` cannot be used here
--> $DIR/assoc-const-elided-lifetime.rs:10:20
|
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 <https://github.com/rust-lang/rust/issues/115010>
note: the lint level is defined here
--> $DIR/assoc-const-elided-lifetime.rs:1:9
|
LL | #![deny(elided_lifetimes_in_associated_constant)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: use the `'static` lifetime
|
LL | const FOO: Foo<'static> = Foo { x: PhantomData::<&()> };
| ~~~~~~~

error: `&` without an explicit lifetime name cannot be used here
--> $DIR/assoc-const-elided-lifetime.rs:14:16
|
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 <https://github.com/rust-lang/rust/issues/115010>
help: use the `'static` lifetime
|
LL | const BAR: &'static () = &();
| +++++++

error: aborting due to 2 previous errors

0 comments on commit 3e4d2ad

Please sign in to comment.