From 50a80970d3e18312d48dc8becca39ceb2abbe87e Mon Sep 17 00:00:00 2001 From: Alex Chi Date: Sun, 12 Mar 2023 21:10:11 -0400 Subject: [PATCH 01/18] suggest lifetime for closure parameter type when mismatch --- .../src/infer/error_reporting/mod.rs | 1 + .../src/infer/error_reporting/suggest.rs | 58 ++++++++++++++++++- tests/ui/lifetimes/issue-79187-2.stderr | 4 ++ tests/ui/lifetimes/issue-79187.stderr | 4 ++ tests/ui/mismatched_types/closure-mismatch.rs | 3 + .../mismatched_types/closure-mismatch.stderr | 38 +++++++++++- 6 files changed, 106 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index de1a2e6a577bf..18530acb5b4f9 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -1883,6 +1883,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { { let span = self.tcx.def_span(def_id); diag.span_note(span, "this closure does not fulfill the lifetime requirements"); + self.suggest_for_all_lifetime_closure(span, &exp_found, diag); } // It reads better to have the error origin as the final diff --git a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs index 55dcfd05e0ad3..9cb7caebca43a 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs @@ -8,7 +8,7 @@ use rustc_middle::traits::{ StatementAsExpression, }; use rustc_middle::ty::print::with_no_trimmed_paths; -use rustc_middle::ty::{self as ty, IsSuggestable, Ty, TypeVisitableExt}; +use rustc_middle::ty::{self as ty, GenericArgKind, IsSuggestable, Ty, TypeVisitableExt}; use rustc_span::{sym, BytePos, Span}; use crate::errors::{ @@ -553,6 +553,62 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } } + + /// For "one type is more general than the other" errors on closures, suggest changing the lifetime + /// of the parameters to accept all lifetimes. + pub(super) fn suggest_for_all_lifetime_closure( + &self, + span: Span, + exp_found: &ty::error::ExpectedFound>, + diag: &mut Diagnostic, + ) { + // 1. Get the substs of the closure. + // 2. Assume exp_found is FnOnce / FnMut / Fn, we can extract function parameters from [1]. + let expected = exp_found.expected.map_bound(|x| x.substs.get(1).cloned()).transpose(); + let found = exp_found.found.map_bound(|x| x.substs.get(1).cloned()).transpose(); + + // 3. Extract the tuple type from Fn trait and suggest the change. + if let (Some(expected), Some(found)) = (expected, found) { + let expected = expected.skip_binder().unpack(); + let found = found.skip_binder().unpack(); + if let (GenericArgKind::Type(expected), GenericArgKind::Type(found)) = (expected, found) + && let (ty::Tuple(expected), ty::Tuple(found)) = (expected.kind(), found.kind()) + && expected.len() == found.len() { + let mut suggestion = "|".to_string(); + let mut is_first = true; + let mut has_suggestion = false; + + for (expected, found) in expected.iter().zip(found.iter()) { + if is_first { + is_first = true; + } else { + suggestion += ", "; + } + + if let (ty::Ref(expected_region, _, _), ty::Ref(found_region, _, _)) = (expected.kind(), found.kind()) + && expected_region.is_late_bound() && !found_region.is_late_bound() { + // If the expected region is late bound, and the found region is not, we can suggest adding `: &_`. + // FIXME: use the actual type + variable name provided by user instead of `_`. + suggestion += "_: &_"; + has_suggestion = true; + } else { + // Otherwise, keep it as-is. + suggestion += "_"; + } + } + suggestion += "|"; + + if has_suggestion { + diag.span_suggestion_verbose( + span, + "consider changing the type of the closure parameters", + suggestion, + Applicability::MaybeIncorrect, + ); + } + } + } + } } impl<'tcx> TypeErrCtxt<'_, 'tcx> { diff --git a/tests/ui/lifetimes/issue-79187-2.stderr b/tests/ui/lifetimes/issue-79187-2.stderr index c5f654b37bf6f..c14d3e0c61b8c 100644 --- a/tests/ui/lifetimes/issue-79187-2.stderr +++ b/tests/ui/lifetimes/issue-79187-2.stderr @@ -43,6 +43,10 @@ note: the lifetime requirement is introduced here | LL | fn take_foo(_: impl Foo) {} | ^^^ +help: consider changing the type of the closure parameters + | +LL | take_foo(|_: &_| a); + | ~~~~~~~ error[E0308]: mismatched types --> $DIR/issue-79187-2.rs:11:5 diff --git a/tests/ui/lifetimes/issue-79187.stderr b/tests/ui/lifetimes/issue-79187.stderr index ee6e7b89d5f0e..16eaf654cf3fc 100644 --- a/tests/ui/lifetimes/issue-79187.stderr +++ b/tests/ui/lifetimes/issue-79187.stderr @@ -16,6 +16,10 @@ note: the lifetime requirement is introduced here | LL | fn thing(x: impl FnOnce(&u32)) {} | ^^^^^^^^^^^^ +help: consider changing the type of the closure parameters + | +LL | let f = |_: &_| (); + | ~~~~~~~ error: implementation of `FnOnce` is not general enough --> $DIR/issue-79187.rs:5:5 diff --git a/tests/ui/mismatched_types/closure-mismatch.rs b/tests/ui/mismatched_types/closure-mismatch.rs index b0644e79611fa..4eb33497c3956 100644 --- a/tests/ui/mismatched_types/closure-mismatch.rs +++ b/tests/ui/mismatched_types/closure-mismatch.rs @@ -8,4 +8,7 @@ fn main() { baz(|_| ()); //~^ ERROR implementation of `FnOnce` is not general enough //~| ERROR mismatched types + baz(|x| ()); + //~^ ERROR implementation of `FnOnce` is not general enough + //~| ERROR mismatched types } diff --git a/tests/ui/mismatched_types/closure-mismatch.stderr b/tests/ui/mismatched_types/closure-mismatch.stderr index a7ef8fa08923f..ab0e137418aec 100644 --- a/tests/ui/mismatched_types/closure-mismatch.stderr +++ b/tests/ui/mismatched_types/closure-mismatch.stderr @@ -25,7 +25,43 @@ note: the lifetime requirement is introduced here | LL | fn baz(_: T) {} | ^^^ +help: consider changing the type of the closure parameters + | +LL | baz(|_: &_| ()); + | ~~~~~~~ + +error: implementation of `FnOnce` is not general enough + --> $DIR/closure-mismatch.rs:11:5 + | +LL | baz(|x| ()); + | ^^^^^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&'2 ())` must implement `FnOnce<(&'1 (),)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 (),)>`, for some specific lifetime `'2` + +error[E0308]: mismatched types + --> $DIR/closure-mismatch.rs:11:5 + | +LL | baz(|x| ()); + | ^^^^^^^^^^^ one type is more general than the other + | + = note: expected trait `for<'a> Fn<(&'a (),)>` + found trait `Fn<(&(),)>` +note: this closure does not fulfill the lifetime requirements + --> $DIR/closure-mismatch.rs:11:9 + | +LL | baz(|x| ()); + | ^^^ +note: the lifetime requirement is introduced here + --> $DIR/closure-mismatch.rs:5:11 + | +LL | fn baz(_: T) {} + | ^^^ +help: consider changing the type of the closure parameters + | +LL | baz(|_: &_| ()); + | ~~~~~~~ -error: aborting due to 2 previous errors +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`. From 7b2c698d278b7409b51b249eab66da6aff5c6f8b Mon Sep 17 00:00:00 2001 From: Alex Chi Date: Fri, 17 Mar 2023 22:22:49 -0400 Subject: [PATCH 02/18] better suggestion based on hir Signed-off-by: Alex Chi --- .../src/infer/error_reporting/mod.rs | 2 +- .../src/infer/error_reporting/suggest.rs | 88 ++++++------ tests/ui/lifetimes/issue-105675.rs | 16 +++ tests/ui/lifetimes/issue-105675.stderr | 131 ++++++++++++++++++ tests/ui/lifetimes/issue-79187.stderr | 2 +- .../mismatched_types/closure-mismatch.stderr | 6 +- 6 files changed, 200 insertions(+), 45 deletions(-) create mode 100644 tests/ui/lifetimes/issue-105675.rs create mode 100644 tests/ui/lifetimes/issue-105675.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 18530acb5b4f9..256df4043b5fe 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -1883,7 +1883,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { { let span = self.tcx.def_span(def_id); diag.span_note(span, "this closure does not fulfill the lifetime requirements"); - self.suggest_for_all_lifetime_closure(span, &exp_found, diag); + self.suggest_for_all_lifetime_closure(span, self.tcx.hir().get_by_def_id(def_id), &exp_found, diag); } // It reads better to have the error origin as the final diff --git a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs index 9cb7caebca43a..9a74b23b6bde6 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs @@ -559,54 +559,62 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { pub(super) fn suggest_for_all_lifetime_closure( &self, span: Span, + hir: hir::Node<'_>, exp_found: &ty::error::ExpectedFound>, diag: &mut Diagnostic, ) { + // 0. Extract fn_decl from hir + let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(hir::Closure { fn_decl, .. }), .. }) = hir else { return; }; + // 1. Get the substs of the closure. // 2. Assume exp_found is FnOnce / FnMut / Fn, we can extract function parameters from [1]. - let expected = exp_found.expected.map_bound(|x| x.substs.get(1).cloned()).transpose(); - let found = exp_found.found.map_bound(|x| x.substs.get(1).cloned()).transpose(); - + let Some(expected) = exp_found.expected.skip_binder().substs.get(1) else { return; }; + let Some(found) = exp_found.found.skip_binder().substs.get(1) else { return; }; + let expected = expected.unpack(); + let found = found.unpack(); // 3. Extract the tuple type from Fn trait and suggest the change. - if let (Some(expected), Some(found)) = (expected, found) { - let expected = expected.skip_binder().unpack(); - let found = found.skip_binder().unpack(); - if let (GenericArgKind::Type(expected), GenericArgKind::Type(found)) = (expected, found) - && let (ty::Tuple(expected), ty::Tuple(found)) = (expected.kind(), found.kind()) - && expected.len() == found.len() { - let mut suggestion = "|".to_string(); - let mut is_first = true; - let mut has_suggestion = false; - - for (expected, found) in expected.iter().zip(found.iter()) { - if is_first { - is_first = true; - } else { - suggestion += ", "; - } - - if let (ty::Ref(expected_region, _, _), ty::Ref(found_region, _, _)) = (expected.kind(), found.kind()) - && expected_region.is_late_bound() && !found_region.is_late_bound() { - // If the expected region is late bound, and the found region is not, we can suggest adding `: &_`. - // FIXME: use the actual type + variable name provided by user instead of `_`. - suggestion += "_: &_"; - has_suggestion = true; - } else { - // Otherwise, keep it as-is. - suggestion += "_"; - } - } - suggestion += "|"; + if let GenericArgKind::Type(expected) = expected && + let GenericArgKind::Type(found) = found && + let ty::Tuple(expected) = expected.kind() && + let ty::Tuple(found)= found.kind() && + expected.len() == found.len() { + let mut suggestion = "|".to_string(); + let mut is_first = true; + let mut has_suggestion = false; + + for ((expected, found), arg_hir) in expected.iter().zip(found.iter()).zip(fn_decl.inputs.iter()) { + if is_first { + is_first = false; + } else { + suggestion += ", "; + } - if has_suggestion { - diag.span_suggestion_verbose( - span, - "consider changing the type of the closure parameters", - suggestion, - Applicability::MaybeIncorrect, - ); - } + if let ty::Ref(expected_region, _, _) = expected.kind() && + let ty::Ref(found_region, _, _) = found.kind() && + expected_region.is_late_bound() && + !found_region.is_late_bound() && + let hir::TyKind::Infer = arg_hir.kind { + // If the expected region is late bound, the found region is not, and users are asking compiler + // to infer the type, we can suggest adding `: &_`. + let Ok(arg) = self.tcx.sess.source_map().span_to_snippet(arg_hir.span) else { return; }; + suggestion += &format!("{}: &_", arg); + has_suggestion = true; + } else { + let Ok(arg) = self.tcx.sess.source_map().span_to_snippet(arg_hir.span) else { return; }; + // Otherwise, keep it as-is. + suggestion += &arg; } + } + suggestion += "|"; + + if has_suggestion { + diag.span_suggestion_verbose( + span, + "consider specifying the type of the closure parameters", + suggestion, + Applicability::MaybeIncorrect, + ); + } } } } diff --git a/tests/ui/lifetimes/issue-105675.rs b/tests/ui/lifetimes/issue-105675.rs new file mode 100644 index 0000000000000..a01fbef4b3bc4 --- /dev/null +++ b/tests/ui/lifetimes/issue-105675.rs @@ -0,0 +1,16 @@ +fn thing(x: impl FnOnce(&u32, &u32)) {} + +fn main() { + let f = |_, _| (); + thing(f); + //~^ ERROR mismatched types + //~^^ ERROR mismatched types + //~^^^ ERROR implementation of `FnOnce` is not general enough + //~^^^^ ERROR implementation of `FnOnce` is not general enough + let f = |x, y| (); + thing(f); + //~^ ERROR mismatched types + //~^^ ERROR mismatched types + //~^^^ ERROR implementation of `FnOnce` is not general enough + //~^^^^ ERROR implementation of `FnOnce` is not general enough +} diff --git a/tests/ui/lifetimes/issue-105675.stderr b/tests/ui/lifetimes/issue-105675.stderr new file mode 100644 index 0000000000000..3d8172ad14a37 --- /dev/null +++ b/tests/ui/lifetimes/issue-105675.stderr @@ -0,0 +1,131 @@ +error[E0308]: mismatched types + --> $DIR/issue-105675.rs:5:5 + | +LL | thing(f); + | ^^^^^^^^ one type is more general than the other + | + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` + found trait `FnOnce<(&u32, &u32)>` +note: this closure does not fulfill the lifetime requirements + --> $DIR/issue-105675.rs:4:13 + | +LL | let f = |_, _| (); + | ^^^^^^ +note: the lifetime requirement is introduced here + --> $DIR/issue-105675.rs:1:18 + | +LL | fn thing(x: impl FnOnce(&u32, &u32)) {} + | ^^^^^^^^^^^^^^^^^^ +help: consider specifying the type of the closure parameters + | +LL | let f = |_: &_, _: &_| (); + | ~~~~~~~~~~~~~~ + +error[E0308]: mismatched types + --> $DIR/issue-105675.rs:5:5 + | +LL | thing(f); + | ^^^^^^^^ one type is more general than the other + | + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` + found trait `FnOnce<(&u32, &u32)>` +note: this closure does not fulfill the lifetime requirements + --> $DIR/issue-105675.rs:4:13 + | +LL | let f = |_, _| (); + | ^^^^^^ +note: the lifetime requirement is introduced here + --> $DIR/issue-105675.rs:1:18 + | +LL | fn thing(x: impl FnOnce(&u32, &u32)) {} + | ^^^^^^^^^^^^^^^^^^ +help: consider specifying the type of the closure parameters + | +LL | let f = |_: &_, _: &_| (); + | ~~~~~~~~~~~~~~ + +error: implementation of `FnOnce` is not general enough + --> $DIR/issue-105675.rs:5:5 + | +LL | thing(f); + | ^^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&'2 u32, &u32)` must implement `FnOnce<(&'1 u32, &u32)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 u32, &u32)>`, for some specific lifetime `'2` + +error: implementation of `FnOnce` is not general enough + --> $DIR/issue-105675.rs:5:5 + | +LL | thing(f); + | ^^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&u32, &'2 u32)` must implement `FnOnce<(&u32, &'1 u32)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&u32, &'2 u32)>`, for some specific lifetime `'2` + +error[E0308]: mismatched types + --> $DIR/issue-105675.rs:11:5 + | +LL | thing(f); + | ^^^^^^^^ one type is more general than the other + | + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` + found trait `FnOnce<(&u32, &u32)>` +note: this closure does not fulfill the lifetime requirements + --> $DIR/issue-105675.rs:10:13 + | +LL | let f = |x, y| (); + | ^^^^^^ +note: the lifetime requirement is introduced here + --> $DIR/issue-105675.rs:1:18 + | +LL | fn thing(x: impl FnOnce(&u32, &u32)) {} + | ^^^^^^^^^^^^^^^^^^ +help: consider specifying the type of the closure parameters + | +LL | let f = |x: &_, y: &_| (); + | ~~~~~~~~~~~~~~ + +error[E0308]: mismatched types + --> $DIR/issue-105675.rs:11:5 + | +LL | thing(f); + | ^^^^^^^^ one type is more general than the other + | + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` + found trait `FnOnce<(&u32, &u32)>` +note: this closure does not fulfill the lifetime requirements + --> $DIR/issue-105675.rs:10:13 + | +LL | let f = |x, y| (); + | ^^^^^^ +note: the lifetime requirement is introduced here + --> $DIR/issue-105675.rs:1:18 + | +LL | fn thing(x: impl FnOnce(&u32, &u32)) {} + | ^^^^^^^^^^^^^^^^^^ +help: consider specifying the type of the closure parameters + | +LL | let f = |x: &_, y: &_| (); + | ~~~~~~~~~~~~~~ + +error: implementation of `FnOnce` is not general enough + --> $DIR/issue-105675.rs:11:5 + | +LL | thing(f); + | ^^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&'2 u32, &u32)` must implement `FnOnce<(&'1 u32, &u32)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 u32, &u32)>`, for some specific lifetime `'2` + +error: implementation of `FnOnce` is not general enough + --> $DIR/issue-105675.rs:11:5 + | +LL | thing(f); + | ^^^^^^^^ implementation of `FnOnce` is not general enough + | + = note: closure with signature `fn(&u32, &'2 u32)` must implement `FnOnce<(&u32, &'1 u32)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&u32, &'2 u32)>`, for some specific lifetime `'2` + +error: aborting due to 8 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/lifetimes/issue-79187.stderr b/tests/ui/lifetimes/issue-79187.stderr index 16eaf654cf3fc..209f2b7b7398a 100644 --- a/tests/ui/lifetimes/issue-79187.stderr +++ b/tests/ui/lifetimes/issue-79187.stderr @@ -16,7 +16,7 @@ note: the lifetime requirement is introduced here | LL | fn thing(x: impl FnOnce(&u32)) {} | ^^^^^^^^^^^^ -help: consider changing the type of the closure parameters +help: consider specifying the type of the closure parameters | LL | let f = |_: &_| (); | ~~~~~~~ diff --git a/tests/ui/mismatched_types/closure-mismatch.stderr b/tests/ui/mismatched_types/closure-mismatch.stderr index ab0e137418aec..c5b8270ba84d5 100644 --- a/tests/ui/mismatched_types/closure-mismatch.stderr +++ b/tests/ui/mismatched_types/closure-mismatch.stderr @@ -25,7 +25,7 @@ note: the lifetime requirement is introduced here | LL | fn baz(_: T) {} | ^^^ -help: consider changing the type of the closure parameters +help: consider specifying the type of the closure parameters | LL | baz(|_: &_| ()); | ~~~~~~~ @@ -57,9 +57,9 @@ note: the lifetime requirement is introduced here | LL | fn baz(_: T) {} | ^^^ -help: consider changing the type of the closure parameters +help: consider specifying the type of the closure parameters | -LL | baz(|_: &_| ()); +LL | baz(|x: &_| ()); | ~~~~~~~ error: aborting due to 4 previous errors From c0254834317c7b2131714c497edca5493373d86d Mon Sep 17 00:00:00 2001 From: Alex Chi Date: Fri, 17 Mar 2023 23:21:01 -0400 Subject: [PATCH 03/18] use param instead of ty Signed-off-by: Alex Chi --- .../src/infer/error_reporting/suggest.rs | 22 +++- tests/ui/lifetimes/issue-105675.rs | 8 +- tests/ui/lifetimes/issue-105675.stderr | 104 +++++++----------- tests/ui/lifetimes/issue-79187-2.stderr | 4 +- 4 files changed, 63 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs index 9a74b23b6bde6..1ada599bcedd3 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/suggest.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/suggest.rs @@ -564,7 +564,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { diag: &mut Diagnostic, ) { // 0. Extract fn_decl from hir - let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(hir::Closure { fn_decl, .. }), .. }) = hir else { return; }; + let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Closure(hir::Closure { body, fn_decl, .. }), .. }) = hir else { return; }; + let hir::Body { params, .. } = self.tcx.hir().body(*body); // 1. Get the substs of the closure. // 2. Assume exp_found is FnOnce / FnMut / Fn, we can extract function parameters from [1]. @@ -582,7 +583,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let mut is_first = true; let mut has_suggestion = false; - for ((expected, found), arg_hir) in expected.iter().zip(found.iter()).zip(fn_decl.inputs.iter()) { + for (((expected, found), param_hir), arg_hir) in expected.iter() + .zip(found.iter()) + .zip(params.iter()) + .zip(fn_decl.inputs.iter()) { if is_first { is_first = false; } else { @@ -596,11 +600,19 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let hir::TyKind::Infer = arg_hir.kind { // If the expected region is late bound, the found region is not, and users are asking compiler // to infer the type, we can suggest adding `: &_`. - let Ok(arg) = self.tcx.sess.source_map().span_to_snippet(arg_hir.span) else { return; }; - suggestion += &format!("{}: &_", arg); + if param_hir.pat.span == param_hir.ty_span { + // for `|x|`, `|_|`, `|x: impl Foo|` + let Ok(pat) = self.tcx.sess.source_map().span_to_snippet(param_hir.pat.span) else { return; }; + suggestion += &format!("{}: &_", pat); + } else { + // for `|x: ty|`, `|_: ty|` + let Ok(pat) = self.tcx.sess.source_map().span_to_snippet(param_hir.pat.span) else { return; }; + let Ok(ty) = self.tcx.sess.source_map().span_to_snippet(param_hir.ty_span) else { return; }; + suggestion += &format!("{}: &{}", pat, ty); + } has_suggestion = true; } else { - let Ok(arg) = self.tcx.sess.source_map().span_to_snippet(arg_hir.span) else { return; }; + let Ok(arg) = self.tcx.sess.source_map().span_to_snippet(param_hir.span) else { return; }; // Otherwise, keep it as-is. suggestion += &arg; } diff --git a/tests/ui/lifetimes/issue-105675.rs b/tests/ui/lifetimes/issue-105675.rs index a01fbef4b3bc4..58d8be8b65f78 100644 --- a/tests/ui/lifetimes/issue-105675.rs +++ b/tests/ui/lifetimes/issue-105675.rs @@ -1,13 +1,11 @@ -fn thing(x: impl FnOnce(&u32, &u32)) {} +fn thing(x: impl FnOnce(&u32, &u32, u32)) {} fn main() { - let f = |_, _| (); + let f = | _ , y: &u32 , z | (); thing(f); //~^ ERROR mismatched types //~^^ ERROR mismatched types - //~^^^ ERROR implementation of `FnOnce` is not general enough - //~^^^^ ERROR implementation of `FnOnce` is not general enough - let f = |x, y| (); + let f = | x, y: _ , z: u32 | (); thing(f); //~^ ERROR mismatched types //~^^ ERROR mismatched types diff --git a/tests/ui/lifetimes/issue-105675.stderr b/tests/ui/lifetimes/issue-105675.stderr index 3d8172ad14a37..66415f72bcb45 100644 --- a/tests/ui/lifetimes/issue-105675.stderr +++ b/tests/ui/lifetimes/issue-105675.stderr @@ -4,22 +4,22 @@ error[E0308]: mismatched types LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` - found trait `FnOnce<(&u32, &u32)>` + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` + found trait `for<'a> FnOnce<(&u32, &'a u32, u32)>` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-105675.rs:4:13 | -LL | let f = |_, _| (); - | ^^^^^^ +LL | let f = | _ , y: &u32 , z | (); + | ^^^^^^^^^^^^^^^^^^^ note: the lifetime requirement is introduced here --> $DIR/issue-105675.rs:1:18 | -LL | fn thing(x: impl FnOnce(&u32, &u32)) {} - | ^^^^^^^^^^^^^^^^^^ +LL | fn thing(x: impl FnOnce(&u32, &u32, u32)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider specifying the type of the closure parameters | -LL | let f = |_: &_, _: &_| (); - | ~~~~~~~~~~~~~~ +LL | let f = |_: &_, y: &u32, z| (); + | ~~~~~~~~~~~~~~~~~~~ error[E0308]: mismatched types --> $DIR/issue-105675.rs:5:5 @@ -27,105 +27,83 @@ error[E0308]: mismatched types LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` - found trait `FnOnce<(&u32, &u32)>` + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` + found trait `for<'a> FnOnce<(&u32, &'a u32, u32)>` note: this closure does not fulfill the lifetime requirements --> $DIR/issue-105675.rs:4:13 | -LL | let f = |_, _| (); - | ^^^^^^ +LL | let f = | _ , y: &u32 , z | (); + | ^^^^^^^^^^^^^^^^^^^ note: the lifetime requirement is introduced here --> $DIR/issue-105675.rs:1:18 | -LL | fn thing(x: impl FnOnce(&u32, &u32)) {} - | ^^^^^^^^^^^^^^^^^^ -help: consider specifying the type of the closure parameters - | -LL | let f = |_: &_, _: &_| (); - | ~~~~~~~~~~~~~~ - -error: implementation of `FnOnce` is not general enough - --> $DIR/issue-105675.rs:5:5 - | -LL | thing(f); - | ^^^^^^^^ implementation of `FnOnce` is not general enough - | - = note: closure with signature `fn(&'2 u32, &u32)` must implement `FnOnce<(&'1 u32, &u32)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&'2 u32, &u32)>`, for some specific lifetime `'2` - -error: implementation of `FnOnce` is not general enough - --> $DIR/issue-105675.rs:5:5 - | -LL | thing(f); - | ^^^^^^^^ implementation of `FnOnce` is not general enough - | - = note: closure with signature `fn(&u32, &'2 u32)` must implement `FnOnce<(&u32, &'1 u32)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&u32, &'2 u32)>`, for some specific lifetime `'2` +LL | fn thing(x: impl FnOnce(&u32, &u32, u32)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types - --> $DIR/issue-105675.rs:11:5 + --> $DIR/issue-105675.rs:9:5 | LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` - found trait `FnOnce<(&u32, &u32)>` + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` + found trait `FnOnce<(&u32, &u32, u32)>` note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-105675.rs:10:13 + --> $DIR/issue-105675.rs:8:13 | -LL | let f = |x, y| (); - | ^^^^^^ +LL | let f = | x, y: _ , z: u32 | (); + | ^^^^^^^^^^^^^^^^^^^^^ note: the lifetime requirement is introduced here --> $DIR/issue-105675.rs:1:18 | -LL | fn thing(x: impl FnOnce(&u32, &u32)) {} - | ^^^^^^^^^^^^^^^^^^ +LL | fn thing(x: impl FnOnce(&u32, &u32, u32)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider specifying the type of the closure parameters | -LL | let f = |x: &_, y: &_| (); - | ~~~~~~~~~~~~~~ +LL | let f = |x: &_, y: &_, z: u32| (); + | ~~~~~~~~~~~~~~~~~~~~~~ error[E0308]: mismatched types - --> $DIR/issue-105675.rs:11:5 + --> $DIR/issue-105675.rs:9:5 | LL | thing(f); | ^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32)>` - found trait `FnOnce<(&u32, &u32)>` + = note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>` + found trait `FnOnce<(&u32, &u32, u32)>` note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-105675.rs:10:13 + --> $DIR/issue-105675.rs:8:13 | -LL | let f = |x, y| (); - | ^^^^^^ +LL | let f = | x, y: _ , z: u32 | (); + | ^^^^^^^^^^^^^^^^^^^^^ note: the lifetime requirement is introduced here --> $DIR/issue-105675.rs:1:18 | -LL | fn thing(x: impl FnOnce(&u32, &u32)) {} - | ^^^^^^^^^^^^^^^^^^ +LL | fn thing(x: impl FnOnce(&u32, &u32, u32)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider specifying the type of the closure parameters | -LL | let f = |x: &_, y: &_| (); - | ~~~~~~~~~~~~~~ +LL | let f = |x: &_, y: &_, z: u32| (); + | ~~~~~~~~~~~~~~~~~~~~~~ error: implementation of `FnOnce` is not general enough - --> $DIR/issue-105675.rs:11:5 + --> $DIR/issue-105675.rs:9:5 | LL | thing(f); | ^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: closure with signature `fn(&'2 u32, &u32)` must implement `FnOnce<(&'1 u32, &u32)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&'2 u32, &u32)>`, for some specific lifetime `'2` + = note: closure with signature `fn(&'2 u32, &u32, u32)` must implement `FnOnce<(&'1 u32, &u32, u32)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 u32, &u32, u32)>`, for some specific lifetime `'2` error: implementation of `FnOnce` is not general enough - --> $DIR/issue-105675.rs:11:5 + --> $DIR/issue-105675.rs:9:5 | LL | thing(f); | ^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: closure with signature `fn(&u32, &'2 u32)` must implement `FnOnce<(&u32, &'1 u32)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&u32, &'2 u32)>`, for some specific lifetime `'2` + = note: closure with signature `fn(&u32, &'2 u32, u32)` must implement `FnOnce<(&u32, &'1 u32, u32)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&u32, &'2 u32, u32)>`, for some specific lifetime `'2` -error: aborting due to 8 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/lifetimes/issue-79187-2.stderr b/tests/ui/lifetimes/issue-79187-2.stderr index c14d3e0c61b8c..75fd87b3fe9b3 100644 --- a/tests/ui/lifetimes/issue-79187-2.stderr +++ b/tests/ui/lifetimes/issue-79187-2.stderr @@ -43,9 +43,9 @@ note: the lifetime requirement is introduced here | LL | fn take_foo(_: impl Foo) {} | ^^^ -help: consider changing the type of the closure parameters +help: consider specifying the type of the closure parameters | -LL | take_foo(|_: &_| a); +LL | take_foo(|a: &_| a); | ~~~~~~~ error[E0308]: mismatched types From 1e95cddc74f88c1bb86f80103d0d7128f779f544 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Wed, 1 Mar 2023 22:17:08 +1300 Subject: [PATCH 04/18] feat: implement basic suggest-tests tool --- Cargo.lock | 13 ++- Cargo.toml | 1 + src/bootstrap/builder.rs | 22 ++++- src/bootstrap/config.rs | 24 ++--- src/bootstrap/flags.rs | 14 ++- src/bootstrap/lib.rs | 19 +++- src/bootstrap/suggest.rs | 72 ++++++++++++++ src/bootstrap/test.rs | 36 +++++++ src/bootstrap/tool.rs | 1 + src/tools/suggest-tests/Cargo.toml | 9 ++ .../suggest-tests/src/dynamic_suggestions.rs | 23 +++++ src/tools/suggest-tests/src/lib.rs | 96 +++++++++++++++++++ src/tools/suggest-tests/src/main.rs | 27 ++++++ .../suggest-tests/src/static_suggestions.rs | 24 +++++ src/tools/suggest-tests/src/tests.rs | 21 ++++ 15 files changed, 377 insertions(+), 25 deletions(-) create mode 100644 src/bootstrap/suggest.rs create mode 100644 src/tools/suggest-tests/Cargo.toml create mode 100644 src/tools/suggest-tests/src/dynamic_suggestions.rs create mode 100644 src/tools/suggest-tests/src/lib.rs create mode 100644 src/tools/suggest-tests/src/main.rs create mode 100644 src/tools/suggest-tests/src/static_suggestions.rs create mode 100644 src/tools/suggest-tests/src/tests.rs diff --git a/Cargo.lock b/Cargo.lock index d4fca32d750f6..4167fd7de9ae0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3459,9 +3459,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.16.0" +version = "1.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "86f0b0d4bf799edbc74508c1e8bf170ff5f41238e5f8225603ca7caaae2b7860" +checksum = "b7e5500299e16ebb147ae15a00a942af264cf3688f47923b8fc2cd5858f23ad3" [[package]] name = "opener" @@ -6097,6 +6097,15 @@ version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601" +[[package]] +name = "suggest-tests" +version = "0.1.0" +dependencies = [ + "build_helper", + "glob", + "once_cell", +] + [[package]] name = "syn" version = "1.0.102" diff --git a/Cargo.toml b/Cargo.toml index 15cbb2659c9b3..1fcaaf6ddc4d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,6 +44,7 @@ members = [ "src/tools/lld-wrapper", "src/tools/collect-license-metadata", "src/tools/generate-copyright", + "src/tools/suggest-tests", ] exclude = [ diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index ade8fa4c74dcb..e959ea06f8b69 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -591,6 +591,7 @@ pub enum Kind { Install, Run, Setup, + Suggest, } impl Kind { @@ -610,6 +611,7 @@ impl Kind { "install" => Kind::Install, "run" | "r" => Kind::Run, "setup" => Kind::Setup, + "suggest" => Kind::Suggest, _ => return None, }) } @@ -629,6 +631,7 @@ impl Kind { Kind::Install => "install", Kind::Run => "run", Kind::Setup => "setup", + Kind::Suggest => "suggest", } } } @@ -709,6 +712,7 @@ impl<'a> Builder<'a> { test::CrateRustdoc, test::CrateRustdocJsonTypes, test::CrateJsonDocLint, + test::SuggestTestsCrate, test::Linkcheck, test::TierCheck, test::ReplacePlaceholderTest, @@ -827,7 +831,7 @@ impl<'a> Builder<'a> { Kind::Setup => describe!(setup::Profile, setup::Hook, setup::Link, setup::Vscode), Kind::Clean => describe!(clean::CleanAll, clean::Rustc, clean::Std), // special-cased in Build::build() - Kind::Format => vec![], + Kind::Format | Kind::Suggest => vec![], } } @@ -891,6 +895,7 @@ impl<'a> Builder<'a> { Subcommand::Run { ref paths, .. } => (Kind::Run, &paths[..]), Subcommand::Clean { ref paths, .. } => (Kind::Clean, &paths[..]), Subcommand::Format { .. } => (Kind::Format, &[][..]), + Subcommand::Suggest { .. } => (Kind::Suggest, &[][..]), Subcommand::Setup { profile: ref path } => ( Kind::Setup, path.as_ref().map_or([].as_slice(), |path| std::slice::from_ref(path)), @@ -900,6 +905,21 @@ impl<'a> Builder<'a> { Self::new_internal(build, kind, paths.to_owned()) } + /// Creates a new standalone builder for use outside of the normal process + pub fn new_standalone( + build: &mut Build, + kind: Kind, + paths: Vec, + stage: Option, + ) -> Builder<'_> { + // FIXME: don't mutate `build` + if let Some(stage) = stage { + build.config.stage = stage; + } + + Self::new_internal(build, kind, paths.to_owned()) + } + pub fn execute_cli(&self) { self.run_step_descriptions(&Builder::get_step_descriptions(self.kind), &self.paths); } diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs index dd65dc91c0cd3..cc3b3bc25f3d5 100644 --- a/src/bootstrap/config.rs +++ b/src/bootstrap/config.rs @@ -56,8 +56,7 @@ pub enum DryRun { /// filled out from the decoded forms of the structs below. For documentation /// each field, see the corresponding fields in /// `config.example.toml`. -#[derive(Default)] -#[cfg_attr(test, derive(Clone))] +#[derive(Default, Clone)] pub struct Config { pub changelog_seen: Option, pub ccache: Option, @@ -240,23 +239,20 @@ pub struct Config { pub initial_rustfmt: RefCell, } -#[derive(Default, Deserialize)] -#[cfg_attr(test, derive(Clone))] +#[derive(Default, Deserialize, Clone)] pub struct Stage0Metadata { pub compiler: CompilerMetadata, pub config: Stage0Config, pub checksums_sha256: HashMap, pub rustfmt: Option, } -#[derive(Default, Deserialize)] -#[cfg_attr(test, derive(Clone))] +#[derive(Default, Deserialize, Clone)] pub struct CompilerMetadata { pub date: String, pub version: String, } -#[derive(Default, Deserialize)] -#[cfg_attr(test, derive(Clone))] +#[derive(Default, Deserialize, Clone)] pub struct Stage0Config { pub dist_server: String, pub artifacts_server: String, @@ -264,8 +260,7 @@ pub struct Stage0Config { pub git_merge_commit_email: String, pub nightly_branch: String, } -#[derive(Default, Deserialize)] -#[cfg_attr(test, derive(Clone))] +#[derive(Default, Deserialize, Clone)] pub struct RustfmtMetadata { pub date: String, pub version: String, @@ -443,8 +438,7 @@ impl PartialEq<&str> for TargetSelection { } /// Per-target configuration stored in the global configuration structure. -#[derive(Default)] -#[cfg_attr(test, derive(Clone))] +#[derive(Default, Clone)] pub struct Target { /// Some(path to llvm-config) if using an external LLVM. pub llvm_config: Option, @@ -1396,7 +1390,8 @@ impl Config { | Subcommand::Fix { .. } | Subcommand::Run { .. } | Subcommand::Setup { .. } - | Subcommand::Format { .. } => flags.stage.unwrap_or(0), + | Subcommand::Format { .. } + | Subcommand::Suggest { .. } => flags.stage.unwrap_or(0), }; // CI should always run stage 2 builds, unless it specifically states otherwise @@ -1421,7 +1416,8 @@ impl Config { | Subcommand::Fix { .. } | Subcommand::Run { .. } | Subcommand::Setup { .. } - | Subcommand::Format { .. } => {} + | Subcommand::Format { .. } + | Subcommand::Suggest { .. } => {} } } diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 2b0b772a61817..b6f5f31039838 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -84,8 +84,7 @@ pub struct Flags { pub free_args: Option>, } -#[derive(Debug)] -#[cfg_attr(test, derive(Clone))] +#[derive(Debug, Clone)] pub enum Subcommand { Build { paths: Vec, @@ -149,6 +148,9 @@ pub enum Subcommand { Setup { profile: Option, }, + Suggest { + run: bool, + }, } impl Default for Subcommand { @@ -183,6 +185,7 @@ Subcommands: install Install distribution artifacts run, r Run tools contained in this repository setup Create a config.toml (making it easier to use `x.py` itself) + suggest Suggest a subset of tests to run, based on modified files To learn more about a subcommand, run `./x.py -h`", ); @@ -349,6 +352,9 @@ To learn more about a subcommand, run `./x.py -h`", Kind::Run => { opts.optmulti("", "args", "arguments for the tool", "ARGS"); } + Kind::Suggest => { + opts.optflag("", "run", "run suggested tests"); + } _ => {} }; @@ -565,7 +571,7 @@ Arguments: Profile::all_for_help(" ").trim_end() )); } - Kind::Bench | Kind::Clean | Kind::Dist | Kind::Install => {} + Kind::Bench | Kind::Clean | Kind::Dist | Kind::Install | Kind::Suggest => {} }; // Get any optional paths which occur after the subcommand let mut paths = matches.free[1..].iter().map(|p| p.into()).collect::>(); @@ -626,6 +632,7 @@ Arguments: Kind::Format => Subcommand::Format { check: matches.opt_present("check"), paths }, Kind::Dist => Subcommand::Dist { paths }, Kind::Install => Subcommand::Install { paths }, + Kind::Suggest => Subcommand::Suggest { run: matches.opt_present("run") }, Kind::Run => { if paths.is_empty() { println!("\nrun requires at least a path!\n"); @@ -734,6 +741,7 @@ impl Subcommand { Subcommand::Install { .. } => Kind::Install, Subcommand::Run { .. } => Kind::Run, Subcommand::Setup { .. } => Kind::Setup, + Subcommand::Suggest { .. } => Kind::Suggest, } } diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 5ee18cf641104..2ce9ddd2a680e 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -58,6 +58,7 @@ mod render_tests; mod run; mod sanity; mod setup; +mod suggest; mod tarball; mod test; mod tool; @@ -189,6 +190,7 @@ pub enum GitRepo { /// although most functions are implemented as free functions rather than /// methods specifically on this structure itself (to make it easier to /// organize). +#[derive(Clone)] pub struct Build { /// User-specified configuration from `config.toml`. config: Config, @@ -242,7 +244,7 @@ pub struct Build { metrics: metrics::BuildMetrics, } -#[derive(Debug)] +#[derive(Debug, Clone)] struct Crate { name: Interned, deps: HashSet>, @@ -656,13 +658,20 @@ impl Build { job::setup(self); } - if let Subcommand::Format { check, paths } = &self.config.cmd { - return format::format(&builder::Builder::new(&self), *check, &paths); - } - // Download rustfmt early so that it can be used in rust-analyzer configs. let _ = &builder::Builder::new(&self).initial_rustfmt(); + // hardcoded subcommands + match &self.config.cmd { + Subcommand::Format { check, paths } => { + return format::format(&builder::Builder::new(&self), *check, &paths); + } + Subcommand::Suggest { run } => { + return suggest::suggest(&builder::Builder::new(&self), *run); + } + _ => (), + } + { let builder = builder::Builder::new(&self); if let Some(path) = builder.paths.get(0) { diff --git a/src/bootstrap/suggest.rs b/src/bootstrap/suggest.rs new file mode 100644 index 0000000000000..1a482e4165944 --- /dev/null +++ b/src/bootstrap/suggest.rs @@ -0,0 +1,72 @@ +use std::str::FromStr; + +use std::path::PathBuf; + +use crate::{ + builder::{Builder, Kind}, + tool::Tool, +}; + +/// Suggests a list of possible `x.py` commands to run based on modified files in branch. +pub fn suggest(builder: &Builder<'_>, run: bool) { + let suggestions = + builder.tool_cmd(Tool::SuggestTests).output().expect("failed to run `suggest-tests` tool"); + + if !suggestions.status.success() { + println!("failed to run `suggest-tests` tool ({})", suggestions.status); + println!( + "`suggest_tests` stdout:\n{}`suggest_tests` stderr:\n{}", + String::from_utf8(suggestions.stdout).unwrap(), + String::from_utf8(suggestions.stderr).unwrap() + ); + panic!("failed to run `suggest-tests`"); + } + + let suggestions = String::from_utf8(suggestions.stdout).unwrap(); + let suggestions = suggestions + .lines() + .map(|line| { + let mut sections = line.split_ascii_whitespace(); + + // this code expects one suggestion per line in the following format: + // {some number of flags} [optional stage number] + let cmd = sections.next().unwrap(); + let stage = sections.next_back().map(|s| str::parse(s).ok()).flatten(); + let paths: Vec = sections.map(|p| PathBuf::from_str(p).unwrap()).collect(); + + (cmd, stage, paths) + }) + .collect::>(); + + if !suggestions.is_empty() { + println!("==== SUGGESTIONS ===="); + for sug in &suggestions { + print!("x {} ", sug.0); + if let Some(stage) = sug.1 { + print!("--stage {stage} "); + } + + for path in &sug.2 { + print!("{} ", path.display()); + } + println!(); + } + println!("====================="); + } else { + println!("No suggestions found!"); + return; + } + + if run { + for sug in suggestions { + let mut build = builder.build.clone(); + + let builder = + Builder::new_standalone(&mut build, Kind::parse(&sug.0).unwrap(), sug.2, sug.1); + + builder.execute_cli() + } + } else { + println!("help: consider using the `--run` flag to automatically run suggested tests"); + } +} diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 058ff429e80f1..1d55992446eb4 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -128,6 +128,42 @@ impl Step for CrateJsonDocLint { } } +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct SuggestTestsCrate { + host: TargetSelection, +} + +impl Step for SuggestTestsCrate { + type Output = (); + const ONLY_HOSTS: bool = true; + const DEFAULT: bool = true; + + fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + run.path("src/tools/suggest-tests") + } + + fn make_run(run: RunConfig<'_>) { + run.builder.ensure(SuggestTestsCrate { host: run.target }); + } + + fn run(self, builder: &Builder<'_>) { + let bootstrap_host = builder.config.build; + let compiler = builder.compiler(0, bootstrap_host); + + let suggest_tests = tool::prepare_tool_cargo( + builder, + compiler, + Mode::ToolBootstrap, + bootstrap_host, + "test", + "src/tools/suggest-tests", + SourceType::InTree, + &[], + ); + add_flags_and_try_run_tests(builder, &mut suggest_tests.into()); + } +} + #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct Linkcheck { host: TargetSelection, diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 6a687a7903e0f..d1fd2e8c42cb0 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -433,6 +433,7 @@ bootstrap_tool!( ReplaceVersionPlaceholder, "src/tools/replace-version-placeholder", "replace-version-placeholder"; CollectLicenseMetadata, "src/tools/collect-license-metadata", "collect-license-metadata"; GenerateCopyright, "src/tools/generate-copyright", "generate-copyright"; + SuggestTests, "src/tools/suggest-tests", "suggest-tests"; ); #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)] diff --git a/src/tools/suggest-tests/Cargo.toml b/src/tools/suggest-tests/Cargo.toml new file mode 100644 index 0000000000000..f4f4d548bb79e --- /dev/null +++ b/src/tools/suggest-tests/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "suggest-tests" +version = "0.1.0" +edition = "2021" + +[dependencies] +glob = "0.3.0" +build_helper = { version = "0.1.0", path = "../build_helper" } +once_cell = "1.17.1" diff --git a/src/tools/suggest-tests/src/dynamic_suggestions.rs b/src/tools/suggest-tests/src/dynamic_suggestions.rs new file mode 100644 index 0000000000000..2b0213cdc223c --- /dev/null +++ b/src/tools/suggest-tests/src/dynamic_suggestions.rs @@ -0,0 +1,23 @@ +use std::path::Path; + +use crate::Suggestion; + +type DynamicSuggestion = fn(&Path) -> Vec; + +pub(crate) const DYNAMIC_SUGGESTIONS: &[DynamicSuggestion] = &[|path: &Path| -> Vec { + if path.starts_with("compiler/") || path.starts_with("library/") { + let path = path.components().take(2).collect::>(); + + vec![Suggestion::with_single_path( + "test", + None, + &format!( + "{}/{}", + path[0].as_os_str().to_str().unwrap(), + path[1].as_os_str().to_str().unwrap() + ), + )] + } else { + Vec::new() + } +}]; diff --git a/src/tools/suggest-tests/src/lib.rs b/src/tools/suggest-tests/src/lib.rs new file mode 100644 index 0000000000000..44cd3c7f6a84d --- /dev/null +++ b/src/tools/suggest-tests/src/lib.rs @@ -0,0 +1,96 @@ +use std::{ + fmt::{self, Display}, + path::Path, +}; + +use dynamic_suggestions::DYNAMIC_SUGGESTIONS; +use glob::Pattern; +use static_suggestions::STATIC_SUGGESTIONS; + +mod dynamic_suggestions; +mod static_suggestions; + +#[cfg(test)] +mod tests; + +macro_rules! sug { + ($cmd:expr) => { + Suggestion::new($cmd, None, &[]) + }; + + ($cmd:expr, $paths:expr) => { + Suggestion::new($cmd, None, $paths.as_slice()) + }; + + ($cmd:expr, $stage:expr, $paths:expr) => { + Suggestion::new($cmd, Some($stage), $paths.as_slice()) + }; +} + +pub(crate) use sug; + +pub fn get_suggestions>(modified_files: &[T]) -> Vec { + let mut suggestions = Vec::new(); + + // static suggestions + for sug in STATIC_SUGGESTIONS.iter() { + let glob = Pattern::new(&sug.0).expect("Found invalid glob pattern!"); + + for file in modified_files { + if glob.matches(file.as_ref()) { + suggestions.extend_from_slice(&sug.1); + } + } + } + + // dynamic suggestions + for sug in DYNAMIC_SUGGESTIONS { + for file in modified_files { + let sugs = sug(Path::new(file.as_ref())); + + suggestions.extend_from_slice(&sugs); + } + } + + suggestions.sort(); + suggestions.dedup(); + + suggestions +} + +#[derive(Clone, PartialOrd, Ord, PartialEq, Eq, Debug)] +pub struct Suggestion { + pub cmd: String, + pub stage: Option, + pub paths: Vec, +} + +impl Suggestion { + pub fn new(cmd: &str, stage: Option, paths: &[&str]) -> Self { + Self { cmd: cmd.to_owned(), stage, paths: paths.iter().map(|p| p.to_string()).collect() } + } + + pub fn with_single_path(cmd: &str, stage: Option, path: &str) -> Self { + Self::new(cmd, stage, &[path]) + } +} + +impl Display for Suggestion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + write!(f, "{} ", self.cmd)?; + + for path in &self.paths { + write!(f, "{} ", path)?; + } + + if let Some(stage) = self.stage { + write!(f, "{}", stage)?; + } else { + // write a sentinel value here (in place of a stage) to be consumed + // by the shim in bootstrap, it will be read and ignored. + write!(f, "N/A")?; + } + + Ok(()) + } +} diff --git a/src/tools/suggest-tests/src/main.rs b/src/tools/suggest-tests/src/main.rs new file mode 100644 index 0000000000000..0b541b60cba98 --- /dev/null +++ b/src/tools/suggest-tests/src/main.rs @@ -0,0 +1,27 @@ +use std::process::ExitCode; + +use build_helper::git::get_git_modified_files; +use suggest_tests::get_suggestions; + +fn main() -> ExitCode { + let modified_files = get_git_modified_files(None, &Vec::new()); + let modified_files = match modified_files { + Ok(Some(files)) => files, + Ok(None) => { + eprintln!("git error"); + return ExitCode::FAILURE; + } + Err(err) => { + eprintln!("Could not get modified files from git: \"{err}\""); + return ExitCode::FAILURE; + } + }; + + let suggestions = get_suggestions(&modified_files); + + for sug in &suggestions { + println!("{sug}"); + } + + ExitCode::SUCCESS +} diff --git a/src/tools/suggest-tests/src/static_suggestions.rs b/src/tools/suggest-tests/src/static_suggestions.rs new file mode 100644 index 0000000000000..d8166ead8c49d --- /dev/null +++ b/src/tools/suggest-tests/src/static_suggestions.rs @@ -0,0 +1,24 @@ +use crate::{sug, Suggestion}; + +// FIXME: perhaps this could use `std::lazy` when it is stablizied +macro_rules! static_suggestions { + ($( $glob:expr => [ $( $suggestion:expr ),* ] ),*) => { + pub(crate) const STATIC_SUGGESTIONS: ::once_cell::unsync::Lazy)>> + = ::once_cell::unsync::Lazy::new(|| vec![ $( ($glob, vec![ $($suggestion),* ]) ),*]); + } +} + +static_suggestions! { + "*.md" => [ + sug!("test", 0, ["linkchecker"]) + ], + + "compiler/*" => [ + sug!("check"), + sug!("test", 1, ["src/test/ui", "src/test/run-make"]) + ], + + "src/librustdoc/*" => [ + sug!("test", 1, ["rustdoc"]) + ] +} diff --git a/src/tools/suggest-tests/src/tests.rs b/src/tools/suggest-tests/src/tests.rs new file mode 100644 index 0000000000000..5bc1a7df7ca15 --- /dev/null +++ b/src/tools/suggest-tests/src/tests.rs @@ -0,0 +1,21 @@ +macro_rules! sugg_test { + ( $( $name:ident: $paths:expr => $suggestions:expr ),* ) => { + $( + #[test] + fn $name() { + let suggestions = crate::get_suggestions(&$paths).into_iter().map(|s| s.to_string()).collect::>(); + assert_eq!(suggestions, $suggestions); + } + )* + }; +} + +sugg_test! { + test_error_code_docs: ["compiler/rustc_error_codes/src/error_codes/E0000.md"] => + ["check N/A", "test compiler/rustc_error_codes N/A", "test linkchecker 0", "test src/test/ui src/test/run-make 1"], + + test_rustdoc: ["src/librustdoc/src/lib.rs"] => ["test rustdoc 1"], + + test_rustdoc_and_libstd: ["src/librustdoc/src/lib.rs", "library/std/src/lib.rs"] => + ["test library/std N/A", "test rustdoc 1"] +} From 65c9c79d3f9da41951f7e181b6163ead0f67b6fd Mon Sep 17 00:00:00 2001 From: Tobias Decking Date: Mon, 10 Apr 2023 21:57:45 +0200 Subject: [PATCH 05/18] remove obsolete test --- library/core/tests/num/dec2flt/mod.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/library/core/tests/num/dec2flt/mod.rs b/library/core/tests/num/dec2flt/mod.rs index a2b9bb551e677..874e0ec7093c7 100644 --- a/library/core/tests/num/dec2flt/mod.rs +++ b/library/core/tests/num/dec2flt/mod.rs @@ -127,14 +127,3 @@ fn massive_exponent() { assert_eq!(format!("1e-{max}000").parse(), Ok(0.0)); assert_eq!(format!("1e{max}000").parse(), Ok(f64::INFINITY)); } - -#[test] -fn borderline_overflow() { - let mut s = "0.".to_string(); - for _ in 0..375 { - s.push('3'); - } - // At the time of this writing, this returns Err(..), but this is a bug that should be fixed. - // It makes no sense to enshrine that in a test, the important part is that it doesn't panic. - let _ = s.parse::(); -} From 171f5414705194067557cd7b70bd680308b9cced Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 11 Apr 2023 10:25:49 +0200 Subject: [PATCH 06/18] don't uniquify regions when canonicalizing --- .../src/solve/canonicalize.rs | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/canonicalize.rs b/compiler/rustc_trait_selection/src/solve/canonicalize.rs index 55025e2e72b9c..479f252987014 100644 --- a/compiler/rustc_trait_selection/src/solve/canonicalize.rs +++ b/compiler/rustc_trait_selection/src/solve/canonicalize.rs @@ -125,8 +125,9 @@ impl<'a, 'tcx> Canonicalizer<'a, 'tcx> { // - var_infos: [E0, U1, E1, U1, E1, E6, U6], curr_compressed_uv: 1, next_orig_uv: 6 // - var_infos: [E0, U1, E1, U1, E1, E2, U2], curr_compressed_uv: 2, next_orig_uv: - // - // This algorithm runs in `O(n²)` where `n` is the number of different universe - // indices in the input. This should be fine as `n` is expected to be small. + // This algorithm runs in `O(nm)` where `n` is the number of different universe + // indices in the input and `m` is the number of canonical variables. + // This should be fine as both `n` and `m` are expected to be small. let mut curr_compressed_uv = ty::UniverseIndex::ROOT; let mut existential_in_new_uv = false; let mut next_orig_uv = Some(ty::UniverseIndex::ROOT); @@ -245,18 +246,14 @@ impl<'tcx> TypeFolder> for Canonicalizer<'_, 'tcx> { ty::ReError(_) => return r, }; - let existing_bound_var = match self.canonicalize_mode { - CanonicalizeMode::Input => None, - CanonicalizeMode::Response { .. } => { - self.variables.iter().position(|&v| v == r.into()).map(ty::BoundVar::from) - } - }; - let var = existing_bound_var.unwrap_or_else(|| { - let var = ty::BoundVar::from(self.variables.len()); - self.variables.push(r.into()); - self.primitive_var_infos.push(CanonicalVarInfo { kind }); - var - }); + let var = ty::BoundVar::from( + self.variables.iter().position(|&v| v == r.into()).unwrap_or_else(|| { + let var = self.variables.len(); + self.variables.push(r.into()); + self.primitive_var_infos.push(CanonicalVarInfo { kind }); + var + }), + ); let br = ty::BoundRegion { var, kind: BrAnon(None) }; self.interner().mk_re_late_bound(self.binder_index, br) } From 43e6f99b9d3f8b4af3373b7c5fab6718410e8cc1 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 11 Apr 2023 10:27:57 +0200 Subject: [PATCH 07/18] remove issue-2718.rs test this test was added for rust 0.4 and doesn't test anything specific. The repro originally relied on extern functions which are now just ordinary methods. It's also a run pass test even though `main` has been commented out. --- tests/ui/regions/issue-2718.rs | 327 --------------------------------- 1 file changed, 327 deletions(-) delete mode 100644 tests/ui/regions/issue-2718.rs diff --git a/tests/ui/regions/issue-2718.rs b/tests/ui/regions/issue-2718.rs deleted file mode 100644 index 6449337eea4e7..0000000000000 --- a/tests/ui/regions/issue-2718.rs +++ /dev/null @@ -1,327 +0,0 @@ -// run-pass -#![allow(dead_code)] -#![allow(unused_unsafe)] -#![allow(unused_imports)] -#![allow(non_camel_case_types)] - -pub type Task = isize; - -// tjc: I don't know why -pub mod pipes { - use self::state::{empty, full, blocked, terminated}; - use super::Task; - use std::mem::{forget, transmute}; - use std::mem::{replace, swap}; - use std::mem; - use std::thread; - use std::marker::Send; - - pub struct Stuff { - state: state, - blocked_task: Option, - payload: Option - } - - #[derive(PartialEq, Debug)] - #[repr(isize)] - pub enum state { - empty, - full, - blocked, - terminated - } - - pub struct packet { - state: state, - blocked_task: Option, - payload: Option - } - - unsafe impl Send for packet {} - - pub fn packet() -> *const packet { - unsafe { - let p: *const packet = mem::transmute(Box::new(Stuff{ - state: empty, - blocked_task: None::, - payload: None:: - })); - p - } - } - - mod rusti { - pub fn atomic_xchg(_dst: &mut isize, _src: isize) -> isize { panic!(); } - pub fn atomic_xchg_acq(_dst: &mut isize, _src: isize) -> isize { panic!(); } - pub fn atomic_xchg_rel(_dst: &mut isize, _src: isize) -> isize { panic!(); } - } - - // We should consider moving this to ::std::unsafe, although I - // suspect graydon would want us to use void pointers instead. - pub unsafe fn uniquify(x: *const T) -> Box { - mem::transmute(x) - } - - pub fn swap_state_acq(dst: &mut state, src: state) -> state { - unsafe { - transmute(rusti::atomic_xchg_acq(transmute(dst), src as isize)) - } - } - - pub fn swap_state_rel(dst: &mut state, src: state) -> state { - unsafe { - transmute(rusti::atomic_xchg_rel(transmute(dst), src as isize)) - } - } - - pub fn send(mut p: send_packet, payload: T) { - let p = p.unwrap(); - let mut p = unsafe { uniquify(p) }; - assert!((*p).payload.is_none()); - (*p).payload = Some(payload); - let old_state = swap_state_rel(&mut (*p).state, full); - match old_state { - empty => { - // Yay, fastpath. - - // The receiver will eventually clean this up. - unsafe { forget(p); } - } - full => { panic!("duplicate send") } - blocked => { - - // The receiver will eventually clean this up. - unsafe { forget(p); } - } - terminated => { - // The receiver will never receive this. Rely on drop_glue - // to clean everything up. - } - } - } - - pub fn recv(mut p: recv_packet) -> Option { - let p = p.unwrap(); - let mut p = unsafe { uniquify(p) }; - loop { - let old_state = swap_state_acq(&mut (*p).state, - blocked); - match old_state { - empty | blocked => { thread::yield_now(); } - full => { - let payload = replace(&mut p.payload, None); - return Some(payload.unwrap()) - } - terminated => { - assert_eq!(old_state, terminated); - return None; - } - } - } - } - - pub fn sender_terminate(p: *const packet) { - let mut p = unsafe { uniquify(p) }; - match swap_state_rel(&mut (*p).state, terminated) { - empty | blocked => { - // The receiver will eventually clean up. - unsafe { forget(p) } - } - full => { - // This is impossible - panic!("you dun goofed") - } - terminated => { - // I have to clean up, use drop_glue - } - } - } - - pub fn receiver_terminate(p: *const packet) { - let mut p = unsafe { uniquify(p) }; - match swap_state_rel(&mut (*p).state, terminated) { - empty => { - // the sender will clean up - unsafe { forget(p) } - } - blocked => { - // this shouldn't happen. - panic!("terminating a blocked packet") - } - terminated | full => { - // I have to clean up, use drop_glue - } - } - } - - pub struct send_packet { - p: Option<*const packet>, - } - - impl Drop for send_packet { - fn drop(&mut self) { - unsafe { - if self.p != None { - let self_p: &mut Option<*const packet> = - mem::transmute(&mut self.p); - let p = replace(self_p, None); - sender_terminate(p.unwrap()) - } - } - } - } - - impl send_packet { - pub fn unwrap(&mut self) -> *const packet { - replace(&mut self.p, None).unwrap() - } - } - - pub fn send_packet(p: *const packet) -> send_packet { - send_packet { - p: Some(p) - } - } - - pub struct recv_packet { - p: Option<*const packet>, - } - - impl Drop for recv_packet { - fn drop(&mut self) { - unsafe { - if self.p != None { - let self_p: &mut Option<*const packet> = - mem::transmute(&mut self.p); - let p = replace(self_p, None); - receiver_terminate(p.unwrap()) - } - } - } - } - - impl recv_packet { - pub fn unwrap(&mut self) -> *const packet { - replace(&mut self.p, None).unwrap() - } - } - - pub fn recv_packet(p: *const packet) -> recv_packet { - recv_packet { - p: Some(p) - } - } - - pub fn entangle() -> (send_packet, recv_packet) { - let p = packet(); - (send_packet(p), recv_packet(p)) - } -} - -pub mod pingpong { - use std::mem; - - pub struct ping(::pipes::send_packet); - - unsafe impl Send for ping {} - - pub struct pong(::pipes::send_packet); - - unsafe impl Send for pong {} - - pub fn liberate_ping(p: ping) -> ::pipes::send_packet { - unsafe { - let _addr : *const ::pipes::send_packet = match &p { - &ping(ref x) => { mem::transmute(x) } - }; - panic!() - } - } - - pub fn liberate_pong(p: pong) -> ::pipes::send_packet { - unsafe { - let _addr : *const ::pipes::send_packet = match &p { - &pong(ref x) => { mem::transmute(x) } - }; - panic!() - } - } - - pub fn init() -> (client::ping, server::ping) { - ::pipes::entangle() - } - - pub mod client { - use pingpong; - - pub type ping = ::pipes::send_packet; - pub type pong = ::pipes::recv_packet; - - pub fn do_ping(c: ping) -> pong { - let (sp, rp) = ::pipes::entangle(); - - ::pipes::send(c, pingpong::ping(sp)); - rp - } - - pub fn do_pong(c: pong) -> (ping, ()) { - let packet = ::pipes::recv(c); - if packet.is_none() { - panic!("sender closed the connection") - } - (pingpong::liberate_pong(packet.unwrap()), ()) - } - } - - pub mod server { - use pingpong; - - pub type ping = ::pipes::recv_packet; - pub type pong = ::pipes::send_packet; - - pub fn do_ping(c: ping) -> (pong, ()) { - let packet = ::pipes::recv(c); - if packet.is_none() { - panic!("sender closed the connection") - } - (pingpong::liberate_ping(packet.unwrap()), ()) - } - - pub fn do_pong(c: pong) -> ping { - let (sp, rp) = ::pipes::entangle(); - ::pipes::send(c, pingpong::pong(sp)); - rp - } - } -} - -fn client(chan: pingpong::client::ping) { - let chan = pingpong::client::do_ping(chan); - println!("Sent ping"); - let (_chan, _data) = pingpong::client::do_pong(chan); - println!("Received pong"); -} - -fn server(chan: pingpong::server::ping) { - let (chan, _data) = pingpong::server::do_ping(chan); - println!("Received ping"); - let _chan = pingpong::server::do_pong(chan); - println!("Sent pong"); -} - -pub fn main() { - /* -// Commented out because of option::get error - - let (client_, server_) = pingpong::init(); - - task::spawn {|client_| - let client__ = client_.take(); - client(client__); - }; - task::spawn {|server_| - let server__ = server_.take(); - server(server_ˊ); - }; - */ -} From a159dcda6292c03faf3adb37f0a176ca7a35f0dc Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Wed, 12 Apr 2023 14:01:59 +1200 Subject: [PATCH 08/18] fix: disable `x suggest` when using `build-metrics` --- src/bootstrap/lib.rs | 2 +- src/bootstrap/suggest.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 2ce9ddd2a680e..1f2fa3df98e1e 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -190,7 +190,7 @@ pub enum GitRepo { /// although most functions are implemented as free functions rather than /// methods specifically on this structure itself (to make it easier to /// organize). -#[derive(Clone)] +#[cfg_attr(not(feature = "build-metrics"), derive(Clone))] pub struct Build { /// User-specified configuration from `config.toml`. config: Config, diff --git a/src/bootstrap/suggest.rs b/src/bootstrap/suggest.rs index 1a482e4165944..ff20ebec26772 100644 --- a/src/bootstrap/suggest.rs +++ b/src/bootstrap/suggest.rs @@ -1,3 +1,5 @@ +#![cfg_attr(feature = "build-metrics", allow(unused))] + use std::str::FromStr; use std::path::PathBuf; @@ -7,7 +9,13 @@ use crate::{ tool::Tool, }; +#[cfg(feature = "build-metrics")] +pub fn suggest(builder: &Builder<'_>, run: bool) { + panic!("`x suggest` is not supported with `build-metrics`") +} + /// Suggests a list of possible `x.py` commands to run based on modified files in branch. +#[cfg(not(feature = "build-metrics"))] pub fn suggest(builder: &Builder<'_>, run: bool) { let suggestions = builder.tool_cmd(Tool::SuggestTests).output().expect("failed to run `suggest-tests` tool"); From 722e07854a8ea7b276c802400ac7224f41aa6910 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 13 Apr 2023 14:46:51 +1000 Subject: [PATCH 09/18] Remove useless match. --- compiler/rustc_hir_typeck/src/generator_interior/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs index 5faa6ab13dd7d..6f0d078fb2caa 100644 --- a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs +++ b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs @@ -293,10 +293,7 @@ pub fn resolve_interior<'a, 'tcx>( type_causes, FnMutDelegate { regions: &mut |br| { - let kind = match br.kind { - ty::BrAnon(span) => ty::BrAnon(span), - _ => br.kind, - }; + let kind = br.kind; let var = ty::BoundVar::from_usize(bound_vars.len()); bound_vars.push(ty::BoundVariableKind::Region(kind)); counter += 1; From c18773a286094f1e7ddfb52208819841cbff8897 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 13 Apr 2023 12:08:45 +1000 Subject: [PATCH 10/18] Make `mk_bound_region` closure more generic. This is necessary for the subsequent commits. --- .../src/generator_interior/mod.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs index 6f0d078fb2caa..161db618b6fe6 100644 --- a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs +++ b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs @@ -239,8 +239,7 @@ pub fn resolve_interior<'a, 'tcx>( // typeck had previously found constraints that would cause them to be related. let mut counter = 0; - let mut mk_bound_region = |span| { - let kind = ty::BrAnon(span); + let mut mk_bound_region = |kind| { let var = ty::BoundVar::from_u32(counter); counter += 1; ty::BoundRegion { var, kind } @@ -252,24 +251,24 @@ pub fn resolve_interior<'a, 'tcx>( let origin = fcx.region_var_origin(vid); match origin { RegionVariableOrigin::EarlyBoundRegion(span, _) => { - mk_bound_region(Some(span)) + mk_bound_region(ty::BrAnon(Some(span))) } - _ => mk_bound_region(None), + _ => mk_bound_region(ty::BrAnon(None)), } } // FIXME: these should use `BrNamed` ty::ReEarlyBound(region) => { - mk_bound_region(Some(fcx.tcx.def_span(region.def_id))) + mk_bound_region(ty::BrAnon(Some(fcx.tcx.def_span(region.def_id)))) } ty::ReLateBound(_, ty::BoundRegion { kind, .. }) | ty::ReFree(ty::FreeRegion { bound_region: kind, .. }) => match kind { - ty::BoundRegionKind::BrAnon(span) => mk_bound_region(span), + ty::BoundRegionKind::BrAnon(span) => mk_bound_region(ty::BrAnon(span)), ty::BoundRegionKind::BrNamed(def_id, _) => { - mk_bound_region(Some(fcx.tcx.def_span(def_id))) + mk_bound_region(ty::BrAnon(Some(fcx.tcx.def_span(def_id)))) } - ty::BoundRegionKind::BrEnv => mk_bound_region(None), + ty::BoundRegionKind::BrEnv => mk_bound_region(ty::BrAnon(None)), }, - _ => mk_bound_region(None), + _ => mk_bound_region(ty::BrAnon(None)), }; let r = fcx.tcx.mk_re_late_bound(current_depth, br); r From 74076684a2fbda793e79406c55a3378f1b883c4d Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 13 Apr 2023 00:36:40 -0700 Subject: [PATCH 11/18] Add `tidy-alphabetical` to features in `core` So that people have to keep them sorted in future. --- library/core/src/lib.rs | 50 +++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 4fd5a4bfc65f6..04243544b8359 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -98,11 +98,14 @@ #![warn(multiple_supertrait_upcastable)] // // Library features: -#![feature(const_align_offset)] +// tidy-alphabetical-start +#![feature(char_indices_offset)] #![feature(const_align_of_val)] #![feature(const_align_of_val_raw)] +#![feature(const_align_offset)] #![feature(const_alloc_layout)] #![feature(const_arguments_as_str)] +#![feature(const_array_from_ref)] #![feature(const_array_into_iter_constructors)] #![feature(const_bigint_helper_methods)] #![feature(const_black_box)] @@ -111,6 +114,9 @@ #![feature(const_char_from_u32_unchecked)] #![feature(const_clone)] #![feature(const_cmp)] +#![feature(const_convert)] +#![feature(const_cstr_methods)] +#![feature(const_default_impls)] #![feature(const_discriminant)] #![feature(const_eval_select)] #![feature(const_exact_div)] @@ -119,17 +125,17 @@ #![feature(const_fmt_arguments_new)] #![feature(const_hash)] #![feature(const_heap)] -#![feature(const_convert)] #![feature(const_index_range_slice_index)] #![feature(const_inherent_unchecked_arith)] #![feature(const_int_unchecked_arith)] #![feature(const_intrinsic_forget)] #![feature(const_ipv4)] #![feature(const_ipv6)] +#![feature(const_is_char_boundary)] #![feature(const_likely)] -#![feature(const_maybe_uninit_uninit_array)] #![feature(const_maybe_uninit_as_mut_ptr)] #![feature(const_maybe_uninit_assume_init)] +#![feature(const_maybe_uninit_uninit_array)] #![feature(const_nonnull_new)] #![feature(const_num_from_num)] #![feature(const_ops)] @@ -138,32 +144,35 @@ #![feature(const_pin)] #![feature(const_pointer_byte_offsets)] #![feature(const_pointer_is_aligned)] -#![feature(const_ptr_sub_ptr)] -#![feature(const_replace)] -#![feature(const_result_drop)] #![feature(const_ptr_as_ref)] #![feature(const_ptr_is_null)] #![feature(const_ptr_read)] +#![feature(const_ptr_sub_ptr)] #![feature(const_ptr_write)] #![feature(const_raw_ptr_comparison)] +#![feature(const_replace)] +#![feature(const_result_drop)] #![feature(const_size_of_val)] #![feature(const_size_of_val_raw)] #![feature(const_slice_from_raw_parts_mut)] +#![feature(const_slice_from_ref)] +#![feature(const_slice_index)] #![feature(const_slice_ptr_len)] #![feature(const_slice_split_at_mut)] #![feature(const_str_from_utf8_unchecked_mut)] #![feature(const_swap)] #![feature(const_trait_impl)] +#![feature(const_transmute_copy)] #![feature(const_try)] #![feature(const_type_id)] #![feature(const_type_name)] -#![feature(const_default_impls)] #![feature(const_unicode_case_lookup)] #![feature(const_unsafecell_get_mut)] #![feature(const_waker)] #![feature(core_panic)] -#![feature(char_indices_offset)] #![feature(duration_consts_float)] +#![feature(ip)] +#![feature(is_ascii_octdigit)] #![feature(maybe_uninit_uninit_array)] #![feature(ptr_alignment_type)] #![feature(ptr_metadata)] @@ -171,25 +180,21 @@ #![feature(slice_ptr_get)] #![feature(slice_split_at_unchecked)] #![feature(str_internals)] -#![feature(str_split_remainder)] #![feature(str_split_inclusive_remainder)] +#![feature(str_split_remainder)] #![feature(strict_provenance)] #![feature(utf16_extra)] #![feature(utf16_extra_const)] #![feature(variant_count)] -#![feature(const_array_from_ref)] -#![feature(const_slice_from_ref)] -#![feature(const_slice_index)] -#![feature(const_is_char_boundary)] -#![feature(const_cstr_methods)] -#![feature(ip)] -#![feature(is_ascii_octdigit)] +// tidy-alphabetical-end // // Language features: +// tidy-alphabetical-start #![feature(abi_unadjusted)] #![feature(adt_const_params)] #![feature(allow_internal_unsafe)] #![feature(allow_internal_unstable)] +#![feature(asm_const)] #![feature(associated_type_bounds)] #![feature(auto_traits)] #![feature(c_unwind)] @@ -206,13 +211,12 @@ #![feature(deprecated_suggestion)] #![feature(derive_const)] #![feature(doc_cfg)] +#![feature(doc_cfg_hide)] #![feature(doc_notable_trait)] -#![feature(generic_arg_infer)] -#![feature(rustdoc_internals)] #![feature(exhaustive_patterns)] -#![feature(doc_cfg_hide)] #![feature(extern_types)] #![feature(fundamental)] +#![feature(generic_arg_infer)] #![feature(if_let_guard)] #![feature(inline_const)] #![feature(intra_doc_pointers)] @@ -221,6 +225,7 @@ #![feature(link_llvm_intrinsics)] #![feature(macro_metavar_expr)] #![feature(min_specialization)] +#![feature(multiple_supertrait_upcastable)] #![feature(must_not_suspend)] #![feature(negative_impls)] #![feature(never_type)] @@ -231,6 +236,7 @@ #![feature(repr_simd)] #![feature(rustc_allow_const_fn_unstable)] #![feature(rustc_attrs)] +#![feature(rustdoc_internals)] #![feature(simd_ffi)] #![feature(staged_api)] #![feature(stmt_expr_attributes)] @@ -240,11 +246,10 @@ #![feature(try_blocks)] #![feature(unboxed_closures)] #![feature(unsized_fn_params)] -#![feature(asm_const)] -#![feature(const_transmute_copy)] -#![feature(multiple_supertrait_upcastable)] +// tidy-alphabetical-end // // Target features: +// tidy-alphabetical-start #![feature(arm_target_feature)] #![feature(avx512_target_feature)] #![feature(hexagon_target_feature)] @@ -255,6 +260,7 @@ #![feature(sse4a_target_feature)] #![feature(tbm_target_feature)] #![feature(wasm_target_feature)] +// tidy-alphabetical-end // allow using `core::` in intra-doc links #[allow(unused_extern_crates)] From cd868dcf987b71148bc40271aede9f5b02645c26 Mon Sep 17 00:00:00 2001 From: jmaargh Date: Thu, 13 Apr 2023 22:26:38 +0100 Subject: [PATCH 12/18] Cover edge cases for {f32, f64}.hypot() docs Re-phrase in a way that handles input values being either 0 or negative. --- library/std/src/f32.rs | 6 ++++-- library/std/src/f64.rs | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/library/std/src/f32.rs b/library/std/src/f32.rs index c7c33678fd34c..408244b2ce9eb 100644 --- a/library/std/src/f32.rs +++ b/library/std/src/f32.rs @@ -581,8 +581,10 @@ impl f32 { unsafe { cmath::cbrtf(self) } } - /// Calculates the length of the hypotenuse of a right-angle triangle given - /// legs of length `x` and `y`. + /// Compute the distance between the origin and a point (`x`, `y`) on the + /// Euclidean plane. Equivalently, compute the length of the hypotenuse of a + /// right-angle triangle with other sides having length `x.abs()` and + /// `y.abs()`. /// /// # Examples /// diff --git a/library/std/src/f64.rs b/library/std/src/f64.rs index b1faa670307d6..6782b861f110b 100644 --- a/library/std/src/f64.rs +++ b/library/std/src/f64.rs @@ -583,8 +583,10 @@ impl f64 { unsafe { cmath::cbrt(self) } } - /// Calculates the length of the hypotenuse of a right-angle triangle given - /// legs of length `x` and `y`. + /// Compute the distance between the origin and a point (`x`, `y`) on the + /// Euclidean plane. Equivalently, compute the length of the hypotenuse of a + /// right-angle triangle with other sides having length `x.abs()` and + /// `y.abs()`. /// /// # Examples /// From dcc51f1ef5e19dba1d791e47c0dc07f45b68fa44 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Thu, 13 Apr 2023 15:16:55 -0600 Subject: [PATCH 13/18] change usage of bound_impl_subject to impl_subject --- compiler/rustc_trait_selection/src/traits/coherence.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 3c918b6028d47..93e1dfcfaae11 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -306,7 +306,7 @@ fn negative_impl(tcx: TyCtxt<'_>, impl1_def_id: DefId, impl2_def_id: DefId) -> b &infcx, ObligationCause::dummy(), impl_env, - tcx.impl_subject(impl1_def_id), + tcx.bound_impl_subject(impl1_def_id).subst_identity(), ) { Ok(s) => s, Err(err) => { From e2f5a5a71fa2a55e976484b9e72f5aa425ebab62 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Thu, 13 Apr 2023 15:23:54 -0600 Subject: [PATCH 14/18] make tcx.impl_subject return EarlyBinder, remove bound_impl_subject, rename usages of bound_impl_subject to impl_subject --- compiler/rustc_middle/src/hir/mod.rs | 14 ++++++++------ compiler/rustc_middle/src/ty/util.rs | 4 ---- .../rustc_trait_selection/src/traits/coherence.rs | 2 +- compiler/rustc_trait_selection/src/traits/util.rs | 2 +- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_middle/src/hir/mod.rs b/compiler/rustc_middle/src/hir/mod.rs index 0d8a8c9cdfd43..50a3067c55953 100644 --- a/compiler/rustc_middle/src/hir/mod.rs +++ b/compiler/rustc_middle/src/hir/mod.rs @@ -7,7 +7,7 @@ pub mod nested_filter; pub mod place; use crate::ty::query::Providers; -use crate::ty::{ImplSubject, TyCtxt}; +use crate::ty::{EarlyBinder, ImplSubject, TyCtxt}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::sync::{par_for_each_in, Send, Sync}; use rustc_hir::def_id::{DefId, LocalDefId}; @@ -104,11 +104,13 @@ impl<'tcx> TyCtxt<'tcx> { self.parent_module_from_def_id(id.owner.def_id) } - pub fn impl_subject(self, def_id: DefId) -> ImplSubject<'tcx> { - self.impl_trait_ref(def_id) - .map(|t| t.subst_identity()) - .map(ImplSubject::Trait) - .unwrap_or_else(|| ImplSubject::Inherent(self.type_of(def_id).subst_identity())) + pub fn impl_subject(self, def_id: DefId) -> EarlyBinder> { + EarlyBinder( + self.impl_trait_ref(def_id) + .map(|t| t.subst_identity()) + .map(ImplSubject::Trait) + .unwrap_or_else(|| ImplSubject::Inherent(self.type_of(def_id).subst_identity())), + ) } } diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 4411bcd927d71..c8a78ec03d947 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -708,10 +708,6 @@ impl<'tcx> TyCtxt<'tcx> { ty::EarlyBinder(self.explicit_item_bounds(def_id)) } - pub fn bound_impl_subject(self, def_id: DefId) -> ty::EarlyBinder> { - ty::EarlyBinder(self.impl_subject(def_id)) - } - /// Returns names of captured upvars for closures and generators. /// /// Here are some examples: diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 93e1dfcfaae11..20c2605f219a7 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -306,7 +306,7 @@ fn negative_impl(tcx: TyCtxt<'_>, impl1_def_id: DefId, impl2_def_id: DefId) -> b &infcx, ObligationCause::dummy(), impl_env, - tcx.bound_impl_subject(impl1_def_id).subst_identity(), + tcx.impl_subject(impl1_def_id).subst_identity(), ) { Ok(s) => s, Err(err) => { diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs index 60630979b34e4..20357d4d2501a 100644 --- a/compiler/rustc_trait_selection/src/traits/util.rs +++ b/compiler/rustc_trait_selection/src/traits/util.rs @@ -198,7 +198,7 @@ pub fn impl_subject_and_oblig<'a, 'tcx>( impl_def_id: DefId, impl_substs: SubstsRef<'tcx>, ) -> (ImplSubject<'tcx>, impl Iterator>) { - let subject = selcx.tcx().bound_impl_subject(impl_def_id); + let subject = selcx.tcx().impl_subject(impl_def_id); let subject = subject.subst(selcx.tcx(), impl_substs); let InferOk { value: subject, obligations: normalization_obligations1 } = From 8d5ee1a0729b91545e856dbf14fd75687dc9e2b8 Mon Sep 17 00:00:00 2001 From: Kyle Matsuda Date: Thu, 13 Apr 2023 16:43:34 -0600 Subject: [PATCH 15/18] make impl_subject more readable --- compiler/rustc_middle/src/hir/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_middle/src/hir/mod.rs b/compiler/rustc_middle/src/hir/mod.rs index 50a3067c55953..7770a5e476418 100644 --- a/compiler/rustc_middle/src/hir/mod.rs +++ b/compiler/rustc_middle/src/hir/mod.rs @@ -105,12 +105,10 @@ impl<'tcx> TyCtxt<'tcx> { } pub fn impl_subject(self, def_id: DefId) -> EarlyBinder> { - EarlyBinder( - self.impl_trait_ref(def_id) - .map(|t| t.subst_identity()) - .map(ImplSubject::Trait) - .unwrap_or_else(|| ImplSubject::Inherent(self.type_of(def_id).subst_identity())), - ) + match self.impl_trait_ref(def_id) { + Some(t) => t.map_bound(ImplSubject::Trait), + None => self.type_of(def_id).map_bound(ImplSubject::Inherent), + } } } From 7dbd2e2370897c8363f4b17eb87a3a376811deaa Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 13 Apr 2023 12:31:40 +1000 Subject: [PATCH 16/18] Remove one use of `BrAnon(Some(_))`. --- compiler/rustc_hir_typeck/src/generator_interior/mod.rs | 2 +- tests/ui/generic-associated-types/bugs/issue-100013.stderr | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs index 161db618b6fe6..0a9992c9557e1 100644 --- a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs +++ b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs @@ -258,7 +258,7 @@ pub fn resolve_interior<'a, 'tcx>( } // FIXME: these should use `BrNamed` ty::ReEarlyBound(region) => { - mk_bound_region(ty::BrAnon(Some(fcx.tcx.def_span(region.def_id)))) + mk_bound_region(ty::BrNamed(region.def_id, region.name)) } ty::ReLateBound(_, ty::BoundRegion { kind, .. }) | ty::ReFree(ty::FreeRegion { bound_region: kind, .. }) => match kind { diff --git a/tests/ui/generic-associated-types/bugs/issue-100013.stderr b/tests/ui/generic-associated-types/bugs/issue-100013.stderr index 9db124a81e487..c023e743e8371 100644 --- a/tests/ui/generic-associated-types/bugs/issue-100013.stderr +++ b/tests/ui/generic-associated-types/bugs/issue-100013.stderr @@ -62,12 +62,12 @@ LL | | async {}.await; // a yield point LL | | } | |_____^ | -note: the lifetime defined here... +note: the lifetime `'b` defined here... --> $DIR/issue-100013.rs:28:18 | LL | fn call3<'a: 'b, 'b, I: FutureIterator>() -> impl Send { | ^^ -note: ...must outlive the lifetime defined here +note: ...must outlive the lifetime `'a` defined here --> $DIR/issue-100013.rs:28:10 | LL | fn call3<'a: 'b, 'b, I: FutureIterator>() -> impl Send { From f07c335e90d2705f2db5a18f0b912a08652610f5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 13 Apr 2023 12:41:12 +1000 Subject: [PATCH 17/18] Remove another use of `BrAnon(Some(_))`. --- compiler/rustc_hir_typeck/src/generator_interior/mod.rs | 5 ++--- tests/ui/generic-associated-types/bugs/issue-100013.stderr | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs index 0a9992c9557e1..f397108044333 100644 --- a/compiler/rustc_hir_typeck/src/generator_interior/mod.rs +++ b/compiler/rustc_hir_typeck/src/generator_interior/mod.rs @@ -256,15 +256,14 @@ pub fn resolve_interior<'a, 'tcx>( _ => mk_bound_region(ty::BrAnon(None)), } } - // FIXME: these should use `BrNamed` ty::ReEarlyBound(region) => { mk_bound_region(ty::BrNamed(region.def_id, region.name)) } ty::ReLateBound(_, ty::BoundRegion { kind, .. }) | ty::ReFree(ty::FreeRegion { bound_region: kind, .. }) => match kind { ty::BoundRegionKind::BrAnon(span) => mk_bound_region(ty::BrAnon(span)), - ty::BoundRegionKind::BrNamed(def_id, _) => { - mk_bound_region(ty::BrAnon(Some(fcx.tcx.def_span(def_id)))) + ty::BoundRegionKind::BrNamed(def_id, sym) => { + mk_bound_region(ty::BrNamed(def_id, sym)) } ty::BoundRegionKind::BrEnv => mk_bound_region(ty::BrAnon(None)), }, diff --git a/tests/ui/generic-associated-types/bugs/issue-100013.stderr b/tests/ui/generic-associated-types/bugs/issue-100013.stderr index c023e743e8371..86dbad84d99d8 100644 --- a/tests/ui/generic-associated-types/bugs/issue-100013.stderr +++ b/tests/ui/generic-associated-types/bugs/issue-100013.stderr @@ -28,12 +28,12 @@ LL | | async {}.await; // a yield point LL | | } | |_____^ | -note: the lifetime defined here... +note: the lifetime `'b` defined here... --> $DIR/issue-100013.rs:21:14 | LL | fn call2<'a, 'b, I: FutureIterator>() -> impl Send { | ^^ -note: ...must outlive the lifetime defined here +note: ...must outlive the lifetime `'a` defined here --> $DIR/issue-100013.rs:21:10 | LL | fn call2<'a, 'b, I: FutureIterator>() -> impl Send { From c68c6c3942d8c3aab4d9e9406a0a4473218f8cea Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 14 Apr 2023 03:21:21 +0000 Subject: [PATCH 18/18] Add test for uniquifying regions --- tests/ui/traits/new-solver/iter-filter-projection.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/ui/traits/new-solver/iter-filter-projection.rs diff --git a/tests/ui/traits/new-solver/iter-filter-projection.rs b/tests/ui/traits/new-solver/iter-filter-projection.rs new file mode 100644 index 0000000000000..8fb62323aa5a7 --- /dev/null +++ b/tests/ui/traits/new-solver/iter-filter-projection.rs @@ -0,0 +1,12 @@ +// compile-flags: -Ztrait-solver=next +// check-pass + +use std::{iter, slice}; + +struct Attr; + +fn test<'a, T: Iterator>() {} + +fn main() { + test::, fn(&&Attr) -> bool>>(); +}