Skip to content

Commit

Permalink
Rollup merge of #125258 - compiler-errors:static-if-no-lt, r=nnethercote
Browse files Browse the repository at this point in the history
Resolve elided lifetimes in assoc const to static if no other lifetimes are in scope

Implements the change to elided lifetime resolution in *associated consts* subject to FCP here: #125190 (comment)

Specifically, walk the enclosing lifetime ribs in an associated const, and if we find no other lifetimes, then resolve to `'static`.

Also make it work for traits, but don't lint -- just give a hard error in that case.
  • Loading branch information
jieyouxu authored Jun 17, 2024
2 parents 6b65c30 + 5f3357c commit 23b936f
Show file tree
Hide file tree
Showing 21 changed files with 386 additions and 210 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
15 changes: 12 additions & 3 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2873,6 +2873,8 @@ pub struct AssociatedConstElidedLifetime {

pub code: &'static str,
pub elided: bool,
#[note]
pub lifetimes_in_scope: MultiSpan,
}

#[derive(LintDiagnostic)]
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
/// }
/// ```
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ pub enum BuiltinLintDiag {
AssociatedConstElidedLifetime {
elided: bool,
span: Span,
lifetimes_in_scope: MultiSpan,
},
RedundantImportVisibility {
span: Span,
Expand Down
142 changes: 89 additions & 53 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,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.
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 (if `emit_lint` is enabled).
StaticIfNoLifetimeInScope { lint_id: NodeId, emit_lint: bool },

/// Signal we cannot find which should be the anonymous lifetime.
ElisionFailure,
Expand Down Expand Up @@ -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(_)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 { .. } => {}
}
}

Expand Down Expand Up @@ -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 { 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));
// 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 if emit_lint {
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 {
Expand Down Expand Up @@ -1904,7 +1925,7 @@ 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::AnonymousWarn(_) => {
| LifetimeRibKind::StaticIfNoLifetimeInScope { .. } => {
let sess = self.r.tcx.sess;
let subdiag = rustc_errors::elided_lifetime_in_path_suggestion(
sess.source_map(),
Expand Down Expand Up @@ -2838,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);
}
},
)
},
);
}
Expand Down Expand Up @@ -3030,30 +3059,37 @@ 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 {
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
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
8 changes: 3 additions & 5 deletions tests/ui/associated-consts/double-elided.rs
Original file line number Diff line number Diff line change
@@ -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() {}
47 changes: 0 additions & 47 deletions tests/ui/associated-consts/double-elided.stderr

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Original file line number Diff line number Diff line change
@@ -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 <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 1 previous error; 1 warning emitted
error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0121`.
12 changes: 12 additions & 0 deletions tests/ui/consts/assoc-const-elided-lifetime.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/rust-lang/rust/issues/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
|
Expand All @@ -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 <https://github.com/rust-lang/rust/issues/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 () = &();
Expand Down
Loading

0 comments on commit 23b936f

Please sign in to comment.