From 7a812c131149e2e730c3605c30d6fb1ae376d4bd Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 14 Nov 2023 19:36:24 -0800 Subject: [PATCH 01/18] Clarify how to choose a FutureIncompatibilityReason variant. There has been some confusion about how to choose these variants, or what the procedure is for handling future-incompatible errors. Hopefully this helps provide some more information on how these work. --- compiler/rustc_lint_defs/src/lib.rs | 67 +++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 878c1a65dbf6e..185634848b49c 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -345,12 +345,34 @@ pub struct FutureIncompatibleInfo { } /// The reason for future incompatibility +/// +/// Future-incompatible lints come in roughly two categories: +/// +/// 1. There was a mistake in the compiler (such as a soundness issue), and +/// we're trying to fix it, but it may be a breaking change. +/// 2. A change across an Edition boundary, typically used for the +/// introduction of new language features that can't otherwise be +/// introduced in a backwards-compatible way. +/// +/// See and +/// +/// for more information. #[derive(Copy, Clone, Debug)] pub enum FutureIncompatibilityReason { /// This will be an error in a future release for all editions /// /// This will *not* show up in cargo's future breakage report. /// The warning will hence only be seen in local crates, not in dependencies. + /// + /// Choose this variant when you are first introducing a "future + /// incompatible" warning that is intended to eventually be fixed in the + /// future. This allows crate developers an opportunity to fix the warning + /// before blasting all dependents with a warning they can't fix + /// (dependents have to wait for a new release of the affected crate to be + /// published). + /// + /// After a lint has been in this state for a while, consider graduating + /// it to [`FutureIncompatibilityReason::FutureReleaseErrorReportInDeps`]. FutureReleaseErrorDontReportInDeps, /// This will be an error in a future release, and /// Cargo should create a report even for dependencies @@ -358,17 +380,62 @@ pub enum FutureIncompatibilityReason { /// This is the *only* reason that will make future incompatibility warnings show up in cargo's /// reports. All other future incompatibility warnings are not visible when they occur in a /// dependency. + /// + /// Choose this variant after the lint has been sitting in the + /// [`FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps`] + /// state for a while, and you feel like it is ready to graduate to + /// warning everyone. It is a good signal that it is ready if you can + /// determine that all or most affected crates on crates.io have been + /// updated. + /// + /// After some period of time, lints with this variant can be turned into + /// hard errors (and the lint removed). Preferably when there is some + /// confidence that the number of impacted projects is very small (few + /// should have a broken dependency in their dependency tree). FutureReleaseErrorReportInDeps, /// Code that changes meaning in some way in a /// future release. + /// + /// Choose this variant when the semantics of existing code is changing, + /// (as opposed to + /// [`FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps`], + /// which is for when code is going to be rejected in the future). FutureReleaseSemanticsChange, /// Previously accepted code that will become an /// error in the provided edition + /// + /// Choose this variant for code that you want to start rejecting across + /// an edition boundary. This will automatically include the lint in the + /// `rust-20xx-compatibility` lint group, which is used by `cargo fix + /// --edition` to do migrations. The lint *should* be auto-fixable with + /// [`Applicability::MachineApplicable`]. + /// + /// The lint can either be `Allow` or `Warn` by default. If it is `Allow`, + /// users usually won't see this warning unless they are doing an edition + /// migration manually or there is a problem during the migration (cargo's + /// automatic migrations will force the level to `Warn`). If it is `Warn` + /// by default, users on all editions will see this warning (only do this + /// if you think it is important for everyone to be aware of the change, + /// and to encourage people to update their code on all editions). + /// + /// See also [`FutureIncompatibilityReason::EditionSemanticsChange`] if + /// you have code that is changing semantics across the edition (as + /// opposed to being rejected). EditionError(Edition), /// Code that changes meaning in some way in /// the provided edition + /// + /// This is the same as [`FutureIncompatibilityReason::EditionError`], + /// except for situations where the semantics change across an edition. It + /// slightly changes the text of the diagnostic, but is otherwise the + /// same. EditionSemanticsChange(Edition), /// A custom reason. + /// + /// Choose this variant if the built-in text of the diagnostic of the + /// other variants doesn't match your situation. This is behaviorally + /// equivalent to + /// [`FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps`]. Custom(&'static str), } From b7a23bc08b2cc9b1ddfac18ea2019d5150d93e0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 14 Nov 2023 20:36:49 +0000 Subject: [PATCH 02/18] On borrow return type, suggest borrowing from arg or owned return type When we encounter a function with a return type that has an anonymous lifetime with no argument to borrow from, besides suggesting the `'static` lifetime we now also suggest changing the arguments to be borrows or changing the return type to be an owned type. ``` error[E0106]: missing lifetime specifier --> $DIR/variadic-ffi-6.rs:7:6 | LL | ) -> &usize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | ) -> &'static usize { | +++++++ help: instead, you are more likely to want to change one of the arguments to be borrowed... | LL | x: &usize, | + help: ...or alternatively, to want to return an owned value | LL - ) -> &usize { LL + ) -> usize { | ``` Fix #85843. --- .../rustc_resolve/src/late/diagnostics.rs | 138 ++++++++++++++++-- src/tools/tidy/src/ui_tests.rs | 2 +- ...nd-lifetime-in-binding-only.elision.stderr | 7 +- ...und-lifetime-in-return-only.elision.stderr | 7 +- tests/ui/c-variadic/variadic-ffi-6.stderr | 11 +- tests/ui/foreign-fn-return-lifetime.fixed | 8 - tests/ui/foreign-fn-return-lifetime.rs | 2 - tests/ui/foreign-fn-return-lifetime.stderr | 9 +- .../issue-70304.stderr | 2 +- .../ui/impl-trait/impl-fn-hrtb-bounds.stderr | 2 +- tests/ui/issues/issue-13497.stderr | 6 +- tests/ui/lifetimes/issue-26638.stderr | 16 +- ...urn-type-requires-explicit-lifetime.stderr | 50 ++++++- tests/ui/self/elision/nested-item.stderr | 11 +- .../impl-trait-missing-lifetime-gated.stderr | 48 +++++- .../impl-trait-missing-lifetime.stderr | 4 +- .../missing-lifetime-specifier.stderr | 15 +- .../suggestions/return-elided-lifetime.stderr | 22 ++- .../underscore-lifetime-binders.stderr | 2 +- 19 files changed, 305 insertions(+), 57 deletions(-) delete mode 100644 tests/ui/foreign-fn-return-lifetime.fixed diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index d62d7fdcae0c2..a32897270a8ad 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -8,7 +8,7 @@ use crate::{PathResult, PathSource, Segment}; use rustc_hir::def::Namespace::{self, *}; use rustc_ast::ptr::P; -use rustc_ast::visit::{FnCtxt, FnKind, LifetimeCtxt}; +use rustc_ast::visit::{walk_ty, FnCtxt, FnKind, LifetimeCtxt, Visitor}; use rustc_ast::{ self as ast, AssocItemKind, Expr, ExprKind, GenericParam, GenericParamKind, Item, ItemKind, MethodCall, NodeId, Path, Ty, TyKind, DUMMY_NODE_ID, @@ -2811,6 +2811,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { .collect(); debug!(?in_scope_lifetimes); + let mut maybe_static = false; debug!(?function_param_lifetimes); if let Some((param_lifetimes, params)) = &function_param_lifetimes { let elided_len = param_lifetimes.len(); @@ -2849,9 +2850,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { if num_params == 0 { err.help( - "this function's return type contains a borrowed value, \ - but there is no value for it to be borrowed from", + "this function's return type contains a borrowed value, but there is no value \ + for it to be borrowed from", ); + maybe_static = true; if in_scope_lifetimes.is_empty() { in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), @@ -2860,10 +2862,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } } else if elided_len == 0 { err.help( - "this function's return type contains a borrowed value with \ - an elided lifetime, but the lifetime cannot be derived from \ - the arguments", + "this function's return type contains a borrowed value with an elided \ + lifetime, but the lifetime cannot be derived from the arguments", ); + maybe_static = true; if in_scope_lifetimes.is_empty() { in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), @@ -2872,13 +2874,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } } else if num_params == 1 { err.help(format!( - "this function's return type contains a borrowed value, \ - but the signature does not say which {m} it is borrowed from" + "this function's return type contains a borrowed value, but the signature does \ + not say which {m} it is borrowed from", )); } else { err.help(format!( - "this function's return type contains a borrowed value, \ - but the signature does not say whether it is borrowed from {m}" + "this function's return type contains a borrowed value, but the signature does \ + not say whether it is borrowed from {m}", )); } } @@ -2943,11 +2945,107 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ); } 1 => { + let post = if maybe_static { + let owned = if let [lt] = &lifetime_refs[..] + && lt.kind != MissingLifetimeKind::Ampersand + { + ", or if you will only have owned values" + } else { + "" + }; + format!( + ", but this is uncommon unless you're returning a borrowed value from a \ + `const` or a `static`{owned}", + ) + } else { + String::new() + }; err.multipart_suggestion_verbose( - format!("consider using the `{existing_name}` lifetime"), + format!("consider using the `{existing_name}` lifetime{post}"), spans_suggs, Applicability::MaybeIncorrect, ); + if maybe_static { + // FIXME: what follows are general suggestions, but we'd want to perform some + // minimal flow analysis to provide more accurate suggestions. For example, if + // we identified that the return expression references only one argument, we + // would suggest borrowing only that argument, and we'd skip the prior + // "use `'static`" suggestion entirely. + if let [lt] = &lifetime_refs[..] && lt.kind == MissingLifetimeKind::Ampersand { + let pre = if let Some((kind, _span)) = + self.diagnostic_metadata.current_function + && let FnKind::Fn(_, _, sig, _, _, _) = kind + && !sig.decl.inputs.is_empty() + && let sugg = sig + .decl + .inputs + .iter() + .filter_map(|param| { + if param.ty.span.contains(lt.span) { + // We don't want to suggest `fn elision(_: &fn() -> &i32)` + // when we have `fn elision(_: fn() -> &i32)` + None + } else if let TyKind::CVarArgs = param.ty.kind { + // Don't suggest `&...` for ffi fn with varargs + None + } else { + Some((param.ty.span.shrink_to_lo(), "&".to_string())) + } + }) + .collect::>() + && !sugg.is_empty() + { + + let (the, s) = if sig.decl.inputs.len() == 1 { + ("the", "") + } else { + ("one of the", "s") + }; + err.multipart_suggestion_verbose( + format!( + "instead, you are more likely to want to change {the} \ + argument{s} to be borrowed...", + ), + sugg, + Applicability::MaybeIncorrect, + ); + "...or alternatively," + } else { + "instead, you are more likely" + }; + let mut sugg = vec![(lt.span, String::new())]; + if let Some((kind, _span)) = + self.diagnostic_metadata.current_function + && let FnKind::Fn(_, _, sig, _, _, _) = kind + && let ast::FnRetTy::Ty(ty) = &sig.decl.output + { + let mut lt_finder = LifetimeFinder { lifetime: lt.span, found: None }; + lt_finder.visit_ty(&ty); + + if let Some(ty) = lt_finder.found { + if let TyKind::Path(None, Path { segments, .. }) = &ty.kind + && segments.len() == 1 + && segments[0].ident.name == sym::str + { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![(lt.span.with_hi(ty.span.hi()), "String".to_string())]; + } + if let TyKind::Slice(inner_ty) = &ty.kind { + // Don't suggest `-> [T]`, suggest `-> Vec`. + sugg = vec![ + (lt.span.with_hi(inner_ty.span.lo()), "Vec<".to_string()), + (ty.span.with_lo(inner_ty.span.hi()), ">".to_string()), + ]; + } + } + }; + err.multipart_suggestion_verbose( + format!("{pre} to want to return an owned value"), + sugg, + Applicability::MaybeIncorrect, + ); + } + } // Record as using the suggested resolution. let (_, (_, res)) = in_scope_lifetimes[0]; @@ -2977,7 +3075,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { fn mk_where_bound_predicate( path: &Path, poly_trait_ref: &ast::PolyTraitRef, - ty: &ast::Ty, + ty: &Ty, ) -> Option { use rustc_span::DUMMY_SP; let modified_segments = { @@ -3054,6 +3152,22 @@ pub(super) fn signal_lifetime_shadowing(sess: &Session, orig: Ident, shadower: I err.emit(); } +struct LifetimeFinder<'ast> { + lifetime: Span, + found: Option<&'ast Ty>, +} + +impl<'ast> Visitor<'ast> for LifetimeFinder<'ast> { + fn visit_ty(&mut self, t: &'ast Ty) { + if t.span.lo() == self.lifetime.lo() + && let TyKind::Ref(_, mut_ty) = &t.kind + { + self.found = Some(&mut_ty.ty); + } + walk_ty(self, t) + } +} + /// Shadowing involving a label is only a warning for historical reasons. //FIXME: make this a proper lint. pub(super) fn signal_label_shadowing(sess: &Session, orig: Span, shadower: Ident) { diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index dfa386b49de7c..40149f8f1c3b6 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. const ISSUES_ENTRY_LIMIT: usize = 1852; -const ROOT_ENTRY_LIMIT: usize = 867; +const ROOT_ENTRY_LIMIT: usize = 866; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files diff --git a/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr b/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr index 4de4afb6e9246..bf22cb3b1aa82 100644 --- a/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr +++ b/tests/ui/associated-types/bound-lifetime-in-binding-only.elision.stderr @@ -5,10 +5,15 @@ LL | fn elision &i32>() { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn elision &'static i32>() { | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn elision &i32>() { +LL + fn elision i32>() { + | error: aborting due to previous error diff --git a/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr b/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr index 7753d186504f8..405d5d9615ba2 100644 --- a/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr +++ b/tests/ui/associated-types/bound-lifetime-in-return-only.elision.stderr @@ -5,10 +5,15 @@ LL | fn elision(_: fn() -> &i32) { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn elision(_: fn() -> &'static i32) { | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn elision(_: fn() -> &i32) { +LL + fn elision(_: fn() -> i32) { + | error: aborting due to previous error diff --git a/tests/ui/c-variadic/variadic-ffi-6.stderr b/tests/ui/c-variadic/variadic-ffi-6.stderr index 4c7792d965019..b3497d31a0cbf 100644 --- a/tests/ui/c-variadic/variadic-ffi-6.stderr +++ b/tests/ui/c-variadic/variadic-ffi-6.stderr @@ -5,10 +5,19 @@ LL | ) -> &usize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | ) -> &'static usize { | +++++++ +help: instead, you are more likely to want to change one of the arguments to be borrowed... + | +LL | x: &usize, + | + +help: ...or alternatively, to want to return an owned value + | +LL - ) -> &usize { +LL + ) -> usize { + | error: aborting due to previous error diff --git a/tests/ui/foreign-fn-return-lifetime.fixed b/tests/ui/foreign-fn-return-lifetime.fixed deleted file mode 100644 index 143d6343d26cd..0000000000000 --- a/tests/ui/foreign-fn-return-lifetime.fixed +++ /dev/null @@ -1,8 +0,0 @@ -// run-rustfix - -extern "C" { - pub fn g(_: &u8) -> &u8; // OK - pub fn f() -> &'static u8; //~ ERROR missing lifetime specifier -} - -fn main() {} diff --git a/tests/ui/foreign-fn-return-lifetime.rs b/tests/ui/foreign-fn-return-lifetime.rs index 76fe50a340ae4..35595bab36d42 100644 --- a/tests/ui/foreign-fn-return-lifetime.rs +++ b/tests/ui/foreign-fn-return-lifetime.rs @@ -1,5 +1,3 @@ -// run-rustfix - extern "C" { pub fn g(_: &u8) -> &u8; // OK pub fn f() -> &u8; //~ ERROR missing lifetime specifier diff --git a/tests/ui/foreign-fn-return-lifetime.stderr b/tests/ui/foreign-fn-return-lifetime.stderr index df1a23a19edd5..d63d13ec8017a 100644 --- a/tests/ui/foreign-fn-return-lifetime.stderr +++ b/tests/ui/foreign-fn-return-lifetime.stderr @@ -1,14 +1,19 @@ error[E0106]: missing lifetime specifier - --> $DIR/foreign-fn-return-lifetime.rs:5:19 + --> $DIR/foreign-fn-return-lifetime.rs:3:19 | LL | pub fn f() -> &u8; | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | pub fn f() -> &'static u8; | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - pub fn f() -> &u8; +LL + pub fn f() -> u8; + | error: aborting due to previous error diff --git a/tests/ui/generic-associated-types/issue-70304.stderr b/tests/ui/generic-associated-types/issue-70304.stderr index 99339e9685959..9b02c1b076837 100644 --- a/tests/ui/generic-associated-types/issue-70304.stderr +++ b/tests/ui/generic-associated-types/issue-70304.stderr @@ -11,7 +11,7 @@ LL | fn create_doc() -> impl Document = DocCursorImpl<'_>> { | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn create_doc() -> impl Document = DocCursorImpl<'static>> { | ~~~~~~~ diff --git a/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr b/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr index 443ffeb55cdee..a5982a5542a54 100644 --- a/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr +++ b/tests/ui/impl-trait/impl-fn-hrtb-bounds.stderr @@ -5,7 +5,7 @@ LL | fn d() -> impl Fn() -> (impl Debug + '_) { | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn d() -> impl Fn() -> (impl Debug + 'static) { | ~~~~~~~ diff --git a/tests/ui/issues/issue-13497.stderr b/tests/ui/issues/issue-13497.stderr index 4b1d979da36e3..5f9aa7162fa64 100644 --- a/tests/ui/issues/issue-13497.stderr +++ b/tests/ui/issues/issue-13497.stderr @@ -5,10 +5,14 @@ LL | &str | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | &'static str | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL | String + | ~~~~~~ error: aborting due to previous error diff --git a/tests/ui/lifetimes/issue-26638.stderr b/tests/ui/lifetimes/issue-26638.stderr index e61158a5d4d02..bea590237bc64 100644 --- a/tests/ui/lifetimes/issue-26638.stderr +++ b/tests/ui/lifetimes/issue-26638.stderr @@ -17,10 +17,18 @@ LL | fn parse_type_2(iter: fn(&u8)->&u8) -> &str { iter() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn parse_type_2(iter: fn(&u8)->&u8) -> &'static str { iter() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn parse_type_2(iter: &fn(&u8)->&u8) -> &str { iter() } + | + +help: ...or alternatively, to want to return an owned value + | +LL | fn parse_type_2(iter: fn(&u8)->&u8) -> String { iter() } + | ~~~~~~ error[E0106]: missing lifetime specifier --> $DIR/issue-26638.rs:10:22 @@ -29,10 +37,14 @@ LL | fn parse_type_3() -> &str { unimplemented!() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn parse_type_3() -> &'static str { unimplemented!() } | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL | fn parse_type_3() -> String { unimplemented!() } + | ~~~~~~ error[E0308]: mismatched types --> $DIR/issue-26638.rs:1:69 diff --git a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr index 5eee953ef189f..caba886a3f7c7 100644 --- a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr +++ b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr @@ -5,10 +5,15 @@ LL | fn f() -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f() -> &'static isize { | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn f() -> &isize { +LL + fn f() -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:7:33 @@ -41,10 +46,19 @@ LL | fn i(_x: isize) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn i(_x: isize) -> &'static isize { | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn i(_x: &isize) -> &isize { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn i(_x: isize) -> &isize { +LL + fn i(_x: isize) -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:34:24 @@ -53,10 +67,19 @@ LL | fn j(_x: StaticStr) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn j(_x: StaticStr) -> &'static isize { | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn j(_x: &StaticStr) -> &isize { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn j(_x: StaticStr) -> &isize { +LL + fn j(_x: StaticStr) -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:40:49 @@ -65,10 +88,19 @@ LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'a` lifetime +help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &'a isize { | ++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn k<'a, T: WithLifetime<'a>>(_x: &T::Output) -> &isize { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { +LL + fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> isize { + | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:45:37 @@ -77,10 +109,18 @@ LL | fn l<'a>(_: &'a str, _: &'a str) -> &str { "" } | ------- ------- ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments -help: consider using the `'a` lifetime +help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn l<'a>(_: &'a str, _: &'a str) -> &'a str { "" } | ++ +help: instead, you are more likely to want to change one of the arguments to be borrowed... + | +LL | fn l<'a>(_: &&'a str, _: &&'a str) -> &str { "" } + | + + +help: ...or alternatively, to want to return an owned value + | +LL | fn l<'a>(_: &'a str, _: &'a str) -> String { "" } + | ~~~~~~ error: aborting due to 7 previous errors diff --git a/tests/ui/self/elision/nested-item.stderr b/tests/ui/self/elision/nested-item.stderr index 752fd82332c38..5b7845559105b 100644 --- a/tests/ui/self/elision/nested-item.stderr +++ b/tests/ui/self/elision/nested-item.stderr @@ -21,10 +21,19 @@ LL | fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &() { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &'static () { | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn wrap(self: &Wrap<{ fn bar(&self) {} }>) -> &() { + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &() { +LL + fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> () { + | error[E0412]: cannot find type `Wrap` in this scope --> $DIR/nested-item.rs:5:15 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index 50806a6725500..c4d156825a339 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -5,10 +5,19 @@ LL | fn g(mut x: impl Iterator) -> Option<&()> { x.next() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn g(mut x: &impl Iterator) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn g(mut x: impl Iterator) -> Option<&()> { x.next() } +LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:19:60 @@ -17,10 +26,19 @@ LL | async fn i(mut x: impl Iterator) -> Option<&()> { x.next() | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | async fn i(mut x: &impl Iterator) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - async fn i(mut x: impl Iterator) -> Option<&()> { x.next() } +LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:27:58 @@ -29,7 +47,7 @@ LL | fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ @@ -41,7 +59,7 @@ LL | async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.n | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ @@ -53,10 +71,19 @@ LL | fn g(mut x: impl Foo) -> Option<&()> { x.next() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn g(mut x: impl Foo) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn g(mut x: &impl Foo) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn g(mut x: impl Foo) -> Option<&()> { x.next() } +LL + fn g(mut x: impl Foo) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:58:41 @@ -65,10 +92,19 @@ LL | fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn g(mut x: impl Foo<()>) -> Option<&'static ()> { x.next() } | +++++++ +help: instead, you are more likely to want to change the argument to be borrowed... + | +LL | fn g(mut x: &impl Foo<()>) -> Option<&()> { x.next() } + | + +help: ...or alternatively, to want to return an owned value + | +LL - fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } +LL + fn g(mut x: impl Foo<()>) -> Option<()> { x.next() } + | error[E0658]: anonymous lifetimes in `impl Trait` are unstable --> $DIR/impl-trait-missing-lifetime-gated.rs:6:35 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr index 2c29cfa0b910f..37c2ad2aaeaaf 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr @@ -5,7 +5,7 @@ LL | fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ @@ -17,7 +17,7 @@ LL | async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.next( | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ diff --git a/tests/ui/suggestions/missing-lifetime-specifier.stderr b/tests/ui/suggestions/missing-lifetime-specifier.stderr index fa4bc2fa79d7c..484a9b3ad8ddb 100644 --- a/tests/ui/suggestions/missing-lifetime-specifier.stderr +++ b/tests/ui/suggestions/missing-lifetime-specifier.stderr @@ -5,7 +5,7 @@ LL | static a: RefCell>>> = RefCell::new(HashMap:: | ^^^ expected 2 lifetime parameters | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | static a: RefCell>>>> = RefCell::new(HashMap::new()); | ++++++++++++++++++ @@ -32,7 +32,7 @@ LL | static b: RefCell>>> = RefCell::new(HashMap: | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | static b: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ ++++++++++++++++++ @@ -59,7 +59,7 @@ LL | static c: RefCell>>>> = RefCell::new(Hash | ^ expected 2 lifetime parameters | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | static c: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++++++++++++ @@ -86,7 +86,7 @@ LL | static d: RefCell>>>> = RefCell::new(Has | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | static d: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ +++++++++++++++++ @@ -113,10 +113,15 @@ LL | static f: RefCell>>>> = RefCell | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - static f: RefCell>>>> = RefCell::new(HashMap::new()); +LL + static f: RefCell>>>> = RefCell::new(HashMap::new()); + | error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:47:44 diff --git a/tests/ui/suggestions/return-elided-lifetime.stderr b/tests/ui/suggestions/return-elided-lifetime.stderr index 273d95bc747d3..54e0cfdf9a326 100644 --- a/tests/ui/suggestions/return-elided-lifetime.stderr +++ b/tests/ui/suggestions/return-elided-lifetime.stderr @@ -5,10 +5,15 @@ LL | fn f1() -> &i32 { loop {} } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f1() -> &'static i32 { loop {} } | +++++++ +help: instead, you are more likely to want to return an owned value + | +LL - fn f1() -> &i32 { loop {} } +LL + fn f1() -> i32 { loop {} } + | error[E0106]: missing lifetime specifiers --> $DIR/return-elided-lifetime.rs:8:14 @@ -19,7 +24,7 @@ LL | fn f1_() -> (&i32, &i32) { loop {} } | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f1_() -> (&'static i32, &'static i32) { loop {} } | +++++++ +++++++ @@ -31,10 +36,19 @@ LL | fn f2(a: i32, b: i32) -> &i32 { loop {} } | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f2(a: i32, b: i32) -> &'static i32 { loop {} } | +++++++ +help: instead, you are more likely to want to change one of the arguments to be borrowed... + | +LL | fn f2(a: &i32, b: &i32) -> &i32 { loop {} } + | + + +help: ...or alternatively, to want to return an owned value + | +LL - fn f2(a: i32, b: i32) -> &i32 { loop {} } +LL + fn f2(a: i32, b: i32) -> i32 { loop {} } + | error[E0106]: missing lifetime specifiers --> $DIR/return-elided-lifetime.rs:13:28 @@ -45,7 +59,7 @@ LL | fn f2_(a: i32, b: i32) -> (&i32, &i32) { loop {} } | expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` | LL | fn f2_(a: i32, b: i32) -> (&'static i32, &'static i32) { loop {} } | +++++++ +++++++ diff --git a/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr b/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr index 50401791effb8..cd74d27dcb55f 100644 --- a/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr +++ b/tests/ui/underscore-lifetime/underscore-lifetime-binders.stderr @@ -28,7 +28,7 @@ LL | fn meh() -> Box Meh<'_>> | ^^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'static` lifetime +help: consider using the `'static` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static`, or if you will only have owned values | LL | fn meh() -> Box Meh<'static>> | ~~~~~~~ From 5fce361d589c870d5fd2f29ff9f0cd3b6485b5be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 14 Nov 2023 22:36:05 +0000 Subject: [PATCH 03/18] Account for impl Trait in lifetime suggestion When encountering ```rust fn g(mut x: impl Iterator) -> Option<&()> { /* */ } ``` Suggest ```rust fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { /* */ } ``` --- .../rustc_resolve/src/late/diagnostics.rs | 77 +++++++++++++++++-- .../impl-trait-missing-lifetime-gated.stderr | 24 +++--- 2 files changed, 84 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index a32897270a8ad..2998609c7c81f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2988,6 +2988,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } else if let TyKind::CVarArgs = param.ty.kind { // Don't suggest `&...` for ffi fn with varargs None + } else if let TyKind::ImplTrait(..) = ¶m.ty.kind { + // We handle these in the next `else if` branch. + None } else { Some((param.ty.span.shrink_to_lo(), "&".to_string())) } @@ -3010,6 +3013,64 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Applicability::MaybeIncorrect, ); "...or alternatively," + } else if let Some((kind, _span)) = + self.diagnostic_metadata.current_function + && let FnKind::Fn(_, _, sig, _, _, _) = kind + && let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output + && !sig.decl.inputs.is_empty() + && let arg_refs = sig + .decl + .inputs + .iter() + .filter_map(|param| match ¶m.ty.kind { + TyKind::ImplTrait(_, bounds) => Some(bounds), + _ => None, + }) + .flat_map(|bounds| bounds.into_iter()) + .collect::>() + && !arg_refs.is_empty() + { + // We have a situation like + // fn g(mut x: impl Iterator) -> Option<&()> + // So we look at every ref in the trait bound. If there's any, we + // suggest + // fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> + let mut lt_finder = LifetimeFinder { + lifetime: lt.span, + found: None, + seen: vec![], + }; + for bound in arg_refs { + if let ast::GenericBound::Trait(trait_ref, _) = bound { + lt_finder.visit_trait_ref(&trait_ref.trait_ref); + } + } + lt_finder.visit_ty(ret_ty); + let spans_suggs: Vec<_> = lt_finder.seen.iter().filter_map(|ty| { + match &ty.kind { + TyKind::Ref(_, mut_ty) => { + let span = ty.span.with_hi(mut_ty.ty.span.lo()); + Some((span, "&'a ".to_string())) + } + _ => None + } + }).collect(); + self.suggest_introducing_lifetime( + err, + None, + |err, higher_ranked, span, message, intro_sugg| { + info!(?span, ?message, ?intro_sugg); + err.multipart_suggestion_verbose( + message, + std::iter::once((span, intro_sugg)) + .chain(spans_suggs.iter().cloned()) + .collect(), + Applicability::MaybeIncorrect, + ); + higher_ranked + }, + ); + "...or alternatively," } else { "instead, you are more likely" }; @@ -3019,7 +3080,11 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ty) = &sig.decl.output { - let mut lt_finder = LifetimeFinder { lifetime: lt.span, found: None }; + let mut lt_finder = LifetimeFinder { + lifetime: lt.span, + found: None, + seen: vec![], + }; lt_finder.visit_ty(&ty); if let Some(ty) = lt_finder.found { @@ -3155,14 +3220,16 @@ pub(super) fn signal_lifetime_shadowing(sess: &Session, orig: Ident, shadower: I struct LifetimeFinder<'ast> { lifetime: Span, found: Option<&'ast Ty>, + seen: Vec<&'ast Ty>, } impl<'ast> Visitor<'ast> for LifetimeFinder<'ast> { fn visit_ty(&mut self, t: &'ast Ty) { - if t.span.lo() == self.lifetime.lo() - && let TyKind::Ref(_, mut_ty) = &t.kind - { - self.found = Some(&mut_ty.ty); + if let TyKind::Ref(_, mut_ty) = &t.kind { + self.seen.push(t); + if t.span.lo() == self.lifetime.lo() { + self.found = Some(&mut_ty.ty); + } } walk_ty(self, t) } diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index c4d156825a339..3425092366f6a 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -9,10 +9,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | fn g(mut x: &impl Iterator) -> Option<&()> { x.next() } - | + +LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ help: ...or alternatively, to want to return an owned value | LL - fn g(mut x: impl Iterator) -> Option<&()> { x.next() } @@ -30,10 +30,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | async fn i(mut x: &impl Iterator) -> Option<&()> { x.next() } - | + +LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ help: ...or alternatively, to want to return an owned value | LL - async fn i(mut x: impl Iterator) -> Option<&()> { x.next() } @@ -75,10 +75,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Foo) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | fn g(mut x: &impl Foo) -> Option<&()> { x.next() } - | + +LL | fn g<'a>(mut x: impl Foo) -> Option<&'a ()> { x.next() } + | ++++ ~~~ help: ...or alternatively, to want to return an owned value | LL - fn g(mut x: impl Foo) -> Option<&()> { x.next() } @@ -96,10 +96,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Foo<()>) -> Option<&'static ()> { x.next() } | +++++++ -help: instead, you are more likely to want to change the argument to be borrowed... +help: consider introducing a named lifetime parameter | -LL | fn g(mut x: &impl Foo<()>) -> Option<&()> { x.next() } - | + +LL | fn g<'a>(mut x: impl Foo<()>) -> Option<&'a ()> { x.next() } + | ++++ ~~~ help: ...or alternatively, to want to return an owned value | LL - fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } From d30252e3593af0c335f86de4242d581ffa11e270 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 14 Nov 2023 22:41:45 +0000 Subject: [PATCH 04/18] Tweak wording --- compiler/rustc_resolve/src/late/diagnostics.rs | 8 ++++---- tests/ui/c-variadic/variadic-ffi-6.stderr | 2 +- tests/ui/lifetimes/issue-26638.stderr | 2 +- ...-elision-return-type-requires-explicit-lifetime.stderr | 8 ++++---- tests/ui/self/elision/nested-item.stderr | 2 +- .../suggestions/impl-trait-missing-lifetime-gated.stderr | 8 ++++---- tests/ui/suggestions/return-elided-lifetime.stderr | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 2998609c7c81f..2cd3e67338252 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3012,7 +3012,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { sugg, Applicability::MaybeIncorrect, ); - "...or alternatively," + "...or alternatively, you might want" } else if let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind @@ -3070,9 +3070,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { higher_ranked }, ); - "...or alternatively," + "alternatively, you might want" } else { - "instead, you are more likely" + "instead, you are more likely to want" }; let mut sugg = vec![(lt.span, String::new())]; if let Some((kind, _span)) = @@ -3105,7 +3105,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } }; err.multipart_suggestion_verbose( - format!("{pre} to want to return an owned value"), + format!("{pre} to return an owned value"), sugg, Applicability::MaybeIncorrect, ); diff --git a/tests/ui/c-variadic/variadic-ffi-6.stderr b/tests/ui/c-variadic/variadic-ffi-6.stderr index b3497d31a0cbf..3511127a9fa32 100644 --- a/tests/ui/c-variadic/variadic-ffi-6.stderr +++ b/tests/ui/c-variadic/variadic-ffi-6.stderr @@ -13,7 +13,7 @@ help: instead, you are more likely to want to change one of the arguments to be | LL | x: &usize, | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - ) -> &usize { LL + ) -> usize { diff --git a/tests/ui/lifetimes/issue-26638.stderr b/tests/ui/lifetimes/issue-26638.stderr index bea590237bc64..ee958686259aa 100644 --- a/tests/ui/lifetimes/issue-26638.stderr +++ b/tests/ui/lifetimes/issue-26638.stderr @@ -25,7 +25,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn parse_type_2(iter: &fn(&u8)->&u8) -> &str { iter() } | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL | fn parse_type_2(iter: fn(&u8)->&u8) -> String { iter() } | ~~~~~~ diff --git a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr index caba886a3f7c7..37ebb178ec621 100644 --- a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr +++ b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr @@ -54,7 +54,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn i(_x: &isize) -> &isize { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn i(_x: isize) -> &isize { LL + fn i(_x: isize) -> isize { @@ -75,7 +75,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn j(_x: &StaticStr) -> &isize { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn j(_x: StaticStr) -> &isize { LL + fn j(_x: StaticStr) -> isize { @@ -96,7 +96,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn k<'a, T: WithLifetime<'a>>(_x: &T::Output) -> &isize { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { LL + fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> isize { @@ -117,7 +117,7 @@ help: instead, you are more likely to want to change one of the arguments to be | LL | fn l<'a>(_: &&'a str, _: &&'a str) -> &str { "" } | + + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL | fn l<'a>(_: &'a str, _: &'a str) -> String { "" } | ~~~~~~ diff --git a/tests/ui/self/elision/nested-item.stderr b/tests/ui/self/elision/nested-item.stderr index 5b7845559105b..7bad26fa13303 100644 --- a/tests/ui/self/elision/nested-item.stderr +++ b/tests/ui/self/elision/nested-item.stderr @@ -29,7 +29,7 @@ help: instead, you are more likely to want to change the argument to be borrowed | LL | fn wrap(self: &Wrap<{ fn bar(&self) {} }>) -> &() { | + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> &() { LL + fn wrap(self: Wrap<{ fn bar(&self) {} }>) -> () { diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index 3425092366f6a..a1ab43922147e 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -13,7 +13,7 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - fn g(mut x: impl Iterator) -> Option<&()> { x.next() } LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } @@ -34,7 +34,7 @@ help: consider introducing a named lifetime parameter | LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - async fn i(mut x: impl Iterator) -> Option<&()> { x.next() } LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } @@ -79,7 +79,7 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Foo) -> Option<&'a ()> { x.next() } | ++++ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - fn g(mut x: impl Foo) -> Option<&()> { x.next() } LL + fn g(mut x: impl Foo) -> Option<()> { x.next() } @@ -100,7 +100,7 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Foo<()>) -> Option<&'a ()> { x.next() } | ++++ ~~~ -help: ...or alternatively, to want to return an owned value +help: alternatively, you might want to return an owned value | LL - fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() } LL + fn g(mut x: impl Foo<()>) -> Option<()> { x.next() } diff --git a/tests/ui/suggestions/return-elided-lifetime.stderr b/tests/ui/suggestions/return-elided-lifetime.stderr index 54e0cfdf9a326..7bfffd30184f8 100644 --- a/tests/ui/suggestions/return-elided-lifetime.stderr +++ b/tests/ui/suggestions/return-elided-lifetime.stderr @@ -44,7 +44,7 @@ help: instead, you are more likely to want to change one of the arguments to be | LL | fn f2(a: &i32, b: &i32) -> &i32 { loop {} } | + + -help: ...or alternatively, to want to return an owned value +help: ...or alternatively, you might want to return an owned value | LL - fn f2(a: i32, b: i32) -> &i32 { loop {} } LL + fn f2(a: i32, b: i32) -> i32 { loop {} } From dec7f00e158f04054320f63796fea1445ac18917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 04:07:19 +0000 Subject: [PATCH 05/18] Fix incorrect lifetime suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 4 ++-- ...urn-type-requires-explicit-lifetime.stderr | 21 ++----------------- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 2cd3e67338252..ce981c333d88a 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2853,8 +2853,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { "this function's return type contains a borrowed value, but there is no value \ for it to be borrowed from", ); - maybe_static = true; if in_scope_lifetimes.is_empty() { + maybe_static = true; in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), (DUMMY_NODE_ID, LifetimeRes::Static), @@ -2865,8 +2865,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { "this function's return type contains a borrowed value with an elided \ lifetime, but the lifetime cannot be derived from the arguments", ); - maybe_static = true; if in_scope_lifetimes.is_empty() { + maybe_static = true; in_scope_lifetimes = vec![( Ident::with_dummy_span(kw::StaticLifetime), (DUMMY_NODE_ID, LifetimeRes::Static), diff --git a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr index 37ebb178ec621..23ef36888f079 100644 --- a/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr +++ b/tests/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr @@ -88,19 +88,10 @@ LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { | ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from -help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` +help: consider using the `'a` lifetime | LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &'a isize { | ++ -help: instead, you are more likely to want to change the argument to be borrowed... - | -LL | fn k<'a, T: WithLifetime<'a>>(_x: &T::Output) -> &isize { - | + -help: ...or alternatively, you might want to return an owned value - | -LL - fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { -LL + fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> isize { - | error[E0106]: missing lifetime specifier --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:45:37 @@ -109,18 +100,10 @@ LL | fn l<'a>(_: &'a str, _: &'a str) -> &str { "" } | ------- ------- ^ expected named lifetime parameter | = help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments -help: consider using the `'a` lifetime, but this is uncommon unless you're returning a borrowed value from a `const` or a `static` +help: consider using the `'a` lifetime | LL | fn l<'a>(_: &'a str, _: &'a str) -> &'a str { "" } | ++ -help: instead, you are more likely to want to change one of the arguments to be borrowed... - | -LL | fn l<'a>(_: &&'a str, _: &&'a str) -> &str { "" } - | + + -help: ...or alternatively, you might want to return an owned value - | -LL | fn l<'a>(_: &'a str, _: &'a str) -> String { "" } - | ~~~~~~ error: aborting due to 7 previous errors From 2a92d820c79e56be8191bdf64e8b2dc7c96210df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 16:56:41 +0000 Subject: [PATCH 06/18] Suggest 'a when trait object assoc type has '_ --- .../rustc_resolve/src/late/diagnostics.rs | 33 ++++++++++++------- .../impl-trait-missing-lifetime-gated.stderr | 8 +++++ .../impl-trait-missing-lifetime.stderr | 8 +++++ 3 files changed, 37 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index ce981c333d88a..984516baa18f4 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2971,9 +2971,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { // we identified that the return expression references only one argument, we // would suggest borrowing only that argument, and we'd skip the prior // "use `'static`" suggestion entirely. - if let [lt] = &lifetime_refs[..] && lt.kind == MissingLifetimeKind::Ampersand { - let pre = if let Some((kind, _span)) = - self.diagnostic_metadata.current_function + if let [lt] = &lifetime_refs[..] + && (lt.kind == MissingLifetimeKind::Ampersand + || lt.kind == MissingLifetimeKind::Underscore) + { + let pre = if lt.kind == MissingLifetimeKind::Ampersand + && let Some((kind, _span)) = + self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && !sig.decl.inputs.is_empty() && let sugg = sig @@ -3013,8 +3017,10 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { Applicability::MaybeIncorrect, ); "...or alternatively, you might want" - } else if let Some((kind, _span)) = - self.diagnostic_metadata.current_function + } else if (lt.kind == MissingLifetimeKind::Ampersand + || lt.kind == MissingLifetimeKind::Underscore) + && let Some((kind, _span)) = + self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output && !sig.decl.inputs.is_empty() @@ -3059,7 +3065,6 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { err, None, |err, higher_ranked, span, message, intro_sugg| { - info!(?span, ?message, ?intro_sugg); err.multipart_suggestion_verbose( message, std::iter::once((span, intro_sugg)) @@ -3093,7 +3098,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { && segments[0].ident.name == sym::str { // Don't suggest `-> str`, suggest `-> String`. - sugg = vec![(lt.span.with_hi(ty.span.hi()), "String".to_string())]; + sugg = vec![ + (lt.span.with_hi(ty.span.hi()), "String".to_string()), + ]; } if let TyKind::Slice(inner_ty) = &ty.kind { // Don't suggest `-> [T]`, suggest `-> Vec`. @@ -3104,11 +3111,13 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } } }; - err.multipart_suggestion_verbose( - format!("{pre} to return an owned value"), - sugg, - Applicability::MaybeIncorrect, - ); + if lt.kind == MissingLifetimeKind::Ampersand { + err.multipart_suggestion_verbose( + format!("{pre} to return an owned value"), + sugg, + Applicability::MaybeIncorrect, + ); + } } } diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index a1ab43922147e..383cba75e285b 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -51,6 +51,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:37:64 @@ -63,6 +67,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:47:37 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr index 37c2ad2aaeaaf..98b0ab82b3ce9 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr @@ -9,6 +9,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | fn g(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime.rs:16:60 @@ -21,6 +25,10 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | async fn i(mut x: impl Iterator) -> Option<&'static ()> { x.next() } | ~~~~~~~ +help: consider introducing a named lifetime parameter + | +LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } + | ++++ ~~~ ~~~ error: lifetime may not live long enough --> $DIR/impl-trait-missing-lifetime.rs:16:69 From 85f26ade8dc3364f29a2c0b0d8f5fe2068df8969 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 18:00:43 +0000 Subject: [PATCH 07/18] Account for '_ in lifetime suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 19 +++++++++++++++---- .../impl-trait-missing-lifetime-gated.stderr | 10 ++++++++++ .../impl-trait-missing-lifetime.stderr | 10 ++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 984516baa18f4..f7560d9949b0b 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3079,6 +3079,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } else { "instead, you are more likely to want" }; + let mut owned_sugg = lt.kind == MissingLifetimeKind::Ampersand; let mut sugg = vec![(lt.span, String::new())]; if let Some((kind, _span)) = self.diagnostic_metadata.current_function @@ -3092,6 +3093,17 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { }; lt_finder.visit_ty(&ty); + if let [Ty { span, kind: TyKind::Ref(_, mut_ty), ..}] + = <_finder.seen[..] + { + // We might have a situation like + // fn g(mut x: impl Iterator) -> Option<&'_ ()> + // but `lt.span` only points at `'_`, so to suggest `-> Option<()>` + // we need to find a more accurate span to end up with + // fn g<'a>(mut x: impl Iterator) -> Option<()> + sugg = vec![(span.with_hi(mut_ty.ty.span.lo()), String::new())]; + owned_sugg = true; + } if let Some(ty) = lt_finder.found { if let TyKind::Path(None, Path { segments, .. }) = &ty.kind && segments.len() == 1 @@ -3101,8 +3113,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { sugg = vec![ (lt.span.with_hi(ty.span.hi()), "String".to_string()), ]; - } - if let TyKind::Slice(inner_ty) = &ty.kind { + } else if let TyKind::Slice(inner_ty) = &ty.kind { // Don't suggest `-> [T]`, suggest `-> Vec`. sugg = vec![ (lt.span.with_hi(inner_ty.span.lo()), "Vec<".to_string()), @@ -3110,8 +3121,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ]; } } - }; - if lt.kind == MissingLifetimeKind::Ampersand { + } + if owned_sugg { err.multipart_suggestion_verbose( format!("{pre} to return an owned value"), sugg, diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr index 383cba75e285b..2dfaa7311948f 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime-gated.stderr @@ -55,6 +55,11 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:37:64 @@ -71,6 +76,11 @@ help: consider introducing a named lifetime parameter | LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime-gated.rs:47:37 diff --git a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr index 98b0ab82b3ce9..c1dbaae0649a0 100644 --- a/tests/ui/suggestions/impl-trait-missing-lifetime.stderr +++ b/tests/ui/suggestions/impl-trait-missing-lifetime.stderr @@ -13,6 +13,11 @@ help: consider introducing a named lifetime parameter | LL | fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - fn g(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + fn g(mut x: impl Iterator) -> Option<()> { x.next() } + | error[E0106]: missing lifetime specifier --> $DIR/impl-trait-missing-lifetime.rs:16:60 @@ -29,6 +34,11 @@ help: consider introducing a named lifetime parameter | LL | async fn i<'a>(mut x: impl Iterator) -> Option<&'a ()> { x.next() } | ++++ ~~~ ~~~ +help: alternatively, you might want to return an owned value + | +LL - async fn i(mut x: impl Iterator) -> Option<&'_ ()> { x.next() } +LL + async fn i(mut x: impl Iterator) -> Option<()> { x.next() } + | error: lifetime may not live long enough --> $DIR/impl-trait-missing-lifetime.rs:16:69 From bb9d720a161221dac6ad7f0fb1cb5e94ab157ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 20:09:19 +0000 Subject: [PATCH 08/18] Do not consider traits as ownable in suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 62 ++++++++++++++++--- .../missing-lifetime-specifier.stderr | 5 -- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index f7560d9949b0b..86846aa214aed 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3105,15 +3105,63 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { owned_sugg = true; } if let Some(ty) = lt_finder.found { - if let TyKind::Path(None, Path { segments, .. }) = &ty.kind + if let TyKind::Path(None, path @ Path { segments, .. }) = &ty.kind && segments.len() == 1 - && segments[0].ident.name == sym::str { - // Don't suggest `-> str`, suggest `-> String`. - sugg = vec![ - (lt.span.with_hi(ty.span.hi()), "String".to_string()), - ]; - } else if let TyKind::Slice(inner_ty) = &ty.kind { + if segments[0].ident.name == sym::str { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![ + (lt.span.with_hi(ty.span.hi()), "String".to_string()), + ]; + } else { + // Check if the path being borrowed is likely to be owned. + let path: Vec<_> = Segment::from_path(path); + match self.resolve_path(&path, Some(TypeNS), None) { + PathResult::Module( + ModuleOrUniformRoot::Module(module), + ) => { + match module.res() { + Some(Res::PrimTy(..)) => {} + Some(Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + )) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } + } + } + PathResult::NonModule(res) => { + match res.base_res() { + Res::PrimTy(..) => {} + Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + ) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } + } + } + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } + } + } + } + if let TyKind::Slice(inner_ty) = &ty.kind { // Don't suggest `-> [T]`, suggest `-> Vec`. sugg = vec![ (lt.span.with_hi(inner_ty.span.lo()), "Vec<".to_string()), diff --git a/tests/ui/suggestions/missing-lifetime-specifier.stderr b/tests/ui/suggestions/missing-lifetime-specifier.stderr index 484a9b3ad8ddb..e41f547ce9b23 100644 --- a/tests/ui/suggestions/missing-lifetime-specifier.stderr +++ b/tests/ui/suggestions/missing-lifetime-specifier.stderr @@ -117,11 +117,6 @@ help: consider using the `'static` lifetime, but this is uncommon unless you're | LL | static f: RefCell>>>> = RefCell::new(HashMap::new()); | +++++++ -help: instead, you are more likely to want to return an owned value - | -LL - static f: RefCell>>>> = RefCell::new(HashMap::new()); -LL + static f: RefCell>>>> = RefCell::new(HashMap::new()); - | error[E0106]: missing lifetime specifier --> $DIR/missing-lifetime-specifier.rs:47:44 From 02bea16c08d6c0202f7cb1658280bd7cf97560d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 15 Nov 2023 20:22:54 +0000 Subject: [PATCH 09/18] Rely in resolve and not on path name for `&str` -> `String` suggestion --- .../rustc_resolve/src/late/diagnostics.rs | 103 +++++++++--------- 1 file changed, 54 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 86846aa214aed..1ffe8a9a26ef4 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -3105,60 +3105,65 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { owned_sugg = true; } if let Some(ty) = lt_finder.found { - if let TyKind::Path(None, path @ Path { segments, .. }) = &ty.kind - && segments.len() == 1 - { - if segments[0].ident.name == sym::str { - // Don't suggest `-> str`, suggest `-> String`. - sugg = vec![ - (lt.span.with_hi(ty.span.hi()), "String".to_string()), - ]; - } else { - // Check if the path being borrowed is likely to be owned. - let path: Vec<_> = Segment::from_path(path); - match self.resolve_path(&path, Some(TypeNS), None) { - PathResult::Module( - ModuleOrUniformRoot::Module(module), - ) => { - match module.res() { - Some(Res::PrimTy(..)) => {} - Some(Res::Def( - DefKind::Struct - | DefKind::Union - | DefKind::Enum - | DefKind::ForeignTy - | DefKind::AssocTy - | DefKind::OpaqueTy - | DefKind::TyParam, - _, - )) => {} - _ => { // Do not suggest in all other cases. - owned_sugg = false; - } + if let TyKind::Path(None, path) = &ty.kind { + // Check if the path being borrowed is likely to be owned. + let path: Vec<_> = Segment::from_path(path); + match self.resolve_path(&path, Some(TypeNS), None) { + PathResult::Module( + ModuleOrUniformRoot::Module(module), + ) => { + match module.res() { + Some(Res::PrimTy(PrimTy::Str)) => { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![( + lt.span.with_hi(ty.span.hi()), + "String".to_string(), + )]; } - } - PathResult::NonModule(res) => { - match res.base_res() { - Res::PrimTy(..) => {} - Res::Def( - DefKind::Struct - | DefKind::Union - | DefKind::Enum - | DefKind::ForeignTy - | DefKind::AssocTy - | DefKind::OpaqueTy - | DefKind::TyParam, - _, - ) => {} - _ => { // Do not suggest in all other cases. - owned_sugg = false; - } + Some(Res::PrimTy(..)) => {} + Some(Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + )) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; } } - _ => { // Do not suggest in all other cases. - owned_sugg = false; + } + PathResult::NonModule(res) => { + match res.base_res() { + Res::PrimTy(PrimTy::Str) => { + // Don't suggest `-> str`, suggest `-> String`. + sugg = vec![( + lt.span.with_hi(ty.span.hi()), + "String".to_string(), + )]; + } + Res::PrimTy(..) => {} + Res::Def( + DefKind::Struct + | DefKind::Union + | DefKind::Enum + | DefKind::ForeignTy + | DefKind::AssocTy + | DefKind::OpaqueTy + | DefKind::TyParam, + _, + ) => {} + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } } } + _ => { // Do not suggest in all other cases. + owned_sugg = false; + } } } if let TyKind::Slice(inner_ty) = &ty.kind { From eee4cc661677be3bf58db12ea1ee598e822762ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 16 Nov 2023 16:55:25 +0000 Subject: [PATCH 10/18] let-chain fmt --- .../rustc_resolve/src/late/diagnostics.rs | 55 ++++++++----------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 1ffe8a9a26ef4..bccce8699c52f 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -2976,8 +2976,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { || lt.kind == MissingLifetimeKind::Underscore) { let pre = if lt.kind == MissingLifetimeKind::Ampersand - && let Some((kind, _span)) = - self.diagnostic_metadata.current_function + && let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && !sig.decl.inputs.is_empty() && let sugg = sig @@ -3002,7 +3001,6 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { .collect::>() && !sugg.is_empty() { - let (the, s) = if sig.decl.inputs.len() == 1 { ("the", "") } else { @@ -3018,9 +3016,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { ); "...or alternatively, you might want" } else if (lt.kind == MissingLifetimeKind::Ampersand - || lt.kind == MissingLifetimeKind::Underscore) - && let Some((kind, _span)) = - self.diagnostic_metadata.current_function + || lt.kind == MissingLifetimeKind::Underscore) + && let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ret_ty) = &sig.decl.output && !sig.decl.inputs.is_empty() @@ -3041,26 +3038,25 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { // So we look at every ref in the trait bound. If there's any, we // suggest // fn g<'a>(mut x: impl Iterator) -> Option<&'a ()> - let mut lt_finder = LifetimeFinder { - lifetime: lt.span, - found: None, - seen: vec![], - }; + let mut lt_finder = + LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] }; for bound in arg_refs { if let ast::GenericBound::Trait(trait_ref, _) = bound { lt_finder.visit_trait_ref(&trait_ref.trait_ref); } } lt_finder.visit_ty(ret_ty); - let spans_suggs: Vec<_> = lt_finder.seen.iter().filter_map(|ty| { - match &ty.kind { + let spans_suggs: Vec<_> = lt_finder + .seen + .iter() + .filter_map(|ty| match &ty.kind { TyKind::Ref(_, mut_ty) => { let span = ty.span.with_hi(mut_ty.ty.span.lo()); Some((span, "&'a ".to_string())) } - _ => None - } - }).collect(); + _ => None, + }) + .collect(); self.suggest_introducing_lifetime( err, None, @@ -3081,20 +3077,16 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { }; let mut owned_sugg = lt.kind == MissingLifetimeKind::Ampersand; let mut sugg = vec![(lt.span, String::new())]; - if let Some((kind, _span)) = - self.diagnostic_metadata.current_function + if let Some((kind, _span)) = self.diagnostic_metadata.current_function && let FnKind::Fn(_, _, sig, _, _, _) = kind && let ast::FnRetTy::Ty(ty) = &sig.decl.output { - let mut lt_finder = LifetimeFinder { - lifetime: lt.span, - found: None, - seen: vec![], - }; + let mut lt_finder = + LifetimeFinder { lifetime: lt.span, found: None, seen: vec![] }; lt_finder.visit_ty(&ty); - if let [Ty { span, kind: TyKind::Ref(_, mut_ty), ..}] - = <_finder.seen[..] + if let [Ty { span, kind: TyKind::Ref(_, mut_ty), .. }] = + <_finder.seen[..] { // We might have a situation like // fn g(mut x: impl Iterator) -> Option<&'_ ()> @@ -3109,9 +3101,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { // Check if the path being borrowed is likely to be owned. let path: Vec<_> = Segment::from_path(path); match self.resolve_path(&path, Some(TypeNS), None) { - PathResult::Module( - ModuleOrUniformRoot::Module(module), - ) => { + PathResult::Module(ModuleOrUniformRoot::Module(module)) => { match module.res() { Some(Res::PrimTy(PrimTy::Str)) => { // Don't suggest `-> str`, suggest `-> String`. @@ -3131,7 +3121,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { | DefKind::TyParam, _, )) => {} - _ => { // Do not suggest in all other cases. + _ => { + // Do not suggest in all other cases. owned_sugg = false; } } @@ -3156,12 +3147,14 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { | DefKind::TyParam, _, ) => {} - _ => { // Do not suggest in all other cases. + _ => { + // Do not suggest in all other cases. owned_sugg = false; } } } - _ => { // Do not suggest in all other cases. + _ => { + // Do not suggest in all other cases. owned_sugg = false; } } From 69a2bd6c044ac4d638f901ff2f5849c6eee3b08e Mon Sep 17 00:00:00 2001 From: Lenko Donchev Date: Sat, 14 Oct 2023 14:47:24 -0500 Subject: [PATCH 11/18] report_not_const_evaluatable_error to avoid ICEing on ConstKind::Expr --- .../error_reporting/type_err_ctxt_ext.rs | 36 +++++++---- .../const_kind_expr/issue_114151.rs | 25 ++++++++ .../const_kind_expr/issue_114151.stderr | 64 +++++++++++++++++++ 3 files changed, 112 insertions(+), 13 deletions(-) create mode 100644 tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.rs create mode 100644 tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 8fa0dceda8742..4988222c135eb 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -3525,20 +3525,30 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { } match obligation.predicate.kind().skip_binder() { - ty::PredicateKind::Clause(ty::ClauseKind::ConstEvaluatable(ct)) => { - let ty::ConstKind::Unevaluated(uv) = ct.kind() else { + ty::PredicateKind::Clause(ty::ClauseKind::ConstEvaluatable(ct)) => match ct.kind() { + ty::ConstKind::Unevaluated(uv) => { + let mut err = + self.tcx.sess.struct_span_err(span, "unconstrained generic constant"); + let const_span = self.tcx.def_span(uv.def); + match self.tcx.sess.source_map().span_to_snippet(const_span) { + Ok(snippet) => err.help(format!( + "try adding a `where` bound using this expression: `where [(); {snippet}]:`" + )), + _ => err.help("consider adding a `where` bound using this expression"), + }; + Some(err) + } + ty::ConstKind::Expr(_) => { + let err = self + .tcx + .sess + .struct_span_err(span, format!("unconstrained generic constant `{ct}`")); + Some(err) + } + _ => { bug!("const evaluatable failed for non-unevaluated const `{ct:?}`"); - }; - let mut err = self.tcx.sess.struct_span_err(span, "unconstrained generic constant"); - let const_span = self.tcx.def_span(uv.def); - match self.tcx.sess.source_map().span_to_snippet(const_span) { - Ok(snippet) => err.help(format!( - "try adding a `where` bound using this expression: `where [(); {snippet}]:`" - )), - _ => err.help("consider adding a `where` bound using this expression"), - }; - Some(err) - } + } + }, _ => { span_bug!( span, diff --git a/tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.rs b/tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.rs new file mode 100644 index 0000000000000..6256000b4919b --- /dev/null +++ b/tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.rs @@ -0,0 +1,25 @@ +#![feature(generic_const_exprs)] +#![allow(incomplete_features)] + +fn foo( + _: [u8; { + { + N + } + }], +) { +} + +fn ice() +where + [(); (L - 1) + 1 + L]:, +{ + foo::<_, L>([(); L + 1 + L]); + //~^ ERROR: mismatched types + //~^^ ERROR: unconstrained generic constant + //~^^^ ERROR: function takes 1 generic argument but 2 generic arguments were supplied + //~^^^^ ERROR: unconstrained generic constant + //~^^^^^ ERROR: unconstrained generic constant `{const expr}` +} + +fn main() {} diff --git a/tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.stderr b/tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.stderr new file mode 100644 index 0000000000000..6001d8247876a --- /dev/null +++ b/tests/ui/const-generics/generic_const_exprs/const_kind_expr/issue_114151.stderr @@ -0,0 +1,64 @@ +error[E0107]: function takes 1 generic argument but 2 generic arguments were supplied + --> $DIR/issue_114151.rs:17:5 + | +LL | foo::<_, L>([(); L + 1 + L]); + | ^^^ - help: remove this generic argument + | | + | expected 1 generic argument + | +note: function defined here, with 1 generic parameter: `N` + --> $DIR/issue_114151.rs:4:4 + | +LL | fn foo( + | ^^^ -------------- + +error[E0308]: mismatched types + --> $DIR/issue_114151.rs:17:18 + | +LL | foo::<_, L>([(); L + 1 + L]); + | ^^ expected `u8`, found `()` + +error: unconstrained generic constant + --> $DIR/issue_114151.rs:17:22 + | +LL | foo::<_, L>([(); L + 1 + L]); + | ^^^^^^^^^ + | + = help: try adding a `where` bound using this expression: `where [(); L + 1 + L]:` + +error: unconstrained generic constant + --> $DIR/issue_114151.rs:17:17 + | +LL | foo::<_, L>([(); L + 1 + L]); + | ----------- ^^^^^^^^^^^^^^^ + | | + | required by a bound introduced by this call + | + = help: try adding a `where` bound using this expression: `where [(); { + { + N + } + }]:` +note: required by a bound in `foo` + --> $DIR/issue_114151.rs:5:13 + | +LL | fn foo( + | --- required by a bound in this function +LL | _: [u8; { + | _____________^ +LL | | { +LL | | N +LL | | } +LL | | }], + | |_____^ required by this bound in `foo` + +error: unconstrained generic constant `{const expr}` + --> $DIR/issue_114151.rs:17:5 + | +LL | foo::<_, L>([(); L + 1 + L]); + | ^^^^^^^^^^^ + +error: aborting due to 5 previous errors + +Some errors have detailed explanations: E0107, E0308. +For more information about an error, try `rustc --explain E0107`. From 226edf64fa58e591d8abc6afef5e38c15c181698 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 12 Dec 2023 09:55:50 +1100 Subject: [PATCH 12/18] Improve an error involving attribute values. Attribute values must be literals. The error you get when that doesn't hold is pretty bad, e.g.: ``` unexpected expression: 1 + 1 ``` You also get the same error if the attribute value is a literal, but an invalid literal, e.g.: ``` unexpected expression: "foo"suffix ``` This commit does two things. - Changes the error message to "attribute value must be a literal", which gives a better idea of what the problem is and how to fix it. It also no longer prints the invalid expression, because the carets below highlight it anyway. - Separates the "not a literal" case from the "invalid literal" case. Which means invalid literals now get the specific error at the literal level, rather than at the attribute level. --- compiler/rustc_parse/src/validate_attr.rs | 60 ++++++++++++------- tests/ui/attributes/issue-90873.rs | 4 +- tests/ui/attributes/issue-90873.stderr | 9 +-- .../attributes/key-value-expansion-on-mac.rs | 4 +- .../key-value-expansion-on-mac.stderr | 2 +- tests/ui/attributes/key-value-expansion.rs | 6 +- .../ui/attributes/key-value-expansion.stderr | 6 +- tests/ui/attributes/unused-item-in-attr.rs | 2 +- .../ui/attributes/unused-item-in-attr.stderr | 4 +- tests/ui/consts/issue-90878-2.rs | 2 +- tests/ui/consts/issue-90878-2.stderr | 2 +- tests/ui/malformed/malformed-interpolated.rs | 4 +- .../malformed/malformed-interpolated.stderr | 4 +- tests/ui/parser/bad-lit-suffixes.rs | 5 +- tests/ui/parser/bad-lit-suffixes.stderr | 27 ++++++--- tests/ui/parser/issues/issue-104620.rs | 2 +- tests/ui/parser/issues/issue-104620.stderr | 6 +- 17 files changed, 87 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_parse/src/validate_attr.rs b/compiler/rustc_parse/src/validate_attr.rs index cbe75b3dab6ee..81055431f649d 100644 --- a/compiler/rustc_parse/src/validate_attr.rs +++ b/compiler/rustc_parse/src/validate_attr.rs @@ -6,9 +6,9 @@ use rustc_ast::token::Delimiter; use rustc_ast::tokenstream::DelimSpan; use rustc_ast::MetaItemKind; use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, Attribute, DelimArgs, MetaItem}; -use rustc_ast_pretty::pprust; use rustc_errors::{Applicability, FatalError, PResult}; use rustc_feature::{AttributeTemplate, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP}; +use rustc_session::errors::report_lit_error; use rustc_session::lint::builtin::ILL_FORMED_ATTRIBUTE_INPUT; use rustc_session::parse::ParseSess; use rustc_span::{sym, Span, Symbol}; @@ -51,28 +51,44 @@ pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, Meta MetaItemKind::List(nmis) } AttrArgs::Eq(_, AttrArgsEq::Ast(expr)) => { - if let ast::ExprKind::Lit(token_lit) = expr.kind - && let Ok(lit) = ast::MetaItemLit::from_token_lit(token_lit, expr.span) - { - if token_lit.suffix.is_some() { - let mut err = sess.span_diagnostic.struct_span_err( - expr.span, - "suffixed literals are not allowed in attributes", - ); - err.help( - "instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), \ - use an unsuffixed version (`1`, `1.0`, etc.)", - ); - return Err(err); - } else { - MetaItemKind::NameValue(lit) - } + if let ast::ExprKind::Lit(token_lit) = expr.kind { + let res = ast::MetaItemLit::from_token_lit(token_lit, expr.span); + let res = match res { + Ok(lit) => { + if token_lit.suffix.is_some() { + let mut err = sess.span_diagnostic.struct_span_err( + expr.span, + "suffixed literals are not allowed in attributes", + ); + err.help( + "instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), \ + use an unsuffixed version (`1`, `1.0`, etc.)", + ); + return Err(err); + } else { + MetaItemKind::NameValue(lit) + } + } + Err(err) => { + report_lit_error(sess, err, token_lit, expr.span); + let lit = ast::MetaItemLit { + symbol: token_lit.symbol, + suffix: token_lit.suffix, + kind: ast::LitKind::Err, + span: expr.span, + }; + MetaItemKind::NameValue(lit) + } + }; + res } else { - // The non-error case can happen with e.g. `#[foo = 1+1]`. The error case can - // happen with e.g. `#[foo = include_str!("nonexistent-file.rs")]`; in that - // case we delay the error because an earlier error will have already been - // reported. - let msg = format!("unexpected expression: `{}`", pprust::expr_to_string(expr)); + // Example cases: + // - `#[foo = 1+1]`: results in `ast::ExprKind::BinOp`. + // - `#[foo = include_str!("nonexistent-file.rs")]`: + // results in `ast::ExprKind::Err`. In that case we delay + // the error because an earlier error will have already + // been reported. + let msg = format!("attribute value must be a literal"); let mut err = sess.span_diagnostic.struct_span_err(expr.span, msg); if let ast::ExprKind::Err = expr.kind { err.downgrade_to_delayed_bug(); diff --git a/tests/ui/attributes/issue-90873.rs b/tests/ui/attributes/issue-90873.rs index 1411f61744d25..53339ce7e28fb 100644 --- a/tests/ui/attributes/issue-90873.rs +++ b/tests/ui/attributes/issue-90873.rs @@ -1,9 +1,9 @@ #![u=||{static d=||1;}] -//~^ unexpected expression +//~^ attribute value must be a literal //~| cannot find attribute `u` in this scope //~| missing type for `static` item #![a={impl std::ops::Neg for i8 {}}] -//~^ ERROR unexpected expression +//~^ ERROR attribute value must be a literal //~| ERROR cannot find attribute `a` in this scope //~| ERROR `main` function not found in crate `issue_90873` diff --git a/tests/ui/attributes/issue-90873.stderr b/tests/ui/attributes/issue-90873.stderr index 894ec8341f8e0..5a8bbaf8ec191 100644 --- a/tests/ui/attributes/issue-90873.stderr +++ b/tests/ui/attributes/issue-90873.stderr @@ -1,15 +1,10 @@ -error: unexpected expression: `|| - { - static d: _ = || 1; - }` +error: attribute value must be a literal --> $DIR/issue-90873.rs:1:6 | LL | #![u=||{static d=||1;}] | ^^^^^^^^^^^^^^^^^ -error: unexpected expression: `{ - impl std::ops::Neg for i8 {} - }` +error: attribute value must be a literal --> $DIR/issue-90873.rs:6:6 | LL | #![a={impl std::ops::Neg for i8 {}}] diff --git a/tests/ui/attributes/key-value-expansion-on-mac.rs b/tests/ui/attributes/key-value-expansion-on-mac.rs index c1d68d8cda9e7..ea7cf7c4f640f 100644 --- a/tests/ui/attributes/key-value-expansion-on-mac.rs +++ b/tests/ui/attributes/key-value-expansion-on-mac.rs @@ -7,8 +7,8 @@ macro_rules! bar { // FIXME?: `bar` here expands before `stringify` has a chance to expand. // `#[rustc_dummy = ...]` is validated and dropped during expansion of `bar`, -// the "unexpected expression" errors comes from the validation. -#[rustc_dummy = stringify!(b)] //~ ERROR unexpected expression: `stringify!(b)` +// the "attribute value must be a literal" error comes from the validation. +#[rustc_dummy = stringify!(b)] //~ ERROR attribute value must be a literal bar!(); fn main() {} diff --git a/tests/ui/attributes/key-value-expansion-on-mac.stderr b/tests/ui/attributes/key-value-expansion-on-mac.stderr index 7d817da13626d..260462cfeefac 100644 --- a/tests/ui/attributes/key-value-expansion-on-mac.stderr +++ b/tests/ui/attributes/key-value-expansion-on-mac.stderr @@ -1,4 +1,4 @@ -error: unexpected expression: `stringify!(b)` +error: attribute value must be a literal --> $DIR/key-value-expansion-on-mac.rs:11:17 | LL | #[rustc_dummy = stringify!(b)] diff --git a/tests/ui/attributes/key-value-expansion.rs b/tests/ui/attributes/key-value-expansion.rs index 83d601e5e3a79..3065c12749c2c 100644 --- a/tests/ui/attributes/key-value-expansion.rs +++ b/tests/ui/attributes/key-value-expansion.rs @@ -18,13 +18,13 @@ macro_rules! bug { // Any expressions containing macro call `X` that's more complex than `X` itself. // Parentheses will work. -bug!((column!())); //~ ERROR unexpected expression: `(7u32)` +bug!((column!())); //~ ERROR attribute value must be a literal // Original test case. macro_rules! bug { () => { - bug!("bug" + stringify!(found)); //~ ERROR unexpected expression: `"bug" + "found"` + bug!("bug" + stringify!(found)); //~ ERROR attribute value must be a literal }; ($test:expr) => { #[doc = $test] @@ -46,7 +46,7 @@ macro_rules! doc_comment { macro_rules! some_macro { ($t1: ty) => { doc_comment! {format!("{coor}", coor = stringify!($t1)).as_str()} - //~^ ERROR unexpected expression: `{ + //~^ ERROR attribute value must be a literal }; } diff --git a/tests/ui/attributes/key-value-expansion.stderr b/tests/ui/attributes/key-value-expansion.stderr index aaa8b169583fd..54d79c5bebb7f 100644 --- a/tests/ui/attributes/key-value-expansion.stderr +++ b/tests/ui/attributes/key-value-expansion.stderr @@ -1,10 +1,10 @@ -error: unexpected expression: `(7u32)` +error: attribute value must be a literal --> $DIR/key-value-expansion.rs:21:6 | LL | bug!((column!())); | ^^^^^^^^^^^ -error: unexpected expression: `"bug" + "found"` +error: attribute value must be a literal --> $DIR/key-value-expansion.rs:27:14 | LL | bug!("bug" + stringify!(found)); @@ -15,7 +15,7 @@ LL | bug!(); | = note: this error originates in the macro `bug` (in Nightly builds, run with -Z macro-backtrace for more info) -error: unexpected expression: `{ let res = ::alloc::fmt::format(format_args!("{0}", "u8")); res }.as_str()` +error: attribute value must be a literal --> $DIR/key-value-expansion.rs:48:23 | LL | doc_comment! {format!("{coor}", coor = stringify!($t1)).as_str()} diff --git a/tests/ui/attributes/unused-item-in-attr.rs b/tests/ui/attributes/unused-item-in-attr.rs index 70dcd5413f1ee..fda0a5d6a3f16 100644 --- a/tests/ui/attributes/unused-item-in-attr.rs +++ b/tests/ui/attributes/unused-item-in-attr.rs @@ -1,5 +1,5 @@ #[w = { extern crate alloc; }] -//~^ ERROR unexpected expression: `{ +//~^ ERROR attribute value must be a literal //~| ERROR cannot find attribute `w` in this scope fn f() {} diff --git a/tests/ui/attributes/unused-item-in-attr.stderr b/tests/ui/attributes/unused-item-in-attr.stderr index 92a8f58582136..84130965d31cf 100644 --- a/tests/ui/attributes/unused-item-in-attr.stderr +++ b/tests/ui/attributes/unused-item-in-attr.stderr @@ -1,6 +1,4 @@ -error: unexpected expression: `{ - extern crate alloc; - }` +error: attribute value must be a literal --> $DIR/unused-item-in-attr.rs:1:7 | LL | #[w = { extern crate alloc; }] diff --git a/tests/ui/consts/issue-90878-2.rs b/tests/ui/consts/issue-90878-2.rs index e5bcecce6ee8f..0e61c65305fbe 100644 --- a/tests/ui/consts/issue-90878-2.rs +++ b/tests/ui/consts/issue-90878-2.rs @@ -1,4 +1,4 @@ - #![l=|x|[b;x ]] //~ ERROR unexpected expression: `|x| [b; x]` + #![l=|x|[b;x ]] //~ ERROR attribute value must be a literal //~^ ERROR cannot find attribute `l` in this scope // notice the space at the start, diff --git a/tests/ui/consts/issue-90878-2.stderr b/tests/ui/consts/issue-90878-2.stderr index 71b8d21fb4dc2..0b332840042ef 100644 --- a/tests/ui/consts/issue-90878-2.stderr +++ b/tests/ui/consts/issue-90878-2.stderr @@ -1,4 +1,4 @@ -error: unexpected expression: `|x| [b; x]` +error: attribute value must be a literal --> $DIR/issue-90878-2.rs:1:7 | LL | #![l=|x|[b;x ]] diff --git a/tests/ui/malformed/malformed-interpolated.rs b/tests/ui/malformed/malformed-interpolated.rs index 0d84e723fc3dd..0d801ebd3ac9a 100644 --- a/tests/ui/malformed/malformed-interpolated.rs +++ b/tests/ui/malformed/malformed-interpolated.rs @@ -10,7 +10,7 @@ macro_rules! check { check!("0"); // OK check!(0); // OK check!(0u8); //~ ERROR suffixed literals are not allowed in attributes -check!(-0); //~ ERROR unexpected expression: `-0` -check!(0 + 0); //~ ERROR unexpected expression: `0 + 0` +check!(-0); //~ ERROR attribute value must be a literal +check!(0 + 0); //~ ERROR attribute value must be a literal fn main() {} diff --git a/tests/ui/malformed/malformed-interpolated.stderr b/tests/ui/malformed/malformed-interpolated.stderr index c24d9f15388fc..92e99b7a70b34 100644 --- a/tests/ui/malformed/malformed-interpolated.stderr +++ b/tests/ui/malformed/malformed-interpolated.stderr @@ -6,13 +6,13 @@ LL | check!(0u8); | = help: instead of using a suffixed literal (`1u8`, `1.0f32`, etc.), use an unsuffixed version (`1`, `1.0`, etc.) -error: unexpected expression: `-0` +error: attribute value must be a literal --> $DIR/malformed-interpolated.rs:13:8 | LL | check!(-0); | ^^ -error: unexpected expression: `0 + 0` +error: attribute value must be a literal --> $DIR/malformed-interpolated.rs:14:8 | LL | check!(0 + 0); diff --git a/tests/ui/parser/bad-lit-suffixes.rs b/tests/ui/parser/bad-lit-suffixes.rs index 8cb9ef7e0c921..c614f49388574 100644 --- a/tests/ui/parser/bad-lit-suffixes.rs +++ b/tests/ui/parser/bad-lit-suffixes.rs @@ -28,11 +28,12 @@ fn main() { } #[rustc_dummy = "string"suffix] -//~^ ERROR unexpected expression: `"string"suffix` +//~^ ERROR suffixes on string literals are invalid fn f() {} #[must_use = "string"suffix] -//~^ ERROR unexpected expression: `"string"suffix` +//~^ ERROR suffixes on string literals are invalid +//~| ERROR malformed `must_use` attribute input fn g() {} #[link(name = "string"suffix)] diff --git a/tests/ui/parser/bad-lit-suffixes.stderr b/tests/ui/parser/bad-lit-suffixes.stderr index 756f99ab12c82..b5dacdf7d0d3d 100644 --- a/tests/ui/parser/bad-lit-suffixes.stderr +++ b/tests/ui/parser/bad-lit-suffixes.stderr @@ -10,26 +10,39 @@ error: suffixes on string literals are invalid LL | "C"suffix | ^^^^^^^^^ invalid suffix `suffix` -error: unexpected expression: `"string"suffix` +error: suffixes on string literals are invalid --> $DIR/bad-lit-suffixes.rs:30:17 | LL | #[rustc_dummy = "string"suffix] - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ invalid suffix `suffix` -error: unexpected expression: `"string"suffix` +error: suffixes on string literals are invalid --> $DIR/bad-lit-suffixes.rs:34:14 | LL | #[must_use = "string"suffix] - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ invalid suffix `suffix` + +error: malformed `must_use` attribute input + --> $DIR/bad-lit-suffixes.rs:34:1 + | +LL | #[must_use = "string"suffix] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: the following are the possible correct uses + | +LL | #[must_use = "reason"] + | +LL | #[must_use] + | error: suffixes on string literals are invalid - --> $DIR/bad-lit-suffixes.rs:38:15 + --> $DIR/bad-lit-suffixes.rs:39:15 | LL | #[link(name = "string"suffix)] | ^^^^^^^^^^^^^^ invalid suffix `suffix` error: invalid suffix `suffix` for number literal - --> $DIR/bad-lit-suffixes.rs:42:41 + --> $DIR/bad-lit-suffixes.rs:43:41 | LL | #[rustc_layout_scalar_valid_range_start(0suffix)] | ^^^^^^^ invalid suffix `suffix` @@ -136,5 +149,5 @@ LL | 1.0e10suffix; | = help: valid suffixes are `f32` and `f64` -error: aborting due to 20 previous errors +error: aborting due to 21 previous errors diff --git a/tests/ui/parser/issues/issue-104620.rs b/tests/ui/parser/issues/issue-104620.rs index f49476c44084c..fd0916b44110a 100644 --- a/tests/ui/parser/issues/issue-104620.rs +++ b/tests/ui/parser/issues/issue-104620.rs @@ -1,4 +1,4 @@ #![feature(rustc_attrs)] -#![rustc_dummy=5z] //~ ERROR unexpected expression: `5z` +#![rustc_dummy=5z] //~ ERROR invalid suffix `z` for number literal fn main() {} diff --git a/tests/ui/parser/issues/issue-104620.stderr b/tests/ui/parser/issues/issue-104620.stderr index fa20b5f8b1625..040c63a5fbfbf 100644 --- a/tests/ui/parser/issues/issue-104620.stderr +++ b/tests/ui/parser/issues/issue-104620.stderr @@ -1,8 +1,10 @@ -error: unexpected expression: `5z` +error: invalid suffix `z` for number literal --> $DIR/issue-104620.rs:3:16 | LL | #![rustc_dummy=5z] - | ^^ + | ^^ invalid suffix `z` + | + = help: the suffix must be one of the numeric types (`u32`, `isize`, `f32`, etc.) error: aborting due to 1 previous error From 4f8083374d99c50868716b53c9925962527b1384 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 11 Dec 2023 22:24:44 -0700 Subject: [PATCH 13/18] rustdoc-search: clean up parser The `c === "="` was redundant when `isSeparatorCharacter` already checks that. The function `isStopCharacter` and `isEndCharacter` functions did exactly the same thing and have synonymous names. There doesn't seem much point in having both. --- src/librustdoc/html/static/js/search.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/html/static/js/search.js b/src/librustdoc/html/static/js/search.js index a521bf66bedd8..f2875b7f01e45 100644 --- a/src/librustdoc/html/static/js/search.js +++ b/src/librustdoc/html/static/js/search.js @@ -298,10 +298,6 @@ function initSearch(rawSearchIndex) { return "=,>-]".indexOf(c) !== -1; } - function isStopCharacter(c) { - return isEndCharacter(c); - } - function isErrorCharacter(c) { return "()".indexOf(c) !== -1; } @@ -617,8 +613,7 @@ function initSearch(rawSearchIndex) { } } else if ( c === "[" || - c === "=" || - isStopCharacter(c) || + isEndCharacter(c) || isSpecialStartCharacter(c) || isSeparatorCharacter(c) ) { @@ -917,7 +912,7 @@ function initSearch(rawSearchIndex) { while (parserState.pos < parserState.length) { const c = parserState.userQuery[parserState.pos]; - if (isStopCharacter(c)) { + if (isEndCharacter(c)) { foundStopChar = true; if (isSeparatorCharacter(c)) { parserState.pos += 1; From f44ccbab2bbdc538b739d6a1f5f9fbdb8339f8f8 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Tue, 12 Dec 2023 13:14:53 +0100 Subject: [PATCH 14/18] rustc_codegen_llvm: Enforce `rustc::potential_query_instability` lint Stop allowing `rustc::potential_query_instability` on all of `rustc_codegen_llvm` and instead allow it on a case-by-case basis. In this case, both instances are safe to allow. --- compiler/rustc_codegen_llvm/src/back/lto.rs | 3 +++ compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs | 5 +++++ compiler/rustc_codegen_llvm/src/lib.rs | 1 - 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index abc33a045982a..97444f1a5bd39 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -816,6 +816,9 @@ impl ThinLTOKeysMap { use std::io::Write; let file = File::create(path)?; let mut writer = io::BufWriter::new(file); + // The entries are loaded back into a hash map in `load_from_file()`, so + // the order in which we write them to file here does not matter. + #[allow(rustc::potential_query_instability)] for (module, key) in &self.keys { writeln!(writer, "{module} {key}")?; } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 51df14df644e0..33bfde03a31c3 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -58,6 +58,11 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) { return; } + // The entries of the map are only used to get a list of all files with + // coverage info. In the end the list of files is passed into + // `GlobalFileTable::new()` which internally do `.sort_unstable_by()`, so + // the iteration order here does not matter. + #[allow(rustc::potential_query_instability)] let function_coverage_entries = function_coverage_map .into_iter() .map(|(instance, function_coverage)| (instance, function_coverage.into_finished())) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 33b19ab362af2..116108ae5a987 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -17,7 +17,6 @@ #![feature(never_type)] #![feature(impl_trait_in_assoc_type)] #![recursion_limit = "256"] -#![allow(rustc::potential_query_instability)] #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] From 46a801559127441675f2341bd1d684809a47def1 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Tue, 12 Dec 2023 12:28:02 +0000 Subject: [PATCH 15/18] llvm-wrapper: adapt for LLVM API change LLVM commit https://github.com/llvm/llvm-project/commit/f09cf34d00625e57dea5317a3ac0412c07292148 moved some functions to a different header: https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/24416#018c5de6-b9c9-4b22-9473-6070d99dcfa7/233-537 --- compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp index 2601d96b8c869..556dc890a8429 100644 --- a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp @@ -37,7 +37,9 @@ #include "llvm/Transforms/Utils/FunctionImportUtils.h" #include "llvm/LTO/LTO.h" #include "llvm/Bitcode/BitcodeWriter.h" - +#if LLVM_VERSION_GE(18, 0) +#include "llvm/TargetParser/Host.h" +#endif #include "llvm/Transforms/Instrumentation.h" #include "llvm/Transforms/Instrumentation/AddressSanitizer.h" #include "llvm/Support/TimeProfiler.h" From e274372689928972e4e78a24d615f6c4d375f7d4 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 12 Dec 2023 13:33:39 +0100 Subject: [PATCH 16/18] Correctly gate the parsing of match arms without body --- compiler/rustc_ast_lowering/src/expr.rs | 7 +- compiler/rustc_ast_passes/messages.ftl | 4 + compiler/rustc_ast_passes/src/errors.rs | 9 ++ compiler/rustc_ast_passes/src/feature_gate.rs | 29 ++++- compiler/rustc_parse/src/parser/expr.rs | 7 +- .../feature-gate-never_patterns.rs | 65 ++++++++-- .../feature-gate-never_patterns.stderr | 114 +++++++++++++++++- tests/ui/parser/match-arm-without-body.rs | 2 - tests/ui/parser/match-arm-without-body.stderr | 16 +-- 9 files changed, 227 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index e9f88d5093714..a44b408feec25 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -581,8 +581,11 @@ impl<'hir> LoweringContext<'_, 'hir> { } else { // Either `body.is_none()` or `is_never_pattern` here. if !is_never_pattern { - let suggestion = span.shrink_to_hi(); - self.tcx.sess.emit_err(MatchArmWithNoBody { span, suggestion }); + if self.tcx.features().never_patterns { + // If the feature is off we already emitted the error after parsing. + let suggestion = span.shrink_to_hi(); + self.tcx.sess.emit_err(MatchArmWithNoBody { span, suggestion }); + } } else if let Some(body) = &arm.body { self.tcx.sess.emit_err(NeverPatternWithBody { span: body.span }); guard = None; diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl index 876126b02ea95..28bd6c2111b72 100644 --- a/compiler/rustc_ast_passes/messages.ftl +++ b/compiler/rustc_ast_passes/messages.ftl @@ -174,6 +174,10 @@ ast_passes_item_underscore = `{$kind}` items in this context need a name ast_passes_keyword_lifetime = lifetimes cannot use keyword names +ast_passes_match_arm_with_no_body = + `match` arm with no body + .suggestion = add a body after the pattern + ast_passes_module_nonascii = trying to load file for module `{$name}` with non-ascii identifier name .help = consider using the `#[path]` attribute to specify filesystem path diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index 7f6fcb493171c..928bf19759ada 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -758,3 +758,12 @@ pub struct AnonStructOrUnionNotAllowed { pub span: Span, pub struct_or_union: &'static str, } + +#[derive(Diagnostic)] +#[diag(ast_passes_match_arm_with_no_body)] +pub struct MatchArmWithNoBody { + #[primary_span] + pub span: Span, + #[suggestion(code = " => todo!(),", applicability = "has-placeholders")] + pub suggestion: Span, +} diff --git a/compiler/rustc_ast_passes/src/feature_gate.rs b/compiler/rustc_ast_passes/src/feature_gate.rs index 897f35a5c4ad9..ac55c6cabd0eb 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -554,7 +554,34 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session, features: &Features) { gate_all!(explicit_tail_calls, "`become` expression is experimental"); gate_all!(generic_const_items, "generic const items are experimental"); gate_all!(unnamed_fields, "unnamed fields are not yet fully implemented"); - gate_all!(never_patterns, "`!` patterns are experimental"); + + if !visitor.features.never_patterns { + if let Some(spans) = spans.get(&sym::never_patterns) { + for &span in spans { + if span.allows_unstable(sym::never_patterns) { + continue; + } + let sm = sess.source_map(); + // We gate two types of spans: the span of a `!` pattern, and the span of a + // match arm without a body. For the latter we want to give the user a normal + // error. + if let Ok(snippet) = sm.span_to_snippet(span) + && snippet == "!" + { + feature_err( + &sess.parse_sess, + sym::never_patterns, + span, + "`!` patterns are experimental", + ) + .emit(); + } else { + let suggestion = span.shrink_to_hi(); + sess.emit_err(errors::MatchArmWithNoBody { span, suggestion }); + } + } + } + } if !visitor.features.negative_bounds { for &span in spans.get(&sym::negative_bounds).iter().copied().flatten() { diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 3c0627526bed5..bdaf8db131160 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -2918,7 +2918,12 @@ impl<'a> Parser<'a> { let mut result = if !is_fat_arrow && !is_almost_fat_arrow { // A pattern without a body, allowed for never patterns. arm_body = None; - this.expect_one_of(&[token::Comma], &[token::CloseDelim(Delimiter::Brace)]) + this.expect_one_of(&[token::Comma], &[token::CloseDelim(Delimiter::Brace)]).map( + |x| { + this.sess.gated_spans.gate(sym::never_patterns, pat.span); + x + }, + ) } else { if let Err(mut err) = this.expect(&token::FatArrow) { // We might have a `=>` -> `=` or `->` typo (issue #89396). diff --git a/tests/ui/feature-gates/feature-gate-never_patterns.rs b/tests/ui/feature-gates/feature-gate-never_patterns.rs index ca5ce3b948943..97c68e460ea70 100644 --- a/tests/ui/feature-gates/feature-gate-never_patterns.rs +++ b/tests/ui/feature-gates/feature-gate-never_patterns.rs @@ -12,16 +12,67 @@ fn main() { unsafe { let ptr: *const Void = NonNull::dangling().as_ptr(); match *ptr { - ! //~ ERROR `!` patterns are experimental + ! + //~^ ERROR `!` patterns are experimental + //~| ERROR `!` patterns are experimental + } + // Check that the gate operates even behind `cfg`. + #[cfg(FALSE)] + match *ptr { + ! + //~^ ERROR `!` patterns are experimental + //~| ERROR `!` patterns are experimental + } + #[cfg(FALSE)] + match *ptr { + ! => {} + //~^ ERROR `!` patterns are experimental } } + // Correctly gate match arms with no body. + match Some(0) { + None => {} + Some(_), + //~^ ERROR unexpected `,` in pattern + } + match Some(0) { + None => {} + Some(_) + //~^ ERROR `match` arm with no body + } + match Some(0) { + _ => {} + Some(_) if false, + //~^ ERROR `match` arm with no body + Some(_) if false + //~^ ERROR `match` arm with no body + } + match res { + Ok(_) => {} + Err(!), + //~^ ERROR `match` arm with no body + //~| ERROR `!` patterns are experimental + } + match res { + Err(!) if false, + //~^ ERROR `match` arm with no body + //~| ERROR a guard on a never pattern will never be run + //~| ERROR `!` patterns are experimental + _ => {} + } + // Check that the gate operates even behind `cfg`. - #[cfg(FALSE)] - unsafe { - let ptr: *const Void = NonNull::dangling().as_ptr(); - match *ptr { - ! => {} //~ ERROR `!` patterns are experimental - } + match Some(0) { + None => {} + #[cfg(FALSE)] + Some(_) + //~^ ERROR `match` arm with no body + } + match Some(0) { + _ => {} + #[cfg(FALSE)] + Some(_) if false + //~^ ERROR `match` arm with no body } } diff --git a/tests/ui/feature-gates/feature-gate-never_patterns.stderr b/tests/ui/feature-gates/feature-gate-never_patterns.stderr index 2354a3b0476d8..4ba80603d9f1c 100644 --- a/tests/ui/feature-gates/feature-gate-never_patterns.stderr +++ b/tests/ui/feature-gates/feature-gate-never_patterns.stderr @@ -1,3 +1,18 @@ +error: unexpected `,` in pattern + --> $DIR/feature-gate-never_patterns.rs:36:16 + | +LL | Some(_), + | ^ + | +help: try adding parentheses to match on a tuple... + | +LL | (Some(_),) + | + + +help: ...or a vertical bar to match on multiple alternatives + | +LL | Some(_) | + | + error[E0408]: variable `_x` is not bound in all patterns --> $DIR/feature-gate-never_patterns.rs:8:19 | @@ -25,7 +40,36 @@ LL | ! = help: add `#![feature(never_patterns)]` to the crate attributes to enable error[E0658]: `!` patterns are experimental - --> $DIR/feature-gate-never_patterns.rs:24:13 + --> $DIR/feature-gate-never_patterns.rs:15:13 + | +LL | ! + | ^ + | + = note: see issue #118155 for more information + = help: add `#![feature(never_patterns)]` to the crate attributes to enable + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0658]: `!` patterns are experimental + --> $DIR/feature-gate-never_patterns.rs:22:13 + | +LL | ! + | ^ + | + = note: see issue #118155 for more information + = help: add `#![feature(never_patterns)]` to the crate attributes to enable + +error[E0658]: `!` patterns are experimental + --> $DIR/feature-gate-never_patterns.rs:22:13 + | +LL | ! + | ^ + | + = note: see issue #118155 for more information + = help: add `#![feature(never_patterns)]` to the crate attributes to enable + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0658]: `!` patterns are experimental + --> $DIR/feature-gate-never_patterns.rs:28:13 | LL | ! => {} | ^ @@ -33,7 +77,73 @@ LL | ! => {} = note: see issue #118155 for more information = help: add `#![feature(never_patterns)]` to the crate attributes to enable -error: aborting due to 4 previous errors +error: `match` arm with no body + --> $DIR/feature-gate-never_patterns.rs:41:9 + | +LL | Some(_) + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` + +error: `match` arm with no body + --> $DIR/feature-gate-never_patterns.rs:46:9 + | +LL | Some(_) if false, + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` + +error: `match` arm with no body + --> $DIR/feature-gate-never_patterns.rs:48:9 + | +LL | Some(_) if false + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` + +error[E0658]: `!` patterns are experimental + --> $DIR/feature-gate-never_patterns.rs:53:13 + | +LL | Err(!), + | ^ + | + = note: see issue #118155 for more information + = help: add `#![feature(never_patterns)]` to the crate attributes to enable + +error: `match` arm with no body + --> $DIR/feature-gate-never_patterns.rs:53:9 + | +LL | Err(!), + | ^^^^^^- help: add a body after the pattern: `=> todo!(),` + +error[E0658]: `!` patterns are experimental + --> $DIR/feature-gate-never_patterns.rs:58:13 + | +LL | Err(!) if false, + | ^ + | + = note: see issue #118155 for more information + = help: add `#![feature(never_patterns)]` to the crate attributes to enable + +error: `match` arm with no body + --> $DIR/feature-gate-never_patterns.rs:58:9 + | +LL | Err(!) if false, + | ^^^^^^- help: add a body after the pattern: `=> todo!(),` + +error: `match` arm with no body + --> $DIR/feature-gate-never_patterns.rs:69:9 + | +LL | Some(_) + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` + +error: `match` arm with no body + --> $DIR/feature-gate-never_patterns.rs:75:9 + | +LL | Some(_) if false + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` + +error: a guard on a never pattern will never be run + --> $DIR/feature-gate-never_patterns.rs:58:19 + | +LL | Err(!) if false, + | ^^^^^ help: remove this guard + +error: aborting due to 18 previous errors Some errors have detailed explanations: E0408, E0658. For more information about an error, try `rustc --explain E0408`. diff --git a/tests/ui/parser/match-arm-without-body.rs b/tests/ui/parser/match-arm-without-body.rs index c3487c2c658ba..4723abff8b6ae 100644 --- a/tests/ui/parser/match-arm-without-body.rs +++ b/tests/ui/parser/match-arm-without-body.rs @@ -66,8 +66,6 @@ fn main() { pat!() //~^ ERROR expected `,` following `match` arm //~| HELP missing a comma here - //~| ERROR `match` arm with no body - //~| HELP add a body after the pattern _ => {} } match Some(false) { diff --git a/tests/ui/parser/match-arm-without-body.stderr b/tests/ui/parser/match-arm-without-body.stderr index 3a06ed050b562..d98c7ec282601 100644 --- a/tests/ui/parser/match-arm-without-body.stderr +++ b/tests/ui/parser/match-arm-without-body.stderr @@ -68,19 +68,19 @@ error: `match` arm with no body --> $DIR/match-arm-without-body.rs:30:9 | LL | Some(_) if true - | ^^^^^^^^^^^^^^^- help: add a body after the pattern: `=> todo!(),` + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error: `match` arm with no body --> $DIR/match-arm-without-body.rs:40:9 | LL | Some(_) if true, - | ^^^^^^^^^^^^^^^- help: add a body after the pattern: `=> todo!(),` + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error: `match` arm with no body --> $DIR/match-arm-without-body.rs:45:9 | LL | Some(_) if true, - | ^^^^^^^^^^^^^^^- help: add a body after the pattern: `=> todo!(),` + | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error: `match` arm with no body --> $DIR/match-arm-without-body.rs:51:9 @@ -98,19 +98,13 @@ error: `match` arm with no body --> $DIR/match-arm-without-body.rs:61:9 | LL | pat!() if true, - | ^^^^^^^^^^^^^^- help: add a body after the pattern: `=> todo!(),` - -error: `match` arm with no body - --> $DIR/match-arm-without-body.rs:66:9 - | -LL | pat!() | ^^^^^^- help: add a body after the pattern: `=> todo!(),` error: `match` arm with no body - --> $DIR/match-arm-without-body.rs:74:9 + --> $DIR/match-arm-without-body.rs:72:9 | LL | pat!(), | ^^^^^^- help: add a body after the pattern: `=> todo!(),` -error: aborting due to 14 previous errors +error: aborting due to 13 previous errors From 19e0c984d3ff44c6e273ddae2f327e8ad8726fae Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 12 Dec 2023 14:52:05 +0100 Subject: [PATCH 17/18] Don't gate the feature twice --- compiler/rustc_ast/src/ast.rs | 13 +++++ compiler/rustc_parse/src/parser/expr.rs | 5 +- .../feature-gate-never_patterns.rs | 8 +-- .../feature-gate-never_patterns.stderr | 56 ++++--------------- 4 files changed, 31 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 190fae9565210..5755ae8a8bc47 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -676,6 +676,19 @@ impl Pat { }); could_be_never_pattern } + + /// Whether this contains a `!` pattern. This in particular means that a feature gate error will + /// be raised if the feature is off. Used to avoid gating the feature twice. + pub fn contains_never_pattern(&self) -> bool { + let mut contains_never_pattern = false; + self.walk(&mut |pat| { + if matches!(pat.kind, PatKind::Never) { + contains_never_pattern = true; + } + true + }); + contains_never_pattern + } } /// A single field in a struct pattern. diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index bdaf8db131160..5b0011e9f706c 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -2920,7 +2920,10 @@ impl<'a> Parser<'a> { arm_body = None; this.expect_one_of(&[token::Comma], &[token::CloseDelim(Delimiter::Brace)]).map( |x| { - this.sess.gated_spans.gate(sym::never_patterns, pat.span); + // Don't gate twice + if !pat.contains_never_pattern() { + this.sess.gated_spans.gate(sym::never_patterns, pat.span); + } x }, ) diff --git a/tests/ui/feature-gates/feature-gate-never_patterns.rs b/tests/ui/feature-gates/feature-gate-never_patterns.rs index 97c68e460ea70..f3910622313d5 100644 --- a/tests/ui/feature-gates/feature-gate-never_patterns.rs +++ b/tests/ui/feature-gates/feature-gate-never_patterns.rs @@ -14,14 +14,12 @@ fn main() { match *ptr { ! //~^ ERROR `!` patterns are experimental - //~| ERROR `!` patterns are experimental } // Check that the gate operates even behind `cfg`. #[cfg(FALSE)] match *ptr { ! //~^ ERROR `!` patterns are experimental - //~| ERROR `!` patterns are experimental } #[cfg(FALSE)] match *ptr { @@ -51,14 +49,12 @@ fn main() { match res { Ok(_) => {} Err(!), - //~^ ERROR `match` arm with no body - //~| ERROR `!` patterns are experimental + //~^ ERROR `!` patterns are experimental } match res { Err(!) if false, - //~^ ERROR `match` arm with no body + //~^ ERROR `!` patterns are experimental //~| ERROR a guard on a never pattern will never be run - //~| ERROR `!` patterns are experimental _ => {} } diff --git a/tests/ui/feature-gates/feature-gate-never_patterns.stderr b/tests/ui/feature-gates/feature-gate-never_patterns.stderr index 4ba80603d9f1c..dd10829d49594 100644 --- a/tests/ui/feature-gates/feature-gate-never_patterns.stderr +++ b/tests/ui/feature-gates/feature-gate-never_patterns.stderr @@ -1,5 +1,5 @@ error: unexpected `,` in pattern - --> $DIR/feature-gate-never_patterns.rs:36:16 + --> $DIR/feature-gate-never_patterns.rs:34:16 | LL | Some(_), | ^ @@ -40,17 +40,7 @@ LL | ! = help: add `#![feature(never_patterns)]` to the crate attributes to enable error[E0658]: `!` patterns are experimental - --> $DIR/feature-gate-never_patterns.rs:15:13 - | -LL | ! - | ^ - | - = note: see issue #118155 for more information - = help: add `#![feature(never_patterns)]` to the crate attributes to enable - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error[E0658]: `!` patterns are experimental - --> $DIR/feature-gate-never_patterns.rs:22:13 + --> $DIR/feature-gate-never_patterns.rs:21:13 | LL | ! | ^ @@ -59,17 +49,7 @@ LL | ! = help: add `#![feature(never_patterns)]` to the crate attributes to enable error[E0658]: `!` patterns are experimental - --> $DIR/feature-gate-never_patterns.rs:22:13 - | -LL | ! - | ^ - | - = note: see issue #118155 for more information - = help: add `#![feature(never_patterns)]` to the crate attributes to enable - = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` - -error[E0658]: `!` patterns are experimental - --> $DIR/feature-gate-never_patterns.rs:28:13 + --> $DIR/feature-gate-never_patterns.rs:26:13 | LL | ! => {} | ^ @@ -78,25 +58,25 @@ LL | ! => {} = help: add `#![feature(never_patterns)]` to the crate attributes to enable error: `match` arm with no body - --> $DIR/feature-gate-never_patterns.rs:41:9 + --> $DIR/feature-gate-never_patterns.rs:39:9 | LL | Some(_) | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error: `match` arm with no body - --> $DIR/feature-gate-never_patterns.rs:46:9 + --> $DIR/feature-gate-never_patterns.rs:44:9 | LL | Some(_) if false, | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error: `match` arm with no body - --> $DIR/feature-gate-never_patterns.rs:48:9 + --> $DIR/feature-gate-never_patterns.rs:46:9 | LL | Some(_) if false | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error[E0658]: `!` patterns are experimental - --> $DIR/feature-gate-never_patterns.rs:53:13 + --> $DIR/feature-gate-never_patterns.rs:51:13 | LL | Err(!), | ^ @@ -104,14 +84,8 @@ LL | Err(!), = note: see issue #118155 for more information = help: add `#![feature(never_patterns)]` to the crate attributes to enable -error: `match` arm with no body - --> $DIR/feature-gate-never_patterns.rs:53:9 - | -LL | Err(!), - | ^^^^^^- help: add a body after the pattern: `=> todo!(),` - error[E0658]: `!` patterns are experimental - --> $DIR/feature-gate-never_patterns.rs:58:13 + --> $DIR/feature-gate-never_patterns.rs:55:13 | LL | Err(!) if false, | ^ @@ -120,30 +94,24 @@ LL | Err(!) if false, = help: add `#![feature(never_patterns)]` to the crate attributes to enable error: `match` arm with no body - --> $DIR/feature-gate-never_patterns.rs:58:9 - | -LL | Err(!) if false, - | ^^^^^^- help: add a body after the pattern: `=> todo!(),` - -error: `match` arm with no body - --> $DIR/feature-gate-never_patterns.rs:69:9 + --> $DIR/feature-gate-never_patterns.rs:65:9 | LL | Some(_) | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error: `match` arm with no body - --> $DIR/feature-gate-never_patterns.rs:75:9 + --> $DIR/feature-gate-never_patterns.rs:71:9 | LL | Some(_) if false | ^^^^^^^- help: add a body after the pattern: `=> todo!(),` error: a guard on a never pattern will never be run - --> $DIR/feature-gate-never_patterns.rs:58:19 + --> $DIR/feature-gate-never_patterns.rs:55:19 | LL | Err(!) if false, | ^^^^^ help: remove this guard -error: aborting due to 18 previous errors +error: aborting due to 14 previous errors Some errors have detailed explanations: E0408, E0658. For more information about an error, try `rustc --explain E0408`. From 41c940413338f5364f834e67c1008e7823d6b5ef Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Tue, 12 Dec 2023 15:48:12 +0100 Subject: [PATCH 18/18] tests: CGU tests require build-pass, not check-pass (remove FIXME) CGU tests require CGU code to be exercised. We can't merely do "cargo check" on these tests. --- tests/incremental/thinlto/cgu_invalidated_via_import.rs | 2 +- tests/incremental/thinlto/cgu_keeps_identical_fn.rs | 2 +- .../thinlto/independent_cgus_dont_affect_each_other.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/incremental/thinlto/cgu_invalidated_via_import.rs b/tests/incremental/thinlto/cgu_invalidated_via_import.rs index 5fe435d796f56..e0cd385eff3b9 100644 --- a/tests/incremental/thinlto/cgu_invalidated_via_import.rs +++ b/tests/incremental/thinlto/cgu_invalidated_via_import.rs @@ -4,7 +4,7 @@ // revisions: cfail1 cfail2 cfail3 // compile-flags: -Z query-dep-graph -O -// build-pass (FIXME(62277): could be check-pass?) +// build-pass #![feature(rustc_attrs)] #![crate_type="rlib"] diff --git a/tests/incremental/thinlto/cgu_keeps_identical_fn.rs b/tests/incremental/thinlto/cgu_keeps_identical_fn.rs index 368a726ea904d..781aae578d443 100644 --- a/tests/incremental/thinlto/cgu_keeps_identical_fn.rs +++ b/tests/incremental/thinlto/cgu_keeps_identical_fn.rs @@ -5,7 +5,7 @@ // revisions: cfail1 cfail2 cfail3 // compile-flags: -Z query-dep-graph -O -// build-pass (FIXME(62277): could be check-pass?) +// build-pass #![feature(rustc_attrs)] #![crate_type = "rlib"] diff --git a/tests/incremental/thinlto/independent_cgus_dont_affect_each_other.rs b/tests/incremental/thinlto/independent_cgus_dont_affect_each_other.rs index 045f2011958bd..8aa036ec97876 100644 --- a/tests/incremental/thinlto/independent_cgus_dont_affect_each_other.rs +++ b/tests/incremental/thinlto/independent_cgus_dont_affect_each_other.rs @@ -3,7 +3,7 @@ // revisions: cfail1 cfail2 cfail3 // compile-flags: -Z query-dep-graph -O -// build-pass (FIXME(62277): could be check-pass?) +// build-pass #![feature(rustc_attrs)] #![crate_type="rlib"]