From 949cf61fdce2309920ebfa9f7e8363308fbdac08 Mon Sep 17 00:00:00 2001 From: Yihai Lin <1161813899@qq.com> Date: Tue, 12 Nov 2024 15:53:37 +0800 Subject: [PATCH 1/8] add parentheses when unboxing suggestion needed --- .../src/fn_ctxt/suggestions.rs | 33 ++++++++--- ...boxing-needing-parenthases-issue-132924.rs | 18 ++++++ ...ng-needing-parenthases-issue-132924.stderr | 59 +++++++++++++++++++ 3 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 tests/ui/coercion/unboxing-needing-parenthases-issue-132924.rs create mode 100644 tests/ui/coercion/unboxing-needing-parenthases-issue-132924.stderr diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index 919e83724d70a..c4c4c2f200bac 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -2648,15 +2648,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } let make_sugg = |expr: &Expr<'_>, span: Span, sugg: &str| { - let needs_parens = match expr.kind { - // parenthesize if needed (Issue #46756) - hir::ExprKind::Cast(_, _) | hir::ExprKind::Binary(_, _, _) => true, - // parenthesize borrows of range literals (Issue #54505) - _ if is_range_literal(expr) => true, - _ => false, - }; - - if needs_parens { + if self.needs_parentheses(expr) { ( vec![ (span.shrink_to_lo(), format!("{prefix}{sugg}(")), @@ -2869,6 +2861,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return None; } + if self.needs_parentheses(expr) { + return Some(( + vec![ + (span, format!("{suggestion}(")), + (expr.span.shrink_to_hi(), ")".to_string()), + ], + message, + Applicability::MachineApplicable, + true, + false, + )); + } + return Some(( vec![(span, suggestion)], message, @@ -2897,6 +2902,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false } + fn needs_parentheses(&self, expr: &hir::Expr<'_>) -> bool { + match expr.kind { + // parenthesize if needed (Issue #46756) + hir::ExprKind::Cast(_, _) | hir::ExprKind::Binary(_, _, _) => true, + // parenthesize borrows of range literals (Issue #54505) + _ if is_range_literal(expr) => true, + _ => false, + } + } + pub(crate) fn suggest_cast( &self, err: &mut Diag<'_>, diff --git a/tests/ui/coercion/unboxing-needing-parenthases-issue-132924.rs b/tests/ui/coercion/unboxing-needing-parenthases-issue-132924.rs new file mode 100644 index 0000000000000..fc4258fc0af89 --- /dev/null +++ b/tests/ui/coercion/unboxing-needing-parenthases-issue-132924.rs @@ -0,0 +1,18 @@ +//@ check-fail +fn main() { + let x = Box::new(Some(1)); + + let test: Option = x; + //~^ ERROR mismatched types + let x = Box::new(Some(1)); + let test: Option = { x as Box> }; + //~^ ERROR mismatched types + + let x = Box::new(Some(1)); + let test: Option = if true { x as Box> } else { None }; + //~^ ERROR mismatched types + + let x = std::rc::Rc::new(Some(1)); + let test: Option = x as std::rc::Rc>; + //~^ ERROR mismatched types +} diff --git a/tests/ui/coercion/unboxing-needing-parenthases-issue-132924.stderr b/tests/ui/coercion/unboxing-needing-parenthases-issue-132924.stderr new file mode 100644 index 0000000000000..429b1b8735752 --- /dev/null +++ b/tests/ui/coercion/unboxing-needing-parenthases-issue-132924.stderr @@ -0,0 +1,59 @@ +error[E0308]: mismatched types + --> $DIR/unboxing-needing-parenthases-issue-132924.rs:5:29 + | +LL | let test: Option = x; + | ----------- ^ expected `Option`, found `Box>` + | | + | expected due to this + | + = note: expected enum `Option` + found struct `Box>` +help: consider unboxing the value + | +LL | let test: Option = *x; + | + + +error[E0308]: mismatched types + --> $DIR/unboxing-needing-parenthases-issue-132924.rs:8:31 + | +LL | let test: Option = { x as Box> }; + | ^^^^^^^^^^^^^^^^^^^^^ expected `Option`, found `Box>` + | + = note: expected enum `Option<_>` + found struct `Box>` +help: consider unboxing the value + | +LL | let test: Option = { *(x as Box>) }; + | ++ + + +error[E0308]: mismatched types + --> $DIR/unboxing-needing-parenthases-issue-132924.rs:12:39 + | +LL | let test: Option = if true { x as Box> } else { None }; + | ^^^^^^^^^^^^^^^^^^^^^ expected `Option`, found `Box>` + | + = note: expected enum `Option<_>` + found struct `Box>` +help: consider unboxing the value + | +LL | let test: Option = if true { *(x as Box>) } else { None }; + | ++ + + +error[E0308]: mismatched types + --> $DIR/unboxing-needing-parenthases-issue-132924.rs:16:29 + | +LL | let test: Option = x as std::rc::Rc>; + | ----------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `Option`, found `Rc>` + | | + | expected due to this + | + = note: expected enum `Option<_>` + found struct `Rc>` +help: consider dereferencing the type + | +LL | let test: Option = *(x as std::rc::Rc>); + | ++ + + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`. From 202caa7c57f635c481670037dc52735635ec2f82 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Wed, 13 Nov 2024 17:55:25 +0800 Subject: [PATCH 2/8] Add `RUSTC_BOOTSTRAP=-1` to make rustc pretend as stable compiler --- compiler/rustc_feature/src/lib.rs | 19 ++++++++++++------- compiler/rustc_feature/src/tests.rs | 12 +++++++++++- compiler/rustc_feature/src/unstable.rs | 2 +- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_feature/src/lib.rs b/compiler/rustc_feature/src/lib.rs index 9f42d3ec45cd9..5d27b8f542cbb 100644 --- a/compiler/rustc_feature/src/lib.rs +++ b/compiler/rustc_feature/src/lib.rs @@ -74,14 +74,19 @@ impl UnstableFeatures { // Returns whether `krate` should be counted as unstable let is_unstable_crate = |var: &str| krate.is_some_and(|name| var.split(',').any(|new_krate| new_krate == name)); - // `true` if we should enable unstable features for bootstrapping. - let bootstrap = - std::env::var("RUSTC_BOOTSTRAP").is_ok_and(|var| var == "1" || is_unstable_crate(&var)); - match (disable_unstable_features, bootstrap) { - (_, true) => UnstableFeatures::Cheat, - (true, _) => UnstableFeatures::Disallow, - (false, _) => UnstableFeatures::Allow, + + let bootstrap = std::env::var("RUSTC_BOOTSTRAP").ok(); + if let Some(val) = bootstrap.as_deref() { + match val { + val if val == "1" || is_unstable_crate(val) => return UnstableFeatures::Cheat, + // Hypnotize ourselves so that we think we are a stable compiler and thus don't + // allow any unstable features. + "-1" => return UnstableFeatures::Disallow, + _ => {} + } } + + if disable_unstable_features { UnstableFeatures::Disallow } else { UnstableFeatures::Allow } } pub fn is_nightly_build(&self) -> bool { diff --git a/compiler/rustc_feature/src/tests.rs b/compiler/rustc_feature/src/tests.rs index 50433e44b1350..cc0e1f3120965 100644 --- a/compiler/rustc_feature/src/tests.rs +++ b/compiler/rustc_feature/src/tests.rs @@ -18,6 +18,16 @@ fn rustc_bootstrap_parsing() { assert!(!is_bootstrap("x,y,z", Some("a"))); assert!(!is_bootstrap("x,y,z", None)); - // this is technically a breaking change, but there are no stability guarantees for RUSTC_BOOTSTRAP + // `RUSTC_BOOTSTRAP=0` is not recognized. assert!(!is_bootstrap("0", None)); + + // `RUSTC_BOOTSTRAP=-1` is force-stable, no unstable features allowed. + let is_force_stable = |krate| { + std::env::set_var("RUSTC_BOOTSTRAP", "-1"); + matches!(UnstableFeatures::from_environment(krate), UnstableFeatures::Disallow) + }; + assert!(is_force_stable(None)); + // Does not support specifying any crate. + assert!(is_force_stable(Some("x"))); + assert!(is_force_stable(Some("x,y,z"))); } diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 8326d0031ea60..a67a5776449d7 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -54,7 +54,7 @@ pub struct EnabledLangFeature { pub stable_since: Option, } -/// Information abhout an enabled library feature. +/// Information about an enabled library feature. #[derive(Debug, Copy, Clone)] pub struct EnabledLibFeature { pub gate_name: Symbol, From c130501d65f846aa26c59293c978e607010d2565 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Wed, 13 Nov 2024 17:56:21 +0800 Subject: [PATCH 3/8] Add a ui test for `RUSTC_BOOTSTRAP` vs rustc's stability --- .../rustc_bootstap.force_stable.stderr | 10 ++++ tests/ui/bootstrap/rustc_bootstap.rs | 47 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 tests/ui/bootstrap/rustc_bootstap.force_stable.stderr create mode 100644 tests/ui/bootstrap/rustc_bootstap.rs diff --git a/tests/ui/bootstrap/rustc_bootstap.force_stable.stderr b/tests/ui/bootstrap/rustc_bootstap.force_stable.stderr new file mode 100644 index 0000000000000..f378f3c70dd03 --- /dev/null +++ b/tests/ui/bootstrap/rustc_bootstap.force_stable.stderr @@ -0,0 +1,10 @@ +error: the option `Z` is only accepted on the nightly compiler + +help: consider switching to a nightly toolchain: `rustup default nightly` + +note: selecting a toolchain with `+toolchain` arguments require a rustup proxy; see + +note: for more information about Rust's stability policy, see + +error: 1 nightly option were parsed + diff --git a/tests/ui/bootstrap/rustc_bootstap.rs b/tests/ui/bootstrap/rustc_bootstap.rs new file mode 100644 index 0000000000000..3d792ef4be4e5 --- /dev/null +++ b/tests/ui/bootstrap/rustc_bootstap.rs @@ -0,0 +1,47 @@ +//! Check `RUSTC_BOOTSTRAP`'s behavior in relation to feature stability and what rustc considers +//! itself to be (stable vs non-stable ). +//! +//! `RUSTC_BOOTSTRAP` accepts: +//! +//! - `1`: cheat, allow usage of unstable features even if rustc thinks it is a stable compiler. +//! - `x,y,z`: comma-delimited list of crates. +//! - `-1`: force rustc to think it is a stable compiler. + +// ignore-tidy-linelength + +//@ revisions: default_nightly cheat cheat_single_crate cheat_multi_crate force_stable invalid_zero invalid_junk +//@ only-nightly + +//@[default_nightly] unset-rustc-env:RUSTC_BOOTSTRAP +//@[default_nightly] check-pass + +// For a nightly compiler, this is same as `default_nightly` as if `RUSTC_BOOTSTRAP` was unset. +//@[invalid_zero] rustc-env:RUSTC_BOOTSTRAP=0 +//@[invalid_zero] check-pass + +// Invalid values are silently discarded, same as `default_nightly`, i.e. as if `RUSTC_BOOTSTRAP` +// was unset. +//@[invalid_junk] rustc-env:RUSTC_BOOTSTRAP=* +//@[invalid_junk] check-pass + +//@[cheat] rustc-env:RUSTC_BOOTSTRAP=1 +//@[cheat] check-pass + +//@[cheat_single_crate] rustc-env:RUSTC_BOOTSTRAP=x +//@[cheat_single_crate] check-pass + +//@[cheat_multi_crate] rustc-env:RUSTC_BOOTSTRAP=x,y,z +//@[cheat_multi_crate] check-pass + +// Note: compiletest passes some `-Z` flags to the compiler for ui testing purposes, so here we +// instead abuse the fact that `-Z unstable-options` is also part of rustc's stability story and is +// also affected by `RUSTC_BOOTSTRAP`. +//@[force_stable] rustc-env:RUSTC_BOOTSTRAP=-1 +//@[force_stable] compile-flags: -Z unstable-options +//@[force_stable] regex-error-pattern: error: the option `Z` is only accepted on the nightly compiler + +#![crate_type = "lib"] + +// Note: `rustc_attrs` is a perma-unstable internal feature that is unlikely to change, which is +// used as a proxy to check `RUSTC_BOOTSTRAP` versus stability checking logic. +#![feature(rustc_attrs)] From fa2e214a439488e06c4a144a2b3e37409810d9f4 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sun, 17 Nov 2024 08:07:16 -0700 Subject: [PATCH 4/8] rustdoc-search: add standalone trailing `::` test Follow up for #132569 --- tests/rustdoc-js/trailing.js | 7 +++++++ tests/rustdoc-js/trailing.rs | 3 +++ 2 files changed, 10 insertions(+) create mode 100644 tests/rustdoc-js/trailing.js create mode 100644 tests/rustdoc-js/trailing.rs diff --git a/tests/rustdoc-js/trailing.js b/tests/rustdoc-js/trailing.js new file mode 100644 index 0000000000000..df5dd22ca4e10 --- /dev/null +++ b/tests/rustdoc-js/trailing.js @@ -0,0 +1,7 @@ +// exact-check +const EXPECTED = { + 'query': 'inner::', + 'others': [ + { 'path': 'trailing::inner', 'name': 'function' }, + ], +} diff --git a/tests/rustdoc-js/trailing.rs b/tests/rustdoc-js/trailing.rs new file mode 100644 index 0000000000000..5c7c9362ffb31 --- /dev/null +++ b/tests/rustdoc-js/trailing.rs @@ -0,0 +1,3 @@ +pub mod inner { + pub fn function() {} +} From 92ff69d368f63423bb85e120cfe8ce832953d1aa Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 17 Nov 2024 22:13:27 +0000 Subject: [PATCH 5/8] fixup some test directives --- tests/incremental/hygiene/load_cached_hygiene.rs | 2 +- tests/ui/symbol-names/basic.rs | 2 +- tests/ui/symbol-names/impl1.rs | 2 +- tests/ui/symbol-names/issue-60925.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/incremental/hygiene/load_cached_hygiene.rs b/tests/incremental/hygiene/load_cached_hygiene.rs index 4ad9c7d49fda9..101d280cd492e 100644 --- a/tests/incremental/hygiene/load_cached_hygiene.rs +++ b/tests/incremental/hygiene/load_cached_hygiene.rs @@ -7,7 +7,7 @@ // This causes hygiene information to be saved to the incr cache. // 2. One function is the foreign crate is modified. This causes the // optimized mir for an unmodified function to be loaded from the -//@ incremental cache and written out to the crate metadata. +// incremental cache and written out to the crate metadata. // 3. In the process of loading and writing out this function's MIR, // we load hygiene information from the incremental cache and // write it to our metadata. diff --git a/tests/ui/symbol-names/basic.rs b/tests/ui/symbol-names/basic.rs index dfcac21ccd6e2..839dda2b3a3be 100644 --- a/tests/ui/symbol-names/basic.rs +++ b/tests/ui/symbol-names/basic.rs @@ -1,7 +1,7 @@ //@ build-fail //@ revisions: legacy v0 //@[legacy]compile-flags: -Z unstable-options -C symbol-mangling-version=legacy - //@[v0]compile-flags: -C symbol-mangling-version=v0 +//@[v0]compile-flags: -C symbol-mangling-version=v0 #![feature(rustc_attrs)] diff --git a/tests/ui/symbol-names/impl1.rs b/tests/ui/symbol-names/impl1.rs index fa4be88f68ffa..9aefca47447f4 100644 --- a/tests/ui/symbol-names/impl1.rs +++ b/tests/ui/symbol-names/impl1.rs @@ -1,7 +1,7 @@ //@ build-fail //@ revisions: legacy v0 //@[legacy]compile-flags: -Z unstable-options -C symbol-mangling-version=legacy - //@[v0]compile-flags: -C symbol-mangling-version=v0 +//@[v0]compile-flags: -C symbol-mangling-version=v0 //@[legacy]normalize-stderr-test: "h[\w]{16}E?\)" -> ")" #![feature(auto_traits, rustc_attrs)] diff --git a/tests/ui/symbol-names/issue-60925.rs b/tests/ui/symbol-names/issue-60925.rs index 9f1f007a0facf..ca0f21b7a781d 100644 --- a/tests/ui/symbol-names/issue-60925.rs +++ b/tests/ui/symbol-names/issue-60925.rs @@ -1,7 +1,7 @@ //@ build-fail //@ revisions: legacy v0 //@[legacy]compile-flags: -Z unstable-options -C symbol-mangling-version=legacy - //@[v0]compile-flags: -C symbol-mangling-version=v0 +//@[v0]compile-flags: -C symbol-mangling-version=v0 #![feature(rustc_attrs)] From 7765f23ea14d16fb0182b475a0ec96891058ae24 Mon Sep 17 00:00:00 2001 From: Kornel Date: Sun, 17 Nov 2024 21:43:55 +0000 Subject: [PATCH 6/8] Diagnostics for let mut in item context --- compiler/rustc_parse/src/parser/item.rs | 33 ++++++++++++++----- .../suggest-const-for-global-var.stderr | 7 +++- .../suggest-static-for-global-var-mut.rs | 5 +++ .../suggest-static-for-global-var-mut.stderr | 11 +++++++ 4 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 tests/ui/parser/suggest-static-for-global-var-mut.rs create mode 100644 tests/ui/parser/suggest-static-for-global-var-mut.stderr diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 6b4e2d0f4e2f5..fddbf5896ad83 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -77,18 +77,35 @@ impl<'a> Parser<'a> { if !self.eat(term) { let token_str = super::token_descr(&self.token); if !self.maybe_consume_incorrect_semicolon(items.last().map(|x| &**x)) { + let is_let = self.token.is_keyword(kw::Let); + let is_let_mut = is_let && self.look_ahead(1, |t| t.is_keyword(kw::Mut)); + let let_has_ident = is_let && !is_let_mut && self.is_kw_followed_by_ident(kw::Let); + let msg = format!("expected item, found {token_str}"); let mut err = self.dcx().struct_span_err(self.token.span, msg); - let span = self.token.span; - if self.is_kw_followed_by_ident(kw::Let) { - err.span_label( - span, - "consider using `const` or `static` instead of `let` for global variables", - ); + + let label = if is_let { + "`let` cannot be used for global variables" } else { - err.span_label(span, "expected item") - .note("for a full list of items that can appear in modules, see "); + "expected item" }; + err.span_label(self.token.span, label); + + if is_let { + if is_let_mut { + err.help("consider using `static` and a `Mutex` instead of `let mut`"); + } else if let_has_ident { + err.span_suggestion_short( + self.token.span, + "consider using `static` or `const` instead of `let`", + "static", + Applicability::MaybeIncorrect, + ); + } else { + err.help("consider using `static` or `const` instead of `let`"); + } + } + err.note("for a full list of items that can appear in modules, see "); return Err(err); } } diff --git a/tests/ui/parser/suggest-const-for-global-var.stderr b/tests/ui/parser/suggest-const-for-global-var.stderr index 235e621d8827b..6ac7fe8f0921b 100644 --- a/tests/ui/parser/suggest-const-for-global-var.stderr +++ b/tests/ui/parser/suggest-const-for-global-var.stderr @@ -2,7 +2,12 @@ error: expected item, found keyword `let` --> $DIR/suggest-const-for-global-var.rs:1:1 | LL | let X: i32 = 12; - | ^^^ consider using `const` or `static` instead of `let` for global variables + | ^^^ + | | + | `let` cannot be used for global variables + | help: consider using `static` or `const` instead of `let` + | + = note: for a full list of items that can appear in modules, see error: aborting due to 1 previous error diff --git a/tests/ui/parser/suggest-static-for-global-var-mut.rs b/tests/ui/parser/suggest-static-for-global-var-mut.rs new file mode 100644 index 0000000000000..c63b09bb7a7f7 --- /dev/null +++ b/tests/ui/parser/suggest-static-for-global-var-mut.rs @@ -0,0 +1,5 @@ +let mut _data = vec![1,2,3]; +//~^ ERROR expected item, found keyword `let` + +fn main() { +} diff --git a/tests/ui/parser/suggest-static-for-global-var-mut.stderr b/tests/ui/parser/suggest-static-for-global-var-mut.stderr new file mode 100644 index 0000000000000..4b00d1a24f317 --- /dev/null +++ b/tests/ui/parser/suggest-static-for-global-var-mut.stderr @@ -0,0 +1,11 @@ +error: expected item, found keyword `let` + --> $DIR/suggest-static-for-global-var-mut.rs:1:1 + | +LL | let mut _data = vec![1,2,3]; + | ^^^ `let` cannot be used for global variables + | + = help: consider using `static` and a `Mutex` instead of `let mut` + = note: for a full list of items that can appear in modules, see + +error: aborting due to 1 previous error + From 32d2340dbd4e9e724839c5ee8c6e73474660fe89 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 9 Nov 2024 00:59:35 +0000 Subject: [PATCH 7/8] Check use<..> in RPITIT for refinement --- compiler/rustc_hir_analysis/messages.ftl | 5 + .../src/check/compare_impl_item/refine.rs | 102 +++++++++++++++++- compiler/rustc_hir_analysis/src/errors.rs | 10 ++ .../ui/impl-trait/in-trait/refine-captures.rs | 36 +++++++ .../in-trait/refine-captures.stderr | 52 +++++++++ 5 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 tests/ui/impl-trait/in-trait/refine-captures.rs create mode 100644 tests/ui/impl-trait/in-trait/refine-captures.stderr diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 6e8ba51612ec4..64a30e633cf9d 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -448,6 +448,11 @@ hir_analysis_rpitit_refined = impl trait in impl method signature does not match .note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate .feedback_note = we are soliciting feedback, see issue #121718 for more information +hir_analysis_rpitit_refined_lifetimes = impl trait in impl method captures fewer lifetimes than in trait + .suggestion = modify the `use<..>` bound to capture the same lifetimes that the trait does + .note = add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate + .feedback_note = we are soliciting feedback, see issue #121718 for more information + hir_analysis_self_in_impl_self = `Self` is not valid in the self type of an impl block .note = replace `Self` with a different type diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs index 646c104f1f553..25ba52b4d7bf7 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs @@ -1,6 +1,7 @@ +use itertools::Itertools as _; use rustc_data_structures::fx::FxIndexSet; use rustc_hir as hir; -use rustc_hir::def_id::DefId; +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_infer::infer::TyCtxtInferExt; use rustc_infer::infer::outlives::env::OutlivesEnvironment; use rustc_lint_defs::builtin::{REFINING_IMPL_TRAIT_INTERNAL, REFINING_IMPL_TRAIT_REACHABLE}; @@ -75,6 +76,8 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( let mut trait_bounds = vec![]; // Bounds that we find on the RPITITs in the impl signature. let mut impl_bounds = vec![]; + // Pairs of trait and impl opaques. + let mut pairs = vec![]; for trait_projection in collector.types.into_iter().rev() { let impl_opaque_args = trait_projection.args.rebase_onto(tcx, trait_m.def_id, impl_m_args); @@ -121,6 +124,8 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( tcx.explicit_item_bounds(impl_opaque.def_id) .iter_instantiated_copied(tcx, impl_opaque.args), )); + + pairs.push((trait_projection, impl_opaque)); } let hybrid_preds = tcx @@ -212,6 +217,39 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( return; } } + + // Make sure that the RPITIT doesn't capture fewer regions than + // the trait definition. We hard-error if it captures *more*, since that + // is literally unrepresentable in the type system; however, we may be + // promising stronger outlives guarantees if we capture *fewer* regions. + for (trait_projection, impl_opaque) in pairs { + let impl_variances = tcx.variances_of(impl_opaque.def_id); + let impl_captures: FxIndexSet<_> = impl_opaque + .args + .iter() + .zip_eq(impl_variances) + .filter(|(_, v)| **v == ty::Invariant) + .map(|(arg, _)| arg) + .collect(); + + let trait_variances = tcx.variances_of(trait_projection.def_id); + let mut trait_captures = FxIndexSet::default(); + for (arg, variance) in trait_projection.args.iter().zip_eq(trait_variances) { + if *variance != ty::Invariant { + continue; + } + arg.visit_with(&mut CollectParams { params: &mut trait_captures }); + } + + if !trait_captures.iter().all(|arg| impl_captures.contains(arg)) { + report_mismatched_rpitit_captures( + tcx, + impl_opaque.def_id.expect_local(), + trait_captures, + is_internal, + ); + } + } } struct ImplTraitInTraitCollector<'tcx> { @@ -342,3 +380,65 @@ impl<'tcx> TypeFolder> for Anonymize<'tcx> { self.tcx.anonymize_bound_vars(t) } } + +struct CollectParams<'a, 'tcx> { + params: &'a mut FxIndexSet>, +} +impl<'tcx> TypeVisitor> for CollectParams<'_, 'tcx> { + fn visit_ty(&mut self, ty: Ty<'tcx>) { + if let ty::Param(_) = ty.kind() { + self.params.insert(ty.into()); + } else { + ty.super_visit_with(self); + } + } + fn visit_region(&mut self, r: ty::Region<'tcx>) { + match r.kind() { + ty::ReEarlyParam(_) | ty::ReLateParam(_) => { + self.params.insert(r.into()); + } + _ => {} + } + } + fn visit_const(&mut self, ct: ty::Const<'tcx>) { + if let ty::ConstKind::Param(_) = ct.kind() { + self.params.insert(ct.into()); + } else { + ct.super_visit_with(self); + } + } +} + +fn report_mismatched_rpitit_captures<'tcx>( + tcx: TyCtxt<'tcx>, + impl_opaque_def_id: LocalDefId, + mut trait_captured_args: FxIndexSet>, + is_internal: bool, +) { + let Some(use_bound_span) = + tcx.hir_node_by_def_id(impl_opaque_def_id).expect_opaque_ty().bounds.iter().find_map( + |bound| match *bound { + rustc_hir::GenericBound::Use(_, span) => Some(span), + hir::GenericBound::Trait(_) | hir::GenericBound::Outlives(_) => None, + }, + ) + else { + // I have no idea when you would ever undercapture without a `use<..>`. + tcx.dcx().delayed_bug("expected use<..> to undercapture in an impl opaque"); + return; + }; + + trait_captured_args + .sort_by_cached_key(|arg| !matches!(arg.unpack(), ty::GenericArgKind::Lifetime(_))); + let suggestion = format!("use<{}>", trait_captured_args.iter().join(", ")); + + tcx.emit_node_span_lint( + if is_internal { REFINING_IMPL_TRAIT_INTERNAL } else { REFINING_IMPL_TRAIT_REACHABLE }, + tcx.local_def_id_to_hir_id(impl_opaque_def_id), + use_bound_span, + crate::errors::ReturnPositionImplTraitInTraitRefinedLifetimes { + suggestion_span: use_bound_span, + suggestion, + }, + ); +} diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index a92a5e4278c56..07d3273b09c77 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1153,6 +1153,16 @@ pub(crate) struct ReturnPositionImplTraitInTraitRefined<'tcx> { pub return_ty: Ty<'tcx>, } +#[derive(LintDiagnostic)] +#[diag(hir_analysis_rpitit_refined_lifetimes)] +#[note] +#[note(hir_analysis_feedback_note)] +pub(crate) struct ReturnPositionImplTraitInTraitRefinedLifetimes { + #[suggestion(applicability = "maybe-incorrect", code = "{suggestion}")] + pub suggestion_span: Span, + pub suggestion: String, +} + #[derive(Diagnostic)] #[diag(hir_analysis_inherent_ty_outside, code = E0390)] #[help] diff --git a/tests/ui/impl-trait/in-trait/refine-captures.rs b/tests/ui/impl-trait/in-trait/refine-captures.rs new file mode 100644 index 0000000000000..e7dffcb52aae3 --- /dev/null +++ b/tests/ui/impl-trait/in-trait/refine-captures.rs @@ -0,0 +1,36 @@ +#![feature(precise_capturing_in_traits)] + +trait LifetimeParam<'a> { + fn test() -> impl Sized; +} +// Refining via capturing fewer lifetimes than the trait definition. +impl<'a> LifetimeParam<'a> for i32 { + fn test() -> impl Sized + use<> {} + //~^ WARN impl trait in impl method captures fewer lifetimes than in trait +} +// If the lifetime is substituted, then we don't refine anything. +impl LifetimeParam<'static> for u32 { + fn test() -> impl Sized + use<> {} + // Ok +} + +trait TypeParam { + fn test() -> impl Sized; +} +// Indirectly capturing a lifetime param through a type param substitution. +impl<'a> TypeParam<&'a ()> for i32 { + fn test() -> impl Sized + use<> {} + //~^ WARN impl trait in impl method captures fewer lifetimes than in trait +} +// Two of them, but only one is captured... +impl<'a, 'b> TypeParam<(&'a (), &'b ())> for u32 { + fn test() -> impl Sized + use<'b> {} + //~^ WARN impl trait in impl method captures fewer lifetimes than in trait +} +// What if we don't capture a type param? That should be an error otherwise. +impl TypeParam for u64 { + fn test() -> impl Sized + use<> {} + //~^ ERROR `impl Trait` must mention all type parameters in scope in `use<...>` +} + +fn main() {} diff --git a/tests/ui/impl-trait/in-trait/refine-captures.stderr b/tests/ui/impl-trait/in-trait/refine-captures.stderr new file mode 100644 index 0000000000000..ad2c2a11601db --- /dev/null +++ b/tests/ui/impl-trait/in-trait/refine-captures.stderr @@ -0,0 +1,52 @@ +warning: impl trait in impl method captures fewer lifetimes than in trait + --> $DIR/refine-captures.rs:8:31 + | +LL | fn test() -> impl Sized + use<> {} + | ^^^^^ + | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate + = note: we are soliciting feedback, see issue #121718 for more information + = note: `#[warn(refining_impl_trait_internal)]` on by default +help: modify the `use<..>` bound to capture the same lifetimes that the trait does + | +LL | fn test() -> impl Sized + use<'a> {} + | ~~~~~~~ + +warning: impl trait in impl method captures fewer lifetimes than in trait + --> $DIR/refine-captures.rs:22:31 + | +LL | fn test() -> impl Sized + use<> {} + | ^^^^^ + | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate + = note: we are soliciting feedback, see issue #121718 for more information +help: modify the `use<..>` bound to capture the same lifetimes that the trait does + | +LL | fn test() -> impl Sized + use<'a> {} + | ~~~~~~~ + +warning: impl trait in impl method captures fewer lifetimes than in trait + --> $DIR/refine-captures.rs:27:31 + | +LL | fn test() -> impl Sized + use<'b> {} + | ^^^^^^^ + | + = note: add `#[allow(refining_impl_trait)]` if it is intended for this to be part of the public API of this crate + = note: we are soliciting feedback, see issue #121718 for more information +help: modify the `use<..>` bound to capture the same lifetimes that the trait does + | +LL | fn test() -> impl Sized + use<'a, 'b> {} + | ~~~~~~~~~~~ + +error: `impl Trait` must mention all type parameters in scope in `use<...>` + --> $DIR/refine-captures.rs:32:18 + | +LL | impl TypeParam for u64 { + | - type parameter is implicitly captured by this `impl Trait` +LL | fn test() -> impl Sized + use<> {} + | ^^^^^^^^^^^^^^^^^^ + | + = note: currently, all type parameters are required to be mentioned in the precise captures list + +error: aborting due to 1 previous error; 3 warnings emitted + From 546ba3d310ae2d795b43188ce1b9980b33e690e5 Mon Sep 17 00:00:00 2001 From: dianne Date: Sun, 17 Nov 2024 00:11:34 -0800 Subject: [PATCH 8/8] `suggest_borrow_generic_arg`: instantiate clauses properly Fixes issue 133118. This also modifies `tests/ui/moves/moved-value-on-as-ref-arg.rs` to have more useful bounds on the tests for suggestions to borrow `Borrow` and `BorrowMut` arguments. With its old tautological `T: BorrowMut` bound, this fix would make it suggest a shared borrow for that argument. --- .../src/diagnostics/conflict_errors.rs | 26 +++++++------------ .../ui/moves/moved-value-on-as-ref-arg.fixed | 4 +-- tests/ui/moves/moved-value-on-as-ref-arg.rs | 4 +-- .../region-var-in-moved-ty-issue-133118.rs | 25 ++++++++++++++++++ ...region-var-in-moved-ty-issue-133118.stderr | 21 +++++++++++++++ 5 files changed, 60 insertions(+), 20 deletions(-) create mode 100644 tests/ui/moves/region-var-in-moved-ty-issue-133118.rs create mode 100644 tests/ui/moves/region-var-in-moved-ty-issue-133118.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 454fd14ea7480..51c2282422e9c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -27,7 +27,7 @@ use rustc_middle::mir::{ }; use rustc_middle::ty::print::PrintTraitRefExt as _; use rustc_middle::ty::{ - self, ClauseKind, PredicateKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, Upcast, + self, PredicateKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, Upcast, suggest_constraining_type_params, }; use rustc_middle::util::CallKind; @@ -649,11 +649,11 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { ) -> Option { let tcx = self.infcx.tcx; let sig = tcx.fn_sig(callee_did).instantiate_identity().skip_binder(); - let clauses = tcx.predicates_of(callee_did).instantiate_identity(self.infcx.tcx).predicates; + let clauses = tcx.predicates_of(callee_did); // First, is there at least one method on one of `param`'s trait bounds? // This keeps us from suggesting borrowing the argument to `mem::drop`, e.g. - if !clauses.iter().any(|clause| { + if !clauses.instantiate_identity(tcx).predicates.iter().any(|clause| { clause.as_trait_clause().is_some_and(|tc| { tc.self_ty().skip_binder().is_param(param.index) && tc.polarity() == ty::PredicatePolarity::Positive @@ -700,23 +700,17 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { return false; } - // Test the callee's predicates, substituting a reference in for the self ty - // in bounds on `param`. - clauses.iter().all(|&clause| { - let clause_for_ref = clause.kind().map_bound(|kind| match kind { - ClauseKind::Trait(c) if c.self_ty().is_param(param.index) => { - ClauseKind::Trait(c.with_self_ty(tcx, ref_ty)) - } - ClauseKind::Projection(c) if c.self_ty().is_param(param.index) => { - ClauseKind::Projection(c.with_self_ty(tcx, ref_ty)) - } - _ => kind, - }); + // Test the callee's predicates, substituting in `ref_ty` for the moved argument type. + clauses.instantiate(tcx, new_args).predicates.iter().all(|&(mut clause)| { + // Normalize before testing to see through type aliases and projections. + if let Ok(normalized) = tcx.try_normalize_erasing_regions(self.param_env, clause) { + clause = normalized; + } self.infcx.predicate_must_hold_modulo_regions(&Obligation::new( tcx, ObligationCause::dummy(), self.param_env, - ty::EarlyBinder::bind(clause_for_ref).instantiate(tcx, generic_args), + clause, )) }) }) { diff --git a/tests/ui/moves/moved-value-on-as-ref-arg.fixed b/tests/ui/moves/moved-value-on-as-ref-arg.fixed index 292fa98a3f71b..97bfe094ce54f 100644 --- a/tests/ui/moves/moved-value-on-as-ref-arg.fixed +++ b/tests/ui/moves/moved-value-on-as-ref-arg.fixed @@ -18,8 +18,8 @@ impl AsMut for Bar { fn foo>(_: T) {} fn qux>(_: T) {} -fn bat>(_: T) {} -fn baz>(_: T) {} +fn bat>(_: T) {} +fn baz>(_: T) {} pub fn main() { let bar = Bar; diff --git a/tests/ui/moves/moved-value-on-as-ref-arg.rs b/tests/ui/moves/moved-value-on-as-ref-arg.rs index 632af9efcda3e..fed41cf710d2a 100644 --- a/tests/ui/moves/moved-value-on-as-ref-arg.rs +++ b/tests/ui/moves/moved-value-on-as-ref-arg.rs @@ -18,8 +18,8 @@ impl AsMut for Bar { fn foo>(_: T) {} fn qux>(_: T) {} -fn bat>(_: T) {} -fn baz>(_: T) {} +fn bat>(_: T) {} +fn baz>(_: T) {} pub fn main() { let bar = Bar; diff --git a/tests/ui/moves/region-var-in-moved-ty-issue-133118.rs b/tests/ui/moves/region-var-in-moved-ty-issue-133118.rs new file mode 100644 index 0000000000000..a49370e315dd4 --- /dev/null +++ b/tests/ui/moves/region-var-in-moved-ty-issue-133118.rs @@ -0,0 +1,25 @@ +//! regression test for #133118 + +pub trait Alpha { + fn y(self) -> usize; +} + +pub trait Beta { + type Gamma; + fn gamma(&self) -> Self::Gamma; +} + +pub fn a(_x: T) -> usize { + todo!(); +} + +pub fn x(beta: &B) -> usize +where + for<'a> &'a B: Beta, + for<'a> <&'a B as Beta>::Gamma: Alpha, +{ + let g1 = beta.gamma(); + a(g1) + a(g1) //~ ERROR use of moved value: `g1` [E0382] +} + +pub fn main() {} diff --git a/tests/ui/moves/region-var-in-moved-ty-issue-133118.stderr b/tests/ui/moves/region-var-in-moved-ty-issue-133118.stderr new file mode 100644 index 0000000000000..691625d042da7 --- /dev/null +++ b/tests/ui/moves/region-var-in-moved-ty-issue-133118.stderr @@ -0,0 +1,21 @@ +error[E0382]: use of moved value: `g1` + --> $DIR/region-var-in-moved-ty-issue-133118.rs:22:15 + | +LL | let g1 = beta.gamma(); + | -- move occurs because `g1` has type `<&B as Beta>::Gamma`, which does not implement the `Copy` trait +LL | a(g1) + a(g1) + | -- ^^ value used here after move + | | + | value moved here + | +note: consider changing this parameter type in function `a` to borrow instead if owning the value isn't necessary + --> $DIR/region-var-in-moved-ty-issue-133118.rs:12:24 + | +LL | pub fn a(_x: T) -> usize { + | - ^ this parameter takes ownership of the value + | | + | in this function + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0382`.