From d3c60672755fd7c5bfaabaac3af88961712a2b9a Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 1 Jan 2025 16:55:10 +0000 Subject: [PATCH] Fix ICE when opaque captures a duplicated/invalid lifetime --- compiler/rustc_ast_lowering/src/lib.rs | 4 ++-- compiler/rustc_hir/src/hir.rs | 16 +++++++++------- compiler/rustc_hir/src/intravisit.rs | 4 ++-- .../rustc_hir_analysis/src/check/wfcheck.rs | 5 ++++- tests/crashes/132766.rs | 9 --------- .../impl-trait/captured-invalid-lifetime.rs | 19 +++++++++++++++++++ .../captured-invalid-lifetime.stderr | 11 +++++++++++ tests/ui/stats/input-stats.stderr | 12 ++++++------ 8 files changed, 53 insertions(+), 27 deletions(-) delete mode 100644 tests/crashes/132766.rs create mode 100644 tests/ui/impl-trait/captured-invalid-lifetime.rs create mode 100644 tests/ui/impl-trait/captured-invalid-lifetime.stderr diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 8438a42122688..46e91636cfbbb 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1845,11 +1845,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { GenericParamKind::Lifetime => { // AST resolution emitted an error on those parameters, so we lower them using // `ParamName::Error`. + let ident = self.lower_ident(param.ident); let param_name = if let Some(LifetimeRes::Error) = self.resolver.get_lifetime_res(param.id) { - ParamName::Error + ParamName::Error(ident) } else { - let ident = self.lower_ident(param.ident); ParamName::Plain(ident) }; let kind = diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 8cea269f29823..9218d515bfc57 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -52,6 +52,13 @@ pub enum ParamName { /// Some user-given name like `T` or `'x`. Plain(Ident), + /// Indicates an illegal name was given and an error has been + /// reported (so we should squelch other derived errors). + /// + /// Occurs when, e.g., `'_` is used in the wrong place, or a + /// lifetime name is duplicated. + Error(Ident), + /// Synthetic name generated when user elided a lifetime in an impl header. /// /// E.g., the lifetimes in cases like these: @@ -67,18 +74,13 @@ pub enum ParamName { /// where `'f` is something like `Fresh(0)`. The indices are /// unique per impl, but not necessarily continuous. Fresh, - - /// Indicates an illegal name was given and an error has been - /// reported (so we should squelch other derived errors). Occurs - /// when, e.g., `'_` is used in the wrong place. - Error, } impl ParamName { pub fn ident(&self) -> Ident { match *self { - ParamName::Plain(ident) => ident, - ParamName::Fresh | ParamName::Error => Ident::with_dummy_span(kw::UnderscoreLifetime), + ParamName::Plain(ident) | ParamName::Error(ident) => ident, + ParamName::Fresh => Ident::with_dummy_span(kw::UnderscoreLifetime), } } } diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 387a195cb2981..a73cf367c0e0c 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -928,8 +928,8 @@ pub fn walk_generic_param<'v, V: Visitor<'v>>( ) -> V::Result { try_visit!(visitor.visit_id(param.hir_id)); match param.name { - ParamName::Plain(ident) => try_visit!(visitor.visit_ident(ident)), - ParamName::Error | ParamName::Fresh => {} + ParamName::Plain(ident) | ParamName::Error(ident) => try_visit!(visitor.visit_ident(ident)), + ParamName::Fresh => {} } match param.kind { GenericParamKind::Lifetime { .. } => {} diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 3cddc9642bae2..efffb24c81d0d 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -2007,7 +2007,10 @@ fn check_variances_for_type_defn<'tcx>( } match hir_param.name { - hir::ParamName::Error => {} + hir::ParamName::Error(_) => { + // Don't report a bivariance error for a lifetime that isn't + // even valid to name. + } _ => { let has_explicit_bounds = explicitly_bounded_params.contains(¶meter); report_bivariance(tcx, hir_param, has_explicit_bounds, item); diff --git a/tests/crashes/132766.rs b/tests/crashes/132766.rs deleted file mode 100644 index 5f5117d77a521..0000000000000 --- a/tests/crashes/132766.rs +++ /dev/null @@ -1,9 +0,0 @@ -//@ known-bug: #132766 - -trait Trait {} -impl<'a> Trait for () { - fn pass2<'a>() -> impl Trait2 {} -} - -trait Trait2 {} -impl Trait2 for () {} diff --git a/tests/ui/impl-trait/captured-invalid-lifetime.rs b/tests/ui/impl-trait/captured-invalid-lifetime.rs new file mode 100644 index 0000000000000..57cd2e85dd0ee --- /dev/null +++ b/tests/ui/impl-trait/captured-invalid-lifetime.rs @@ -0,0 +1,19 @@ +// This uses edition 2024 for new lifetime capture rules. +//@ edition: 2024 + +// The problem here is that the presence of the opaque which captures all lifetimes in scope +// means that the duplicated `'a` (which I'll call the dupe) is considered to be *early-bound* +// since it shows up in the output but not the inputs. This is paired with the fact that we +// were previously setting the name of the dupe to `'_` in the generic param definition, which +// means that the identity args for the function were `['a#0, '_#1]` even though the lifetime +// for the dupe should've been `'a#1`. This difference in symbol meant that NLL couldn't +// actually match the lifetime against the identity lifetimes, leading to an ICE. + +struct Foo<'a>(&'a ()); + +impl<'a> Foo<'a> { + fn pass<'a>() -> impl Sized {} + //~^ ERROR lifetime name `'a` shadows a lifetime name that is already in scope +} + +fn main() {} diff --git a/tests/ui/impl-trait/captured-invalid-lifetime.stderr b/tests/ui/impl-trait/captured-invalid-lifetime.stderr new file mode 100644 index 0000000000000..c1315e342416e --- /dev/null +++ b/tests/ui/impl-trait/captured-invalid-lifetime.stderr @@ -0,0 +1,11 @@ +error[E0496]: lifetime name `'a` shadows a lifetime name that is already in scope + --> $DIR/captured-invalid-lifetime.rs:15:13 + | +LL | impl<'a> Foo<'a> { + | -- first declared here +LL | fn pass<'a>() -> impl Sized {} + | ^^ lifetime `'a` already in scope + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0496`. diff --git a/tests/ui/stats/input-stats.stderr b/tests/ui/stats/input-stats.stderr index 7183073d66588..9b1568fa11665 100644 --- a/tests/ui/stats/input-stats.stderr +++ b/tests/ui/stats/input-stats.stderr @@ -146,13 +146,13 @@ hir-stats Variant 144 ( 1.6%) 2 72 hir-stats GenericBound 256 ( 2.9%) 4 64 hir-stats - Trait 256 ( 2.9%) 4 hir-stats Block 288 ( 3.2%) 6 48 -hir-stats GenericParam 360 ( 4.0%) 5 72 hir-stats Pat 360 ( 4.0%) 5 72 hir-stats - Struct 72 ( 0.8%) 1 hir-stats - Wild 72 ( 0.8%) 1 hir-stats - Binding 216 ( 2.4%) 3 -hir-stats Generics 560 ( 6.3%) 10 56 -hir-stats Ty 720 ( 8.1%) 15 48 +hir-stats GenericParam 400 ( 4.5%) 5 80 +hir-stats Generics 560 ( 6.2%) 10 56 +hir-stats Ty 720 ( 8.0%) 15 48 hir-stats - Ptr 48 ( 0.5%) 1 hir-stats - Ref 48 ( 0.5%) 1 hir-stats - Path 624 ( 7.0%) 13 @@ -171,8 +171,8 @@ hir-stats - Impl 88 ( 1.0%) 1 hir-stats - Trait 88 ( 1.0%) 1 hir-stats - Fn 176 ( 2.0%) 2 hir-stats - Use 352 ( 3.9%) 4 -hir-stats Path 1_240 (13.9%) 31 40 -hir-stats PathSegment 1_920 (21.5%) 40 48 +hir-stats Path 1_240 (13.8%) 31 40 +hir-stats PathSegment 1_920 (21.4%) 40 48 hir-stats ---------------------------------------------------------------- -hir-stats Total 8_936 180 +hir-stats Total 8_976 180 hir-stats