From 012a458dcabc6438ee070c074435860690475370 Mon Sep 17 00:00:00 2001 From: Gurinder Singh Date: Mon, 13 May 2024 17:42:44 +0530 Subject: [PATCH 01/14] Suggest removing unused tuple fields if they are the last fields --- compiler/rustc_passes/messages.ftl | 9 +++ compiler/rustc_passes/src/dead.rs | 58 ++++++++++++++----- compiler/rustc_passes/src/errors.rs | 19 ++++-- tests/ui/lint/dead-code/tuple-struct-field.rs | 31 ++++++---- .../lint/dead-code/tuple-struct-field.stderr | 37 +++++++----- 5 files changed, 109 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index a545c170297de..70d314913d2a3 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -597,6 +597,15 @@ passes_pass_by_value = passes_proc_macro_bad_sig = {$kind} has incorrect signature +passes_remove_fields = + consider removing { $num -> + [one] this + *[other] these + } { $num -> + [one] field + *[other] fields + } + passes_repr_conflicting = conflicting representation hints diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 51f288b3c9595..f977f90e8acec 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -23,8 +23,7 @@ use rustc_target::abi::FieldIdx; use std::mem; use crate::errors::{ - ChangeFieldsToBeOfUnitType, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, - UselessAssignment, + ChangeFields, IgnoredDerivedImpls, MultipleDeadCodes, ParentInfo, UselessAssignment, }; // Any local node that may call something in its body block should be @@ -993,17 +992,50 @@ impl<'tcx> DeadVisitor<'tcx> { }; let diag = match report_on { - ReportOn::TupleField => MultipleDeadCodes::UnusedTupleStructFields { - multiple, - num, - descr, - participle, - name_list, - change_fields_suggestion: ChangeFieldsToBeOfUnitType { num, spans: spans.clone() }, - parent_info, - ignored_derived_impls, - }, - + ReportOn::TupleField => { + let tuple_fields = if let Some(parent_id) = parent_item + && let node = tcx.hir_node_by_def_id(parent_id) + && let hir::Node::Item(hir::Item { + kind: hir::ItemKind::Struct(hir::VariantData::Tuple(fields, _, _), _), + .. + }) = node + { + *fields + } else { + &[] + }; + + let trailing_tuple_fields = if tuple_fields.len() >= dead_codes.len() { + LocalDefIdSet::from_iter( + tuple_fields + .iter() + .skip(tuple_fields.len() - dead_codes.len()) + .map(|f| f.def_id), + ) + } else { + LocalDefIdSet::default() + }; + + let fields_suggestion = + // Suggest removal if all tuple fields are at the end. + // Otherwise suggest removal or changing to unit type + if dead_codes.iter().all(|dc| trailing_tuple_fields.contains(&dc.def_id)) { + ChangeFields::Remove { num } + } else { + ChangeFields::ChangeToUnitTypeOrRemove { num, spans: spans.clone() } + }; + + MultipleDeadCodes::UnusedTupleStructFields { + multiple, + num, + descr, + participle, + name_list, + change_fields_suggestion: fields_suggestion, + parent_info, + ignored_derived_impls, + } + } ReportOn::NamedField => MultipleDeadCodes::DeadCodes { multiple, num, diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 03a607348e88c..a590ef8f3a1e2 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1589,7 +1589,7 @@ pub enum MultipleDeadCodes<'tcx> { participle: &'tcx str, name_list: DiagSymbolList, #[subdiagnostic] - change_fields_suggestion: ChangeFieldsToBeOfUnitType, + change_fields_suggestion: ChangeFields, #[subdiagnostic] parent_info: Option>, #[subdiagnostic] @@ -1616,11 +1616,18 @@ pub struct IgnoredDerivedImpls { } #[derive(Subdiagnostic)] -#[multipart_suggestion(passes_change_fields_to_be_of_unit_type, applicability = "has-placeholders")] -pub struct ChangeFieldsToBeOfUnitType { - pub num: usize, - #[suggestion_part(code = "()")] - pub spans: Vec, +pub enum ChangeFields { + #[multipart_suggestion( + passes_change_fields_to_be_of_unit_type, + applicability = "has-placeholders" + )] + ChangeToUnitTypeOrRemove { + num: usize, + #[suggestion_part(code = "()")] + spans: Vec, + }, + #[help(passes_remove_fields)] + Remove { num: usize }, } #[derive(Diagnostic)] diff --git a/tests/ui/lint/dead-code/tuple-struct-field.rs b/tests/ui/lint/dead-code/tuple-struct-field.rs index d13fe029289ad..ff3da4105000d 100644 --- a/tests/ui/lint/dead-code/tuple-struct-field.rs +++ b/tests/ui/lint/dead-code/tuple-struct-field.rs @@ -5,15 +5,20 @@ use std::marker::PhantomData; const LEN: usize = 4; -struct SingleUnused(i32, [u8; LEN], String); -//~^ ERROR: field `1` is never read +struct UnusedAtTheEnd(i32, f32, [u8; LEN], String, u8); +//~^ ERROR:fields `1`, `2`, `3`, and `4` are never read +//~| NOTE: fields in this struct +//~| HELP: consider removing these fields + +struct UnusedJustOneField(i32); +//~^ ERROR: field `0` is never read //~| NOTE: field in this struct -//~| HELP: consider changing the field to be of unit type +//~| HELP: consider removing this field -struct MultipleUnused(i32, f32, String, u8); -//~^ ERROR: fields `0`, `1`, `2`, and `3` are never read +struct UnusedInTheMiddle(i32, f32, String, u8, u32); +//~^ ERROR: fields `1`, `2`, and `4` are never read //~| NOTE: fields in this struct -//~| HELP: consider changing the fields to be of unit type +//~| HELP: consider changing the fields to be of unit type to suppress this warning while preserving the field numbering, or remove the fields struct GoodUnit(()); @@ -23,15 +28,19 @@ struct Void; struct GoodVoid(Void); fn main() { - let w = SingleUnused(42, [0, 1, 2, 3], "abc".to_string()); - let _ = w.0; - let _ = w.2; + let u1 = UnusedAtTheEnd(42, 3.14, [0, 1, 2, 3], "def".to_string(), 4u8); + let _ = u1.0; + + let _ = UnusedJustOneField(42); + + let u2 = UnusedInTheMiddle(42, 3.14, "def".to_string(), 4u8, 5); + let _ = u2.0; + let _ = u2.3; - let m = MultipleUnused(42, 3.14, "def".to_string(), 4u8); let gu = GoodUnit(()); let gp = GoodPhantom(PhantomData); let gv = GoodVoid(Void); - let _ = (gu, gp, gv, m); + let _ = (gu, gp, gv); } diff --git a/tests/ui/lint/dead-code/tuple-struct-field.stderr b/tests/ui/lint/dead-code/tuple-struct-field.stderr index 0154d5489f9fb..434554d7ae5ee 100644 --- a/tests/ui/lint/dead-code/tuple-struct-field.stderr +++ b/tests/ui/lint/dead-code/tuple-struct-field.stderr @@ -1,33 +1,40 @@ -error: field `1` is never read - --> $DIR/tuple-struct-field.rs:8:26 +error: fields `1`, `2`, `3`, and `4` are never read + --> $DIR/tuple-struct-field.rs:8:28 | -LL | struct SingleUnused(i32, [u8; LEN], String); - | ------------ ^^^^^^^^^ +LL | struct UnusedAtTheEnd(i32, f32, [u8; LEN], String, u8); + | -------------- ^^^ ^^^^^^^^^ ^^^^^^ ^^ | | - | field in this struct + | fields in this struct | + = help: consider removing these fields note: the lint level is defined here --> $DIR/tuple-struct-field.rs:1:9 | LL | #![deny(dead_code)] | ^^^^^^^^^ -help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field + +error: field `0` is never read + --> $DIR/tuple-struct-field.rs:13:27 + | +LL | struct UnusedJustOneField(i32); + | ------------------ ^^^ + | | + | field in this struct | -LL | struct SingleUnused(i32, (), String); - | ~~ + = help: consider removing this field -error: fields `0`, `1`, `2`, and `3` are never read - --> $DIR/tuple-struct-field.rs:13:23 +error: fields `1`, `2`, and `4` are never read + --> $DIR/tuple-struct-field.rs:18:31 | -LL | struct MultipleUnused(i32, f32, String, u8); - | -------------- ^^^ ^^^ ^^^^^^ ^^ +LL | struct UnusedInTheMiddle(i32, f32, String, u8, u32); + | ----------------- ^^^ ^^^^^^ ^^^ | | | fields in this struct | help: consider changing the fields to be of unit type to suppress this warning while preserving the field numbering, or remove the fields | -LL | struct MultipleUnused((), (), (), ()); - | ~~ ~~ ~~ ~~ +LL | struct UnusedInTheMiddle(i32, (), (), u8, ()); + | ~~ ~~ ~~ -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors From 586821eacd7389a4a30b60633ceb53596d06e238 Mon Sep 17 00:00:00 2001 From: bohan Date: Sat, 1 Jun 2024 17:43:48 +0800 Subject: [PATCH 02/14] tip for inaccessible traits --- .../rustc_hir_typeck/src/method/suggest.rs | 185 +++++++++++++----- tests/ui/traits/item-privacy.stderr | 15 +- 2 files changed, 143 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index daaaf630f2c45..e2b60bd52b267 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -284,14 +284,57 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } if !candidates.is_empty() { let help = format!( - "{an}other candidate{s} {were} found in the following trait{s}, perhaps \ - add a `use` for {one_of_them}:", + "{an}other candidate{s} {were} found in the following trait{s}", an = if candidates.len() == 1 { "an" } else { "" }, s = pluralize!(candidates.len()), were = pluralize!("was", candidates.len()), - one_of_them = if candidates.len() == 1 { "it" } else { "one_of_them" }, ); - self.suggest_use_candidates(&mut err, help, candidates); + self.suggest_use_candidates( + candidates, + |accessible_sugg, inaccessible_sugg, span| { + let suggest_for_access = + |err: &mut Diag<'_>, mut msg: String, sugg: Vec<_>| { + msg += &format!( + ", perhaps add a `use` for {one_of_them}:", + one_of_them = + if sugg.len() == 1 { "it" } else { "one_of_them" }, + ); + err.span_suggestions( + span, + msg, + sugg, + Applicability::MaybeIncorrect, + ); + }; + let suggest_for_privacy = + |err: &mut Diag<'_>, mut msg: String, sugg: Vec| { + if sugg.len() == 1 { + let msg = format!("\ + trait `{}` provides `{item_name}` is implemented but not reachable", + sugg[0].trim() + ); + err.help(msg); + } else { + msg += &format!(" but {} not reachable", pluralize!("is", sugg.len())); + err.span_suggestions( + span, + msg, + sugg, + Applicability::MaybeIncorrect, + ); + } + }; + if accessible_sugg.is_empty() { + // `inaccessible_sugg` must not be empty + suggest_for_privacy(&mut err, help, inaccessible_sugg); + } else if inaccessible_sugg.is_empty() { + suggest_for_access(&mut err, help, accessible_sugg); + } else { + suggest_for_access(&mut err, help.clone(), accessible_sugg); + suggest_for_privacy(&mut err, help, inaccessible_sugg); + } + }, + ); } if let ty::Ref(region, t_type, mutability) = rcvr_ty.kind() { if needs_mut { @@ -3069,49 +3112,69 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - fn suggest_use_candidates(&self, err: &mut Diag<'_>, msg: String, candidates: Vec) { + fn suggest_use_candidates(&self, candidates: Vec, handle_candidates: F) + where + F: FnOnce(Vec, Vec, Span), + { let parent_map = self.tcx.visible_parent_map(()); - // Separate out candidates that must be imported with a glob, because they are named `_` - // and cannot be referred with their identifier. - let (candidates, globs): (Vec<_>, Vec<_>) = candidates.into_iter().partition(|trait_did| { - if let Some(parent_did) = parent_map.get(trait_did) { - // If the item is re-exported as `_`, we should suggest a glob-import instead. - if *parent_did != self.tcx.parent(*trait_did) - && self - .tcx - .module_children(*parent_did) - .iter() - .filter(|child| child.res.opt_def_id() == Some(*trait_did)) - .all(|child| child.ident.name == kw::Underscore) - { - return false; - } - } + let scope = self.tcx.parent_module_from_def_id(self.body_id); + let (accessible_candidates, inaccessible_candidates): (Vec<_>, Vec<_>) = + candidates.into_iter().partition(|id| { + let vis = self.tcx.visibility(*id); + vis.is_accessible_from(scope, self.tcx) + }); - true - }); + let sugg = |candidates: Vec<_>, visible| { + // Separate out candidates that must be imported with a glob, because they are named `_` + // and cannot be referred with their identifier. + let (candidates, globs): (Vec<_>, Vec<_>) = + candidates.into_iter().partition(|trait_did| { + if let Some(parent_did) = parent_map.get(trait_did) { + // If the item is re-exported as `_`, we should suggest a glob-import instead. + if *parent_did != self.tcx.parent(*trait_did) + && self + .tcx + .module_children(*parent_did) + .iter() + .filter(|child| child.res.opt_def_id() == Some(*trait_did)) + .all(|child| child.ident.name == kw::Underscore) + { + return false; + } + } - let module_did = self.tcx.parent_module_from_def_id(self.body_id); - let (module, _, _) = self.tcx.hir().get_module(module_did); - let span = module.spans.inject_use_span; + true + }); - let path_strings = candidates.iter().map(|trait_did| { - format!("use {};\n", with_crate_prefix!(self.tcx.def_path_str(*trait_did)),) - }); + let prefix = if visible { "use " } else { "" }; + let postfix = if visible { ";" } else { "" }; + let path_strings = candidates.iter().map(|trait_did| { + format!( + "{prefix}{}{postfix}\n", + with_crate_prefix!(self.tcx.def_path_str(*trait_did)), + ) + }); - let glob_path_strings = globs.iter().map(|trait_did| { - let parent_did = parent_map.get(trait_did).unwrap(); - format!( - "use {}::*; // trait {}\n", - with_crate_prefix!(self.tcx.def_path_str(*parent_did)), - self.tcx.item_name(*trait_did), - ) - }); - let mut sugg: Vec<_> = path_strings.chain(glob_path_strings).collect(); - sugg.sort(); + let glob_path_strings = globs.iter().map(|trait_did| { + let parent_did = parent_map.get(trait_did).unwrap(); + format!( + "{prefix}{}::*{postfix} // trait {}\n", + with_crate_prefix!(self.tcx.def_path_str(*parent_did)), + self.tcx.item_name(*trait_did), + ) + }); + let mut sugg: Vec<_> = path_strings.chain(glob_path_strings).collect(); + sugg.sort(); + sugg + }; - err.span_suggestions(span, msg, sugg, Applicability::MaybeIncorrect); + let accessible_sugg = sugg(accessible_candidates, true); + let inaccessible_sugg = sugg(inaccessible_candidates, false); + + let (module, _, _) = self.tcx.hir().get_module(scope); + let span = module.spans.inject_use_span; + handle_candidates(accessible_sugg, inaccessible_sugg, span); } fn suggest_valid_traits( @@ -3135,9 +3198,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if explain { err.help("items from traits can only be used if the trait is in scope"); } + let msg = format!( - "{this_trait_is} implemented but not in scope; perhaps you want to import \ - {one_of_them}", + "{this_trait_is} implemented but not in scope", this_trait_is = if candidates.len() == 1 { format!( "trait `{}` which provides `{item_name}` is", @@ -3145,11 +3208,43 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) } else { format!("the following traits which provide `{item_name}` are") - }, - one_of_them = if candidates.len() == 1 { "it" } else { "one of them" }, + } ); - self.suggest_use_candidates(err, msg, candidates); + self.suggest_use_candidates(candidates, |accessible_sugg, inaccessible_sugg, span| { + let suggest_for_access = |err: &mut Diag<'_>, mut msg: String, sugg: Vec<_>| { + msg += &format!( + "; perhaps you want to import {one_of}", + one_of = if sugg.len() == 1 { "it" } else { "one of them" }, + ); + err.span_suggestions(span, msg, sugg, Applicability::MaybeIncorrect); + }; + let suggest_for_privacy = |err: &mut Diag<'_>, sugg: Vec| { + let msg = format!( + "{this_trait_is} implemented but not reachable", + this_trait_is = if sugg.len() == 1 { + format!("trait `{}` which provides `{item_name}` is", sugg[0].trim()) + } else { + format!("the following traits which provide `{item_name}` are") + } + ); + if sugg.len() == 1 { + err.help(msg); + } else { + err.span_suggestions(span, msg, sugg, Applicability::MaybeIncorrect); + } + }; + if accessible_sugg.is_empty() { + // `inaccessible_sugg` must not be empty + suggest_for_privacy(err, inaccessible_sugg); + } else if inaccessible_sugg.is_empty() { + suggest_for_access(err, msg, accessible_sugg); + } else { + suggest_for_access(err, msg, accessible_sugg); + suggest_for_privacy(err, inaccessible_sugg); + } + }); + if let Some(did) = edition_fix { err.note(format!( "'{}' is included in the prelude starting in Edition 2021", diff --git a/tests/ui/traits/item-privacy.stderr b/tests/ui/traits/item-privacy.stderr index d08bb4745bf57..fca7473f3bc30 100644 --- a/tests/ui/traits/item-privacy.stderr +++ b/tests/ui/traits/item-privacy.stderr @@ -8,10 +8,7 @@ LL | S.a(); | ^ | = help: items from traits can only be used if the trait is implemented and in scope -help: trait `A` which provides `a` is implemented but not in scope; perhaps you want to import it - | -LL + use method::A; - | + = help: trait `method::A` which provides `a` is implemented but not reachable help: there is a method `b` with a similar name | LL | S.b(); @@ -58,15 +55,12 @@ LL | S::a(&S); | ^ function or associated item not found in `S` | = help: items from traits can only be used if the trait is implemented and in scope + = help: trait `method::A` which provides `a` is implemented but not reachable help: there is an associated constant `B` with a similar name --> $DIR/item-privacy.rs:29:9 | LL | const B: u8 = 0; | ^^^^^^^^^^^ -help: trait `A` which provides `a` is implemented but not in scope; perhaps you want to import it - | -LL + use method::A; - | error[E0599]: no function or associated item named `b` found for struct `S` in the current scope --> $DIR/item-privacy.rs:80:8 @@ -107,10 +101,7 @@ LL | S::A; | ^ associated item not found in `S` | = help: items from traits can only be used if the trait is implemented and in scope -help: trait `A` which provides `A` is implemented but not in scope; perhaps you want to import it - | -LL + use assoc_const::A; - | + = help: trait `assoc_const::A` which provides `A` is implemented but not reachable help: there is an associated constant `B` with a similar name | LL | S::B; From 2752a374c759f70dd879989b88f8b891e14d646b Mon Sep 17 00:00:00 2001 From: Oneirical Date: Tue, 11 Jun 2024 14:18:27 -0400 Subject: [PATCH 03/14] rewrite intrinsic-unreachable to rmake --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/intrinsic-unreachable/Makefile | 12 ------------ tests/run-make/intrinsic-unreachable/rmake.rs | 19 +++++++++++++++++++ 3 files changed, 19 insertions(+), 13 deletions(-) delete mode 100644 tests/run-make/intrinsic-unreachable/Makefile create mode 100644 tests/run-make/intrinsic-unreachable/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index be9df226d64e8..45b571f4db6d9 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -72,7 +72,6 @@ run-make/incremental-debugger-visualizer/Makefile run-make/incremental-session-fail/Makefile run-make/inline-always-many-cgu/Makefile run-make/interdependent-c-libraries/Makefile -run-make/intrinsic-unreachable/Makefile run-make/invalid-library/Makefile run-make/invalid-so/Makefile run-make/invalid-staticlib/Makefile diff --git a/tests/run-make/intrinsic-unreachable/Makefile b/tests/run-make/intrinsic-unreachable/Makefile deleted file mode 100644 index ff9cc57098c6b..0000000000000 --- a/tests/run-make/intrinsic-unreachable/Makefile +++ /dev/null @@ -1,12 +0,0 @@ -include ../tools.mk - -# needs-asm-support -# ignore-windows-msvc -# -# Because of Windows exception handling, the code is not necessarily any shorter. -# https://github.com/llvm-mirror/llvm/commit/64b2297786f7fd6f5fa24cdd4db0298fbf211466 - -all: - $(RUSTC) -O --emit asm exit-ret.rs - $(RUSTC) -O --emit asm exit-unreachable.rs - test `wc -l < $(TMPDIR)/exit-unreachable.s` -lt `wc -l < $(TMPDIR)/exit-ret.s` diff --git a/tests/run-make/intrinsic-unreachable/rmake.rs b/tests/run-make/intrinsic-unreachable/rmake.rs new file mode 100644 index 0000000000000..6c951147604a1 --- /dev/null +++ b/tests/run-make/intrinsic-unreachable/rmake.rs @@ -0,0 +1,19 @@ +// intrinsics::unreachable tells the compiler that a certain point in the code +// is not reachable by any means, which enables some useful optimizations. +// In this test, exit-unreachable contains this instruction and exit-ret does not, +// which means the emitted artifacts should be shorter in length. +// See https://github.com/rust-lang/rust/pull/16970 + +//@ needs-asm-support +//@ ignore-windows-msvc +// Reason: Because of Windows exception handling, the code is not necessarily any shorter. + +use run_make_support::rustc; +use std::io::BufReader; +use std::fs::File; + +fn main() { + rustc().opt().emit("asm").input("exit-ret.rs").run(); + rustc().opt().emit("asm").input("exit-unreachable.rs").run(); + assert!(BufReader::new(File::open("exit-unreachable.s")).lines().count() < BufReader::new(File::open("exit-ret.s")).lines().count()); +} From 76c7a67c722142c6286d506f361892152354cfca Mon Sep 17 00:00:00 2001 From: Oneirical Date: Wed, 12 Jun 2024 11:24:21 -0400 Subject: [PATCH 04/14] rewrite sepcomp-separate to rmake --- Cargo.lock | 1 + src/tools/run-make-support/Cargo.toml | 1 + src/tools/run-make-support/src/fs_wrapper.rs | 27 ++++++++++++------- src/tools/run-make-support/src/lib.rs | 1 + .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/intrinsic-unreachable/rmake.rs | 13 +++++---- tests/run-make/sepcomp-separate/Makefile | 9 ------- tests/run-make/sepcomp-separate/rmake.rs | 24 +++++++++++++++++ 8 files changed, 52 insertions(+), 25 deletions(-) delete mode 100644 tests/run-make/sepcomp-separate/Makefile create mode 100644 tests/run-make/sepcomp-separate/rmake.rs diff --git a/Cargo.lock b/Cargo.lock index 699e5ebf754fb..e419c0e9a46d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3461,6 +3461,7 @@ name = "run_make_support" version = "0.2.0" dependencies = [ "gimli 0.28.1", + "glob", "object 0.34.0", "regex", "similar", diff --git a/src/tools/run-make-support/Cargo.toml b/src/tools/run-make-support/Cargo.toml index 2f7f51442f164..4a244252c84d8 100644 --- a/src/tools/run-make-support/Cargo.toml +++ b/src/tools/run-make-support/Cargo.toml @@ -9,3 +9,4 @@ similar = "2.5.0" wasmparser = "0.118.2" regex = "1.8" # 1.8 to avoid memchr 2.6.0, as 2.5.0 is pinned in the workspace gimli = "0.28.1" +glob = "0.3.1" diff --git a/src/tools/run-make-support/src/fs_wrapper.rs b/src/tools/run-make-support/src/fs_wrapper.rs index 8a2bfce8b4a72..d65c10b17c991 100644 --- a/src/tools/run-make-support/src/fs_wrapper.rs +++ b/src/tools/run-make-support/src/fs_wrapper.rs @@ -1,7 +1,7 @@ use std::fs; use std::path::Path; -/// A wrapper around [`std::fs::remove_file`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::remove_file`] which includes the file path in the panic message. #[track_caller] pub fn remove_file>(path: P) { fs::remove_file(path.as_ref()) @@ -18,21 +18,28 @@ pub fn copy, Q: AsRef>(from: P, to: Q) { )); } -/// A wrapper around [`std::fs::File::create`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::File::create`] which includes the file path in the panic message. #[track_caller] pub fn create_file>(path: P) { fs::File::create(path.as_ref()) .expect(&format!("the file in path \"{}\" could not be created", path.as_ref().display())); } -/// A wrapper around [`std::fs::read`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::File::open`] which includes the file path in the panic message. +#[track_caller] +pub fn open_file>(path: P) -> fs::File { + fs::File::open(path.as_ref()) + .expect(&format!("the file in path \"{}\" could not be opened", path.as_ref().display())) +} + +/// A wrapper around [`std::fs::read`] which includes the file path in the panic message. #[track_caller] pub fn read>(path: P) -> Vec { fs::read(path.as_ref()) .expect(&format!("the file in path \"{}\" could not be read", path.as_ref().display())) } -/// A wrapper around [`std::fs::read_to_string`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::read_to_string`] which includes the file path in the panic message. #[track_caller] pub fn read_to_string>(path: P) -> String { fs::read_to_string(path.as_ref()).expect(&format!( @@ -41,14 +48,14 @@ pub fn read_to_string>(path: P) -> String { )) } -/// A wrapper around [`std::fs::read_dir`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::read_dir`] which includes the file path in the panic message. #[track_caller] pub fn read_dir>(path: P) -> fs::ReadDir { fs::read_dir(path.as_ref()) .expect(&format!("the directory in path \"{}\" could not be read", path.as_ref().display())) } -/// A wrapper around [`std::fs::write`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::write`] which includes the file path in the panic message. #[track_caller] pub fn write, C: AsRef<[u8]>>(path: P, contents: C) { fs::write(path.as_ref(), contents.as_ref()).expect(&format!( @@ -57,7 +64,7 @@ pub fn write, C: AsRef<[u8]>>(path: P, contents: C) { )); } -/// A wrapper around [`std::fs::remove_dir_all`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::remove_dir_all`] which includes the file path in the panic message. #[track_caller] pub fn remove_dir_all>(path: P) { fs::remove_dir_all(path.as_ref()).expect(&format!( @@ -66,7 +73,7 @@ pub fn remove_dir_all>(path: P) { )); } -/// A wrapper around [`std::fs::create_dir`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::create_dir`] which includes the file path in the panic message. #[track_caller] pub fn create_dir>(path: P) { fs::create_dir(path.as_ref()).expect(&format!( @@ -75,7 +82,7 @@ pub fn create_dir>(path: P) { )); } -/// A wrapper around [`std::fs::create_dir_all`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::create_dir_all`] which includes the file path in the panic message. #[track_caller] pub fn create_dir_all>(path: P) { fs::create_dir_all(path.as_ref()).expect(&format!( @@ -84,7 +91,7 @@ pub fn create_dir_all>(path: P) { )); } -/// A wrapper around [`std::fs::metadata`] which includes the file path in the panic message.. +/// A wrapper around [`std::fs::metadata`] which includes the file path in the panic message. #[track_caller] pub fn metadata>(path: P) -> fs::Metadata { fs::metadata(path.as_ref()).expect(&format!( diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index ba4524c150c46..a5dacfd7a6942 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -22,6 +22,7 @@ use std::panic; use std::path::{Path, PathBuf}; pub use gimli; +pub use glob; pub use object; pub use regex; pub use wasmparser; diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 45b571f4db6d9..f858874313c44 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -208,7 +208,6 @@ run-make/separate-link-fail/Makefile run-make/separate-link/Makefile run-make/sepcomp-cci-copies/Makefile run-make/sepcomp-inlining/Makefile -run-make/sepcomp-separate/Makefile run-make/share-generics-dylib/Makefile run-make/silly-file-names/Makefile run-make/simd-ffi/Makefile diff --git a/tests/run-make/intrinsic-unreachable/rmake.rs b/tests/run-make/intrinsic-unreachable/rmake.rs index 6c951147604a1..5e62a966c5476 100644 --- a/tests/run-make/intrinsic-unreachable/rmake.rs +++ b/tests/run-make/intrinsic-unreachable/rmake.rs @@ -5,15 +5,18 @@ // See https://github.com/rust-lang/rust/pull/16970 //@ needs-asm-support -//@ ignore-windows-msvc +//@ ignore-windows // Reason: Because of Windows exception handling, the code is not necessarily any shorter. -use run_make_support::rustc; -use std::io::BufReader; -use std::fs::File; +use run_make_support::{fs_wrapper, rustc}; +use std::io::{BufRead, BufReader}; fn main() { rustc().opt().emit("asm").input("exit-ret.rs").run(); rustc().opt().emit("asm").input("exit-unreachable.rs").run(); - assert!(BufReader::new(File::open("exit-unreachable.s")).lines().count() < BufReader::new(File::open("exit-ret.s")).lines().count()); + let unreachable_file = fs_wrapper::open_file("exit-unreachable.s"); + let ret_file = fs_wrapper::open_file("exit-ret.s"); + assert!( + BufReader::new(unreachable_file).lines().count() < BufReader::new(ret_file).lines().count() + ); } diff --git a/tests/run-make/sepcomp-separate/Makefile b/tests/run-make/sepcomp-separate/Makefile deleted file mode 100644 index 62cf54a88fbe9..0000000000000 --- a/tests/run-make/sepcomp-separate/Makefile +++ /dev/null @@ -1,9 +0,0 @@ -include ../tools.mk - -# Test that separate compilation actually puts code into separate compilation -# units. `foo.rs` defines `magic_fn` in three different modules, which should -# wind up in three different compilation units. - -all: - $(RUSTC) foo.rs --emit=llvm-ir -C codegen-units=3 - [ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c define\ .*magic_fn)" -eq "3" ] diff --git a/tests/run-make/sepcomp-separate/rmake.rs b/tests/run-make/sepcomp-separate/rmake.rs new file mode 100644 index 0000000000000..d3ef2450a847d --- /dev/null +++ b/tests/run-make/sepcomp-separate/rmake.rs @@ -0,0 +1,24 @@ +// Test that separate compilation actually puts code into separate compilation +// units. `foo.rs` defines `magic_fn` in three different modules, which should +// wind up in three different compilation units. +// See https://github.com/rust-lang/rust/pull/16367 + +use run_make_support::{fs_wrapper, glob, regex, rustc}; +use std::io::{BufRead, BufReader}; + +fn main() { + let mut match_count = 0; + rustc().input("foo.rs").emit("llvm-ir").codegen_units(3).run(); + let re = regex::Regex::new(r#"define.*magic_fn"#).unwrap(); + let paths = glob::glob("foo.*.ll").unwrap(); + paths.filter_map(|entry| entry.ok()).filter(|path| path.is_file()).for_each(|path| { + let file = fs_wrapper::open_file(path); + let reader = BufReader::new(file); + reader + .lines() + .filter_map(|line| line.ok()) + .filter(|line| re.is_match(line)) + .for_each(|_| match_count += 1); + }); + assert_eq!(match_count, 3); +} From 348c183df92fb5b90f2e54d692b6fd000e25148d Mon Sep 17 00:00:00 2001 From: Oneirical Date: Thu, 13 Jun 2024 15:02:08 -0400 Subject: [PATCH 05/14] rewrite sepcomp-inlining and -separate to rmake.rs --- src/tools/run-make-support/src/lib.rs | 17 +++++++++++++++++ .../tidy/src/allowed_run_make_makefiles.txt | 2 -- tests/run-make/sepcomp-cci-copies/Makefile | 12 ------------ tests/run-make/sepcomp-cci-copies/rmake.rs | 13 +++++++++++++ tests/run-make/sepcomp-inlining/Makefile | 15 --------------- tests/run-make/sepcomp-inlining/rmake.rs | 19 +++++++++++++++++++ tests/run-make/sepcomp-separate/rmake.rs | 17 ++--------------- 7 files changed, 51 insertions(+), 44 deletions(-) delete mode 100644 tests/run-make/sepcomp-cci-copies/Makefile create mode 100644 tests/run-make/sepcomp-cci-copies/rmake.rs delete mode 100644 tests/run-make/sepcomp-inlining/Makefile create mode 100644 tests/run-make/sepcomp-inlining/rmake.rs diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index a5dacfd7a6942..17b62bc8dbbc9 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -245,6 +245,23 @@ pub fn uname() -> String { output.stdout_utf8() } +/// Inside a glob pattern of files (paths), read their contents and count the +/// number of regex matches with a given expression (re). +#[track_caller] +pub fn count_regex_matches_in_file_glob(re: &str, paths: &str) -> usize { + let re = regex::Regex::new(re).expect(format!("Regex expression {re} is not valid.").as_str()); + let paths = glob::glob(paths).expect(format!("Glob expression {paths} is not valid.").as_str()); + use io::BufRead; + paths + .filter_map(|entry| entry.ok()) + .filter(|entry| entry.as_path().is_file()) + .filter_map(|path| fs::File::open(&path).ok()) + .map(|file| io::BufReader::new(file)) + .flat_map(|reader| reader.lines().filter_map(|entry| entry.ok())) + .filter(|line| re.is_match(line)) + .count() +} + fn handle_failed_output(cmd: &Command, output: CompletedProcess, caller_line_number: u32) -> ! { if output.status().success() { eprintln!("command unexpectedly succeeded at line {caller_line_number}"); diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index f858874313c44..d77741ceb3add 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -206,8 +206,6 @@ run-make/sanitizer-dylib-link/Makefile run-make/sanitizer-staticlib-link/Makefile run-make/separate-link-fail/Makefile run-make/separate-link/Makefile -run-make/sepcomp-cci-copies/Makefile -run-make/sepcomp-inlining/Makefile run-make/share-generics-dylib/Makefile run-make/silly-file-names/Makefile run-make/simd-ffi/Makefile diff --git a/tests/run-make/sepcomp-cci-copies/Makefile b/tests/run-make/sepcomp-cci-copies/Makefile deleted file mode 100644 index df289d0b0b1a2..0000000000000 --- a/tests/run-make/sepcomp-cci-copies/Makefile +++ /dev/null @@ -1,12 +0,0 @@ -include ../tools.mk - -# Check that cross-crate inlined items are inlined in all compilation units -# that refer to them, and not in any other compilation units. -# Note that we have to pass `-C codegen-units=6` because up to two CGUs may be -# created for each source module (see `rustc_const_eval::monomorphize::partitioning`). - -all: - $(RUSTC) cci_lib.rs - $(RUSTC) foo.rs --emit=llvm-ir -C codegen-units=6 \ - -Z inline-in-all-cgus - [ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c define\ .*cci_fn)" -eq "2" ] diff --git a/tests/run-make/sepcomp-cci-copies/rmake.rs b/tests/run-make/sepcomp-cci-copies/rmake.rs new file mode 100644 index 0000000000000..8d2411c1faec4 --- /dev/null +++ b/tests/run-make/sepcomp-cci-copies/rmake.rs @@ -0,0 +1,13 @@ +// Check that cross-crate inlined items are inlined in all compilation units +// that refer to them, and not in any other compilation units. +// Note that we have to pass `-C codegen-units=6` because up to two CGUs may be +// created for each source module (see `rustc_const_eval::monomorphize::partitioning`). +// See https://github.com/rust-lang/rust/pull/16367 + +use run_make_support::{count_regex_matches_in_file_glob, rustc}; + +fn main() { + rustc().input("cci_lib.rs").run(); + rustc().input("foo.rs").emit("llvm-ir").codegen_units(6).arg("-Zinline-in-all-cgus").run(); + assert_eq!(count_regex_matches_in_file_glob(r#"define\ .*cci_fn"#, "foo.*.ll"), 2); +} diff --git a/tests/run-make/sepcomp-inlining/Makefile b/tests/run-make/sepcomp-inlining/Makefile deleted file mode 100644 index 327aeb75e5eda..0000000000000 --- a/tests/run-make/sepcomp-inlining/Makefile +++ /dev/null @@ -1,15 +0,0 @@ -include ../tools.mk - -# Test that #[inline] functions still get inlined across compilation unit -# boundaries. Compilation should produce three IR files, but only the two -# compilation units that have a usage of the #[inline] function should -# contain a definition. Also, the non-#[inline] function should be defined -# in only one compilation unit. - -all: - $(RUSTC) foo.rs --emit=llvm-ir -C codegen-units=3 \ - -Z inline-in-all-cgus - [ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c define\ i32\ .*inlined)" -eq "0" ] - [ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c define\ internal\ i32\ .*inlined)" -eq "2" ] - [ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c define\ hidden\ i32\ .*normal)" -eq "1" ] - [ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c declare\ hidden\ i32\ .*normal)" -eq "2" ] diff --git a/tests/run-make/sepcomp-inlining/rmake.rs b/tests/run-make/sepcomp-inlining/rmake.rs new file mode 100644 index 0000000000000..0315fd6997204 --- /dev/null +++ b/tests/run-make/sepcomp-inlining/rmake.rs @@ -0,0 +1,19 @@ +// Test that #[inline] functions still get inlined across compilation unit +// boundaries. Compilation should produce three IR files, but only the two +// compilation units that have a usage of the #[inline] function should +// contain a definition. Also, the non-#[inline] function should be defined +// in only one compilation unit. +// See https://github.com/rust-lang/rust/pull/16367 + +use run_make_support::{count_regex_matches_in_file_glob, glob, regex, rustc}; + +fn main() { + rustc().input("foo.rs").emit("llvm-ir").codegen_units(3).arg("-Zinline-in-all-cgus").run(); + assert_eq!(count_regex_matches_in_file_glob(r#"define\ i32\ .*inlined"#, "foo.*.ll"), 0); + assert_eq!(count_regex_matches_in_file_glob(r#"define\ internal\ .*inlined"#, "foo.*.ll"), 2); + assert_eq!(count_regex_matches_in_file_glob(r#"define\ hidden\ i32\ .*normal"#, "foo.*.ll"), 1); + assert_eq!( + count_regex_matches_in_file_glob(r#"declare\ hidden\ i32\ .*normal"#, "foo.*.ll"), + 2 + ); +} diff --git a/tests/run-make/sepcomp-separate/rmake.rs b/tests/run-make/sepcomp-separate/rmake.rs index d3ef2450a847d..99ee221adc1b6 100644 --- a/tests/run-make/sepcomp-separate/rmake.rs +++ b/tests/run-make/sepcomp-separate/rmake.rs @@ -3,22 +3,9 @@ // wind up in three different compilation units. // See https://github.com/rust-lang/rust/pull/16367 -use run_make_support::{fs_wrapper, glob, regex, rustc}; -use std::io::{BufRead, BufReader}; +use run_make_support::{count_regex_matches_in_file_glob, rustc}; fn main() { - let mut match_count = 0; rustc().input("foo.rs").emit("llvm-ir").codegen_units(3).run(); - let re = regex::Regex::new(r#"define.*magic_fn"#).unwrap(); - let paths = glob::glob("foo.*.ll").unwrap(); - paths.filter_map(|entry| entry.ok()).filter(|path| path.is_file()).for_each(|path| { - let file = fs_wrapper::open_file(path); - let reader = BufReader::new(file); - reader - .lines() - .filter_map(|line| line.ok()) - .filter(|line| re.is_match(line)) - .for_each(|_| match_count += 1); - }); - assert_eq!(match_count, 3); + assert_eq!(count_regex_matches_in_file_glob(r#"define\ .*magic_fn"#, "foo.*.ll"), 3); } From df1d6168f4deef4954f3b74a23fe1e923d9b95b6 Mon Sep 17 00:00:00 2001 From: Jack Wrenn Date: Fri, 14 Jun 2024 17:02:07 +0000 Subject: [PATCH 06/14] safe transmute: support non-ZST, variantful, uninhabited enums Previously, `Tree::from_enum`'s implementation branched into three disjoint cases: 1. enums that uninhabited 2. enums for which all but one variant is uninhabited 3. enums with multiple inhabited variants This branching (incorrectly) did not differentiate between variantful and variantless uninhabited enums. In both cases, we assumed (and asserted) that uninhabited enums are zero-sized types. This assumption is false for enums like: enum Uninhabited { A(!, u128) } ...which, currently, has the same size as `u128`. This faulty assumption manifested as the ICE reported in #126460. In this PR, we revise the first case of `Tree::from_enum` to consider only the narrow category of "enums that are uninhabited ZSTs". These enums, whose layouts are described with `Variants::Single { index }`, are special in their layouts otherwise resemble the `!` type and cannot be descended into like typical enums. This first case captures uninhabited enums like: enum Uninhabited { A(!, !), B(!) } The second case is revised to consider the broader category of "enums that defer their layout to one of their variants"; i.e., enums whose layouts are described with `Variants::Single { index }` and that do have a variant at `index`. This second case captures uninhabited enums that are not ZSTs, like: enum Uninhabited { A(!, u128) } ...which represent their variants with `Variants::Single`. Finally, the third case is revised to cover the broader category of "enums with multiple variants", which captures uninhabited, non-ZST enums like: enum Uninhabited { A(u8, !), B(!, u32) } ...which represent their variants with `Variants::Multiple`. This PR also adds a comment requested by RalfJung in his review of #126358 to `compiler/rustc_const_eval/src/interpret/discriminant.rs`. Fixes #126460 --- .../src/interpret/discriminant.rs | 11 ++++- compiler/rustc_transmute/src/layout/tree.rs | 36 +++++++--------- .../enums/uninhabited_optimization.rs | 6 +++ tests/ui/transmutability/uninhabited.rs | 24 ++++++++++- tests/ui/transmutability/uninhabited.stderr | 42 ++++++++++++++++--- 5 files changed, 89 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/discriminant.rs b/compiler/rustc_const_eval/src/interpret/discriminant.rs index a50b50d231d78..b3a139d553ade 100644 --- a/compiler/rustc_const_eval/src/interpret/discriminant.rs +++ b/compiler/rustc_const_eval/src/interpret/discriminant.rs @@ -241,7 +241,16 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { variant_index: VariantIdx, ) -> InterpResult<'tcx, Option<(ScalarInt, usize)>> { match self.layout_of(ty)?.variants { - abi::Variants::Single { .. } => Ok(None), + abi::Variants::Single { .. } => { + // The tag of a `Single` enum is like the tag of the niched + // variant: there's no tag as the discriminant is encoded + // entirely implicitly. If `write_discriminant` ever hits this + // case, we do a "validation read" to ensure the the right + // discriminant is encoded implicitly, so any attempt to write + // the wrong discriminant for a `Single` enum will reliably + // result in UB. + Ok(None) + } abi::Variants::Multiple { tag_encoding: TagEncoding::Direct, diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs index 241381f5875ed..865f9487213f8 100644 --- a/compiler/rustc_transmute/src/layout/tree.rs +++ b/compiler/rustc_transmute/src/layout/tree.rs @@ -341,37 +341,29 @@ pub(crate) mod rustc { // We consider three kinds of enums, each demanding a different // treatment of their layout computation: - // 1. enums that are uninhabited - // 2. enums for which all but one variant is uninhabited - // 3. enums with multiple inhabited variants + // 1. enums that are uninhabited ZSTs + // 2. enums that delegate their layout to a variant + // 3. enums with multiple variants match layout.variants() { - _ if layout.abi.is_uninhabited() => { - // Uninhabited enums are usually (always?) zero-sized. In - // the (unlikely?) event that an uninhabited enum is - // non-zero-sized, this assert will trigger an ICE, and this - // code should be modified such that a `layout.size` amount - // of uninhabited bytes is returned instead. - // - // Uninhabited enums are currently implemented such that - // their layout is described with `Variants::Single`, even - // though they don't necessarily have a 'single' variant to - // defer to. That said, we don't bother specifically - // matching on `Variants::Single` in this arm because the - // behavioral principles here remain true even if, for - // whatever reason, the compiler describes an uninhabited - // enum with `Variants::Multiple`. - assert_eq!(layout.size, Size::ZERO); + Variants::Single { .. } + if layout.abi.is_uninhabited() && layout.size == Size::ZERO => + { + // The layout representation of uninhabited, ZST enums is + // defined to be like that of the `!` type, as opposed of a + // typical enum. Consequently, they cannot be descended into + // as if they typical enums. We therefore special-case this + // scenario and simply return an uninhabited `Tree`. Ok(Self::uninhabited()) } Variants::Single { index } => { - // `Variants::Single` on non-uninhabited enums denotes that + // `Variants::Single` on enums with variants denotes that // the enum delegates its layout to the variant at `index`. layout_of_variant(*index) } Variants::Multiple { tag_field, .. } => { // `Variants::Multiple` denotes an enum with multiple - // inhabited variants. The layout of such an enum is the - // disjunction of the layouts of its tagged variants. + // variants. The layout of such an enum is the disjunction + // of the layouts of its tagged variants. // For enums (but not coroutines), the tag field is // currently always the first field of the layout. diff --git a/tests/ui/transmutability/enums/uninhabited_optimization.rs b/tests/ui/transmutability/enums/uninhabited_optimization.rs index 04a8eb40c8b8d..c2d5b67ab2ce4 100644 --- a/tests/ui/transmutability/enums/uninhabited_optimization.rs +++ b/tests/ui/transmutability/enums/uninhabited_optimization.rs @@ -19,8 +19,14 @@ enum SingleUninhabited { Y(Uninhabited), } +enum MultipleUninhabited { + X(u8, Uninhabited), + Y(Uninhabited, u16), +} + fn main() { assert_transmutable::(); assert_transmutable::(); assert_transmutable::(); + assert_transmutable::(); } diff --git a/tests/ui/transmutability/uninhabited.rs b/tests/ui/transmutability/uninhabited.rs index b61b110f6a11c..7524922c16a7e 100644 --- a/tests/ui/transmutability/uninhabited.rs +++ b/tests/ui/transmutability/uninhabited.rs @@ -30,7 +30,7 @@ fn void() { } // Non-ZST uninhabited types are, nonetheless, uninhabited. -fn yawning_void() { +fn yawning_void_struct() { enum Void {} struct YawningVoid(Void, u128); @@ -49,6 +49,28 @@ fn yawning_void() { assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted } +// Non-ZST uninhabited types are, nonetheless, uninhabited. +fn yawning_void_enum() { + enum Void {} + + enum YawningVoid { + A(Void, u128), + } + + const _: () = { + assert!(std::mem::size_of::() == std::mem::size_of::()); + // Just to be sure the above constant actually evaluated: + assert!(false); //~ ERROR: evaluation of constant value failed + }; + + // This transmutation is vacuously acceptable; since one cannot construct a + // `Void`, unsoundness cannot directly arise from transmuting a void into + // anything else. + assert::is_maybe_transmutable::(); + + assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted +} + // References to uninhabited types are, logically, uninhabited, but for layout // purposes are not ZSTs, and aren't treated as uninhabited when they appear in // enum variants. diff --git a/tests/ui/transmutability/uninhabited.stderr b/tests/ui/transmutability/uninhabited.stderr index 60219b0f263c6..88a98c798fc3d 100644 --- a/tests/ui/transmutability/uninhabited.stderr +++ b/tests/ui/transmutability/uninhabited.stderr @@ -7,10 +7,18 @@ LL | assert!(false); = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0080]: evaluation of constant value failed - --> $DIR/uninhabited.rs:65:9 + --> $DIR/uninhabited.rs:63:9 | LL | assert!(false); - | ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:65:9 + | ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:63:9 + | + = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0080]: evaluation of constant value failed + --> $DIR/uninhabited.rs:87:9 + | +LL | assert!(false); + | ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:87:9 | = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) @@ -36,11 +44,33 @@ LL | | } LL | | }> | |__________^ required by this bound in `is_maybe_transmutable` -error[E0277]: `()` cannot be safely transmuted into `yawning_void::Void` +error[E0277]: `()` cannot be safely transmuted into `yawning_void_struct::Void` --> $DIR/uninhabited.rs:49:41 | LL | assert::is_maybe_transmutable::<(), Void>(); - | ^^^^ `yawning_void::Void` is uninhabited + | ^^^^ `yawning_void_struct::Void` is uninhabited + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/uninhabited.rs:10:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error[E0277]: `()` cannot be safely transmuted into `yawning_void_enum::Void` + --> $DIR/uninhabited.rs:71:41 + | +LL | assert::is_maybe_transmutable::<(), Void>(); + | ^^^^ `yawning_void_enum::Void` is uninhabited | note: required by a bound in `is_maybe_transmutable` --> $DIR/uninhabited.rs:10:14 @@ -59,7 +89,7 @@ LL | | }> | |__________^ required by this bound in `is_maybe_transmutable` error[E0277]: `u128` cannot be safely transmuted into `DistantVoid` - --> $DIR/uninhabited.rs:70:43 + --> $DIR/uninhabited.rs:92:43 | LL | assert::is_maybe_transmutable::(); | ^^^^^^^^^^^ at least one value of `u128` isn't a bit-valid value of `DistantVoid` @@ -80,7 +110,7 @@ LL | | } LL | | }> | |__________^ required by this bound in `is_maybe_transmutable` -error: aborting due to 5 previous errors +error: aborting due to 7 previous errors Some errors have detailed explanations: E0080, E0277. For more information about an error, try `rustc --explain E0080`. From ab0e72781f6bccb1c033a7c79edaebc6124ac2b5 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 13 Jun 2024 17:08:04 +0200 Subject: [PATCH 07/14] Suggest standalone doctest for non-local impl defs --- compiler/rustc_lint/messages.ftl | 1 + compiler/rustc_lint/src/lints.rs | 5 ++ compiler/rustc_lint/src/non_local_def.rs | 84 ++++++++++--------- .../rustdoc-ui/doctest/auxiliary/pub_trait.rs | 1 + .../rustdoc-ui/doctest/non-local-defs-impl.rs | 31 +++++++ .../doctest/non-local-defs-impl.stdout | 37 ++++++++ 6 files changed, 121 insertions(+), 38 deletions(-) create mode 100644 tests/rustdoc-ui/doctest/auxiliary/pub_trait.rs create mode 100644 tests/rustdoc-ui/doctest/non-local-defs-impl.rs create mode 100644 tests/rustdoc-ui/doctest/non-local-defs-impl.stdout diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 733c73bc3d078..7430f1fc13ae7 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -548,6 +548,7 @@ lint_non_local_definitions_impl = non-local `impl` definition, `impl` blocks sho .without_trait = methods and associated constants are still usable outside the current expression, only `impl Local` and `impl dyn Local` can ever be private, and only if the type is nested in the same item as the `impl` .with_trait = an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl` .bounds = `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type + .doctest = make this doc-test a standalone test with its own `fn main() {"{"} ... {"}"}` .exception = items in an anonymous const item (`const _: () = {"{"} ... {"}"}`) are treated as in the same scope as the anonymous const's declaration .const_anon = use a const-anon item to suppress this lint diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index b377da31a581b..6cc0a81aa4f67 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1358,6 +1358,7 @@ pub enum NonLocalDefinitionsDiag { cargo_update: Option, const_anon: Option>, move_to: Option<(Span, Vec)>, + doctest: bool, may_remove: Option<(Span, String)>, has_trait: bool, self_ty_str: String, @@ -1383,6 +1384,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag { cargo_update, const_anon, move_to, + doctest, may_remove, has_trait, self_ty_str, @@ -1411,6 +1413,9 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag { } diag.span_help(ms, fluent::lint_non_local_definitions_impl_move_help); } + if doctest { + diag.help(fluent::lint_doctest); + } if let Some((span, part)) = may_remove { diag.arg("may_remove_part", part); diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index d7ffc34d824fd..423862dcdbae9 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -111,6 +111,12 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { } }; + // determining if we are in a doctest context can't currently be determined + // by the code itself (there are no specific attributes), but fortunately rustdoc + // sets a perma-unstable env var for libtest so we just reuse that for now + let is_at_toplevel_doctest = + || self.body_depth == 2 && std::env::var("UNSTABLE_RUSTDOC_TEST_PATH").is_ok(); + match item.kind { ItemKind::Impl(impl_) => { // The RFC states: @@ -191,29 +197,6 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { None }; - let mut collector = PathCollector { paths: Vec::new() }; - collector.visit_ty(&impl_.self_ty); - if let Some(of_trait) = &impl_.of_trait { - collector.visit_trait_ref(of_trait); - } - collector.visit_generics(&impl_.generics); - - let mut may_move: Vec = collector - .paths - .into_iter() - .filter_map(|path| { - if let Some(did) = path.res.opt_def_id() - && did_has_local_parent(did, cx.tcx, parent, parent_parent) - { - Some(cx.tcx.def_span(did)) - } else { - None - } - }) - .collect(); - may_move.sort(); - may_move.dedup(); - let const_anon = matches!(parent_def_kind, DefKind::Const | DefKind::Static { .. }) .then_some(span_for_const_anon_suggestion); @@ -248,14 +231,44 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { } else { None }; - let move_to = if may_move.is_empty() { - ms.push_span_label( - cx.tcx.def_span(parent), - fluent::lint_non_local_definitions_impl_move_help, - ); - None + + let (doctest, move_to) = if is_at_toplevel_doctest() { + (true, None) } else { - Some((cx.tcx.def_span(parent), may_move)) + let mut collector = PathCollector { paths: Vec::new() }; + collector.visit_ty(&impl_.self_ty); + if let Some(of_trait) = &impl_.of_trait { + collector.visit_trait_ref(of_trait); + } + collector.visit_generics(&impl_.generics); + + let mut may_move: Vec = collector + .paths + .into_iter() + .filter_map(|path| { + if let Some(did) = path.res.opt_def_id() + && did_has_local_parent(did, cx.tcx, parent, parent_parent) + { + Some(cx.tcx.def_span(did)) + } else { + None + } + }) + .collect(); + may_move.sort(); + may_move.dedup(); + + let move_to = if may_move.is_empty() { + ms.push_span_label( + cx.tcx.def_span(parent), + fluent::lint_non_local_definitions_impl_move_help, + ); + None + } else { + Some((cx.tcx.def_span(parent), may_move)) + }; + + (false, move_to) }; cx.emit_span_lint( @@ -272,6 +285,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { self_ty_str, of_trait_str, move_to, + doctest, may_remove, has_trait: impl_.of_trait.is_some(), }, @@ -280,12 +294,6 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { ItemKind::Macro(_macro, MacroKind::Bang) if cx.tcx.has_attr(item.owner_id.def_id, sym::macro_export) => { - // determining we if are in a doctest context can't currently be determined - // by the code it-self (no specific attrs), but fortunatly rustdoc sets a - // perma-unstable env for libtest so we just re-use that env for now - let is_at_toplevel_doctest = - self.body_depth == 2 && std::env::var("UNSTABLE_RUSTDOC_TEST_PATH").is_ok(); - cx.emit_span_lint( NON_LOCAL_DEFINITIONS, item.span, @@ -296,8 +304,8 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { .map(|s| s.to_ident_string()) .unwrap_or_else(|| "".to_string()), cargo_update: cargo_update(), - help: (!is_at_toplevel_doctest).then_some(()), - doctest_help: is_at_toplevel_doctest.then_some(()), + help: (!is_at_toplevel_doctest()).then_some(()), + doctest_help: is_at_toplevel_doctest().then_some(()), }, ) } diff --git a/tests/rustdoc-ui/doctest/auxiliary/pub_trait.rs b/tests/rustdoc-ui/doctest/auxiliary/pub_trait.rs new file mode 100644 index 0000000000000..0a47fdc74d721 --- /dev/null +++ b/tests/rustdoc-ui/doctest/auxiliary/pub_trait.rs @@ -0,0 +1 @@ +pub trait Trait {} diff --git a/tests/rustdoc-ui/doctest/non-local-defs-impl.rs b/tests/rustdoc-ui/doctest/non-local-defs-impl.rs new file mode 100644 index 0000000000000..c984e097c0463 --- /dev/null +++ b/tests/rustdoc-ui/doctest/non-local-defs-impl.rs @@ -0,0 +1,31 @@ +//@ check-fail +//@ edition:2018 +//@ failure-status: 101 +//@ aux-build:pub_trait.rs +//@ compile-flags: --test --test-args --test-threads=1 +//@ normalize-stdout-test: "tests/rustdoc-ui/doctest" -> "$$DIR" +//@ normalize-stdout-test "finished in \d+\.\d+s" -> "finished in $$TIME" + +#![doc(test(attr(deny(non_local_definitions))))] +#![doc(test(attr(allow(dead_code))))] + +/// This will produce a warning: +/// ```rust,no_run +/// # extern crate pub_trait; +/// # use pub_trait::Trait; +/// +/// struct Local; +/// impl Trait for &Local {} +/// ``` +/// +/// But this shoudln't produce a warning: +/// ```rust,no_run +/// # extern crate pub_trait; +/// # use pub_trait::Trait; +/// +/// struct Local; +/// impl Trait for &Local {} +/// +/// # fn main() {} +/// ``` +pub fn doctest() {} diff --git a/tests/rustdoc-ui/doctest/non-local-defs-impl.stdout b/tests/rustdoc-ui/doctest/non-local-defs-impl.stdout new file mode 100644 index 0000000000000..27797e22f8ecb --- /dev/null +++ b/tests/rustdoc-ui/doctest/non-local-defs-impl.stdout @@ -0,0 +1,37 @@ + +running 2 tests +test $DIR/non-local-defs-impl.rs - doctest (line 13) - compile ... FAILED +test $DIR/non-local-defs-impl.rs - doctest (line 22) - compile ... ok + +failures: + +---- $DIR/non-local-defs-impl.rs - doctest (line 13) stdout ---- +error: non-local `impl` definition, `impl` blocks should be written at the same level as their item + --> $DIR/non-local-defs-impl.rs:18:1 + | +LL | impl Trait for &Local {} + | ^^^^^-----^^^^^------ + | | | + | | `&'_ Local` is not local + | | help: remove `&` to make the `impl` local + | `Trait` is not local + | + = note: `impl` may be usable in bounds, etc. from outside the expression, which might e.g. make something constructible that previously wasn't, because it's still on a publicly-visible type + = note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl` + = help: make this doc-test a standalone test with its own `fn main() { ... }` + = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue +note: the lint level is defined here + --> $DIR/non-local-defs-impl.rs:11:9 + | +LL | #![deny(non_local_definitions)] + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error + +Couldn't compile the test. + +failures: + $DIR/non-local-defs-impl.rs - doctest (line 13) + +test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME + From 94c282197d7081370d3fd538c66976fb41554cf6 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 13 Jun 2024 19:23:30 +0200 Subject: [PATCH 08/14] Also simplify macro_rules doctest code --- compiler/rustc_lint/src/lints.rs | 13 +++++-------- compiler/rustc_lint/src/non_local_def.rs | 3 +-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 6cc0a81aa4f67..1c983a516c853 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1368,8 +1368,7 @@ pub enum NonLocalDefinitionsDiag { depth: u32, body_kind_descr: &'static str, body_name: String, - help: Option<()>, - doctest_help: Option<()>, + doctest: bool, cargo_update: Option, }, } @@ -1448,8 +1447,7 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag { depth, body_kind_descr, body_name, - help, - doctest_help, + doctest, cargo_update, } => { diag.primary_message(fluent::lint_non_local_definitions_macro_rules); @@ -1457,11 +1455,10 @@ impl<'a> LintDiagnostic<'a, ()> for NonLocalDefinitionsDiag { diag.arg("body_kind_descr", body_kind_descr); diag.arg("body_name", body_name); - if let Some(()) = help { - diag.help(fluent::lint_help); - } - if let Some(()) = doctest_help { + if doctest { diag.help(fluent::lint_help_doctest); + } else { + diag.help(fluent::lint_help); } diag.note(fluent::lint_non_local); diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index 423862dcdbae9..48d791a385968 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -304,8 +304,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { .map(|s| s.to_ident_string()) .unwrap_or_else(|| "".to_string()), cargo_update: cargo_update(), - help: (!is_at_toplevel_doctest()).then_some(()), - doctest_help: is_at_toplevel_doctest().then_some(()), + doctest: is_at_toplevel_doctest(), }, ) } From 207c5bc5a9007acd5bf3dbd58ef361bbb3ff3866 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 17 Jun 2024 08:48:49 +0300 Subject: [PATCH 09/14] override user defined channel when using precompiled rustc Signed-off-by: onur-ozkan --- src/bootstrap/src/core/config/config.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index a1d8ca3cbcaa7..0438dee7241fe 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1718,7 +1718,23 @@ impl Config { config.omit_git_hash = omit_git_hash.unwrap_or(default); config.rust_info = GitInfo::new(config.omit_git_hash, &config.src); - if config.rust_info.is_from_tarball() && !is_user_configured_rust_channel { + // We need to override `rust.channel` if it's manually specified when using the CI rustc. + // This is because if the compiler uses a different channel than the one specified in config.toml, + // tests may fail due to using a different channel than the one used by the compiler during tests. + if let Some(commit) = &config.download_rustc_commit { + if is_user_configured_rust_channel { + println!( + "WARNING: `rust.download-rustc` is enabled. The `rust.channel` option will be overridden by the CI rustc's channel." + ); + + let channel = config + .read_file_by_commit(&PathBuf::from("src/ci/channel"), commit) + .trim() + .to_owned(); + + config.channel = channel; + } + } else if config.rust_info.is_from_tarball() && !is_user_configured_rust_channel { ci_channel.clone_into(&mut config.channel); } From 5ae2446109ccdc64921561228eb983f795705b17 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Mon, 17 Jun 2024 08:49:36 +0300 Subject: [PATCH 10/14] simplify `Builder::doc_rust_lang_org_channel` This is already handled at the config parsing level, so we can simplify it. Signed-off-by: onur-ozkan --- src/bootstrap/src/core/builder.rs | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index d9e4cbae17d71..73e2cebc21c15 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1036,23 +1036,12 @@ impl<'a> Builder<'a> { } pub fn doc_rust_lang_org_channel(&self) -> String { - // When using precompiled compiler from CI, we need to use CI rustc's channel and - // ignore `rust.channel` from the configuration. Otherwise most of the rustdoc tests - // will fail due to incompatible `DOC_RUST_LANG_ORG_CHANNEL`. - let channel = if let Some(commit) = self.config.download_rustc_commit() { - self.config - .read_file_by_commit(&PathBuf::from("src/ci/channel"), commit) - .trim() - .to_owned() - } else { - match &*self.config.channel { - "stable" => &self.version, - "beta" => "beta", - "nightly" | "dev" => "nightly", - // custom build of rustdoc maybe? link to the latest stable docs just in case - _ => "stable", - } - .to_owned() + let channel = match &*self.config.channel { + "stable" => &self.version, + "beta" => "beta", + "nightly" | "dev" => "nightly", + // custom build of rustdoc maybe? link to the latest stable docs just in case + _ => "stable", }; format!("https://doc.rust-lang.org/{channel}") From ea30a9d2470cc34e93fc9cac8bbcb3a49c3030f5 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Mon, 17 Jun 2024 11:51:32 -0400 Subject: [PATCH 11/14] Rework count function without glob --- Cargo.lock | 2 +- src/tools/run-make-support/Cargo.toml | 2 +- src/tools/run-make-support/src/lib.rs | 44 +++++++++++++++------- tests/run-make/sepcomp-cci-copies/rmake.rs | 5 ++- tests/run-make/sepcomp-inlining/rmake.rs | 17 +++++---- tests/run-make/sepcomp-separate/rmake.rs | 5 ++- 6 files changed, 47 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e419c0e9a46d6..1ba01fcaf362f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3461,10 +3461,10 @@ name = "run_make_support" version = "0.2.0" dependencies = [ "gimli 0.28.1", - "glob", "object 0.34.0", "regex", "similar", + "walkdir", "wasmparser", ] diff --git a/src/tools/run-make-support/Cargo.toml b/src/tools/run-make-support/Cargo.toml index 4a244252c84d8..d4b2f963a6da4 100644 --- a/src/tools/run-make-support/Cargo.toml +++ b/src/tools/run-make-support/Cargo.toml @@ -9,4 +9,4 @@ similar = "2.5.0" wasmparser = "0.118.2" regex = "1.8" # 1.8 to avoid memchr 2.6.0, as 2.5.0 is pinned in the workspace gimli = "0.28.1" -glob = "0.3.1" +walkdir = "2.5.0" diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index 17b62bc8dbbc9..78e7601ff967b 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -22,9 +22,9 @@ use std::panic; use std::path::{Path, PathBuf}; pub use gimli; -pub use glob; pub use object; pub use regex; +pub use walkdir; pub use wasmparser; pub use cc::{cc, extra_c_flags, extra_cxx_flags, Cc}; @@ -245,21 +245,37 @@ pub fn uname() -> String { output.stdout_utf8() } -/// Inside a glob pattern of files (paths), read their contents and count the +/// Search for all files in the current working directory with the extension `ext`, +/// read their contents and count the /// number of regex matches with a given expression (re). #[track_caller] -pub fn count_regex_matches_in_file_glob(re: &str, paths: &str) -> usize { - let re = regex::Regex::new(re).expect(format!("Regex expression {re} is not valid.").as_str()); - let paths = glob::glob(paths).expect(format!("Glob expression {paths} is not valid.").as_str()); - use io::BufRead; - paths - .filter_map(|entry| entry.ok()) - .filter(|entry| entry.as_path().is_file()) - .filter_map(|path| fs::File::open(&path).ok()) - .map(|file| io::BufReader::new(file)) - .flat_map(|reader| reader.lines().filter_map(|entry| entry.ok())) - .filter(|line| re.is_match(line)) - .count() +pub fn count_regex_matches_in_files_with_extension(re: ®ex::Regex, ext: &str) -> usize { + use std::io::BufRead; + use walkdir::{DirEntry, WalkDir}; + + let walker = WalkDir::new(cwd()).into_iter(); + + fn is_hidden(entry: &DirEntry) -> bool { + entry.file_name().to_str().map(|s| s.starts_with(".")).unwrap_or(false) + } + + let mut count = 0; + + for entry in walker.filter_entry(|e| !is_hidden(e)) { + let entry = entry.expect("failed to get DirEntry"); + if !entry.path().is_file() { + continue; + } + + if !entry.path().extension().is_some_and(|e| e == ext) { + continue; + } + + let content = fs_wrapper::read(entry.path()); + count += content.lines().filter(|line| re.is_match(&line.as_ref().unwrap())).count(); + } + + count } fn handle_failed_output(cmd: &Command, output: CompletedProcess, caller_line_number: u32) -> ! { diff --git a/tests/run-make/sepcomp-cci-copies/rmake.rs b/tests/run-make/sepcomp-cci-copies/rmake.rs index 8d2411c1faec4..a66cc2872b434 100644 --- a/tests/run-make/sepcomp-cci-copies/rmake.rs +++ b/tests/run-make/sepcomp-cci-copies/rmake.rs @@ -4,10 +4,11 @@ // created for each source module (see `rustc_const_eval::monomorphize::partitioning`). // See https://github.com/rust-lang/rust/pull/16367 -use run_make_support::{count_regex_matches_in_file_glob, rustc}; +use run_make_support::{count_regex_matches_in_files_with_extension, regex, rustc}; fn main() { rustc().input("cci_lib.rs").run(); rustc().input("foo.rs").emit("llvm-ir").codegen_units(6).arg("-Zinline-in-all-cgus").run(); - assert_eq!(count_regex_matches_in_file_glob(r#"define\ .*cci_fn"#, "foo.*.ll"), 2); + let re = regex::Regex::new(r#"define\ .*cci_fn"#).unwrap(); + assert_eq!(count_regex_matches_in_files_with_extension(&re, "ll"), 2); } diff --git a/tests/run-make/sepcomp-inlining/rmake.rs b/tests/run-make/sepcomp-inlining/rmake.rs index 0315fd6997204..ea4a4d210cc3e 100644 --- a/tests/run-make/sepcomp-inlining/rmake.rs +++ b/tests/run-make/sepcomp-inlining/rmake.rs @@ -5,15 +5,16 @@ // in only one compilation unit. // See https://github.com/rust-lang/rust/pull/16367 -use run_make_support::{count_regex_matches_in_file_glob, glob, regex, rustc}; +use run_make_support::{count_regex_matches_in_files_with_extension, regex, rustc}; fn main() { rustc().input("foo.rs").emit("llvm-ir").codegen_units(3).arg("-Zinline-in-all-cgus").run(); - assert_eq!(count_regex_matches_in_file_glob(r#"define\ i32\ .*inlined"#, "foo.*.ll"), 0); - assert_eq!(count_regex_matches_in_file_glob(r#"define\ internal\ .*inlined"#, "foo.*.ll"), 2); - assert_eq!(count_regex_matches_in_file_glob(r#"define\ hidden\ i32\ .*normal"#, "foo.*.ll"), 1); - assert_eq!( - count_regex_matches_in_file_glob(r#"declare\ hidden\ i32\ .*normal"#, "foo.*.ll"), - 2 - ); + let re = regex::Regex::new(r#"define\ i32\ .*inlined"#).unwrap(); + assert_eq!(count_regex_matches_in_files_with_extension(&re, "ll"), 0); + let re = regex::Regex::new(r#"define\ internal\ .*inlined"#).unwrap(); + assert_eq!(count_regex_matches_in_files_with_extension(&re, "ll"), 2); + let re = regex::Regex::new(r#"define\ hidden\ i32\ .*normal"#).unwrap(); + assert_eq!(count_regex_matches_in_files_with_extension(&re, "ll"), 1); + let re = regex::Regex::new(r#"declare\ hidden\ i32\ .*normal"#).unwrap(); + assert_eq!(count_regex_matches_in_files_with_extension(&re, "ll"), 2); } diff --git a/tests/run-make/sepcomp-separate/rmake.rs b/tests/run-make/sepcomp-separate/rmake.rs index 99ee221adc1b6..49958044a612a 100644 --- a/tests/run-make/sepcomp-separate/rmake.rs +++ b/tests/run-make/sepcomp-separate/rmake.rs @@ -3,9 +3,10 @@ // wind up in three different compilation units. // See https://github.com/rust-lang/rust/pull/16367 -use run_make_support::{count_regex_matches_in_file_glob, rustc}; +use run_make_support::{count_regex_matches_in_files_with_extension, regex, rustc}; fn main() { rustc().input("foo.rs").emit("llvm-ir").codegen_units(3).run(); - assert_eq!(count_regex_matches_in_file_glob(r#"define\ .*magic_fn"#, "foo.*.ll"), 3); + let re = regex::Regex::new(r#"define\ .*magic_fn"#).unwrap(); + assert_eq!(count_regex_matches_in_files_with_extension(&re, "ll"), 3); } From 475b63fa283efc4b5aafd8ddfde69229204dd059 Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Mon, 17 Jun 2024 22:56:12 -0500 Subject: [PATCH 12/14] Add `/rustc-ice*/ to `.gitignore` --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 485968d9c56ff..87d02563ed048 100644 --- a/.gitignore +++ b/.gitignore @@ -52,6 +52,7 @@ build/ /src/tools/x/target # Created by default with `src/ci/docker/run.sh` /obj/ +/rustc-ice* ## Temporary files *~ From 7321e791141ddf2ec4d5f8a9893146832118bb19 Mon Sep 17 00:00:00 2001 From: Vonr Date: Tue, 18 Jun 2024 23:25:08 +0800 Subject: [PATCH 13/14] Replace `move||` with `move ||` in `compiler/` and `library/` Edit from #126631 to revert changes on ui tests --- .../rustc_hir_typeck/src/typeck_root_ctxt.rs | 2 +- library/core/src/sync/atomic.rs | 2 +- library/std/src/sync/barrier.rs | 4 ++-- library/std/src/sync/condvar.rs | 16 ++++++++-------- library/std/src/sync/mpsc/mod.rs | 10 +++++----- library/std/src/thread/local.rs | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs b/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs index 28745af3a5309..b6e9000ef9506 100644 --- a/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs +++ b/compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs @@ -22,7 +22,7 @@ use std::ops::Deref; /// e.g. closures defined within the function. For example: /// ```ignore (illustrative) /// fn foo() { -/// bar(move|| { ... }) +/// bar(move || { ... }) /// } /// ``` /// Here, the function `foo()` and the closure passed to diff --git a/library/core/src/sync/atomic.rs b/library/core/src/sync/atomic.rs index 482bd19705c2f..c709ea2a15db1 100644 --- a/library/core/src/sync/atomic.rs +++ b/library/core/src/sync/atomic.rs @@ -183,7 +183,7 @@ //! //! let spinlock_clone = Arc::clone(&spinlock); //! -//! let thread = thread::spawn(move|| { +//! let thread = thread::spawn(move || { //! spinlock_clone.store(0, Ordering::Release); //! }); //! diff --git a/library/std/src/sync/barrier.rs b/library/std/src/sync/barrier.rs index b4bac081e7ab7..82cc13a74b7f1 100644 --- a/library/std/src/sync/barrier.rs +++ b/library/std/src/sync/barrier.rs @@ -20,7 +20,7 @@ use crate::sync::{Condvar, Mutex}; /// let c = Arc::clone(&barrier); /// // The same messages will be printed together. /// // You will NOT see any interleaving. -/// handles.push(thread::spawn(move|| { +/// handles.push(thread::spawn(move || { /// println!("before wait"); /// c.wait(); /// println!("after wait"); @@ -115,7 +115,7 @@ impl Barrier { /// let c = Arc::clone(&barrier); /// // The same messages will be printed together. /// // You will NOT see any interleaving. - /// handles.push(thread::spawn(move|| { + /// handles.push(thread::spawn(move || { /// println!("before wait"); /// c.wait(); /// println!("after wait"); diff --git a/library/std/src/sync/condvar.rs b/library/std/src/sync/condvar.rs index b20574e4f1493..f9f83fb4f63c3 100644 --- a/library/std/src/sync/condvar.rs +++ b/library/std/src/sync/condvar.rs @@ -88,7 +88,7 @@ impl WaitTimeoutResult { /// let pair2 = Arc::clone(&pair); /// /// // Inside of our lock, spawn a new thread, and then wait for it to start. -/// thread::spawn(move|| { +/// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut started = lock.lock().unwrap(); /// *started = true; @@ -166,7 +166,7 @@ impl Condvar { /// let pair = Arc::new((Mutex::new(false), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// - /// thread::spawn(move|| { + /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut started = lock.lock().unwrap(); /// *started = true; @@ -221,7 +221,7 @@ impl Condvar { /// let pair = Arc::new((Mutex::new(true), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// - /// thread::spawn(move|| { + /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut pending = lock.lock().unwrap(); /// *pending = false; @@ -280,7 +280,7 @@ impl Condvar { /// let pair = Arc::new((Mutex::new(false), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// - /// thread::spawn(move|| { + /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut started = lock.lock().unwrap(); /// *started = true; @@ -352,7 +352,7 @@ impl Condvar { /// let pair = Arc::new((Mutex::new(false), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// - /// thread::spawn(move|| { + /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut started = lock.lock().unwrap(); /// *started = true; @@ -420,7 +420,7 @@ impl Condvar { /// let pair = Arc::new((Mutex::new(true), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// - /// thread::spawn(move|| { + /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut pending = lock.lock().unwrap(); /// *pending = false; @@ -484,7 +484,7 @@ impl Condvar { /// let pair = Arc::new((Mutex::new(false), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// - /// thread::spawn(move|| { + /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut started = lock.lock().unwrap(); /// *started = true; @@ -524,7 +524,7 @@ impl Condvar { /// let pair = Arc::new((Mutex::new(false), Condvar::new())); /// let pair2 = Arc::clone(&pair); /// - /// thread::spawn(move|| { + /// thread::spawn(move || { /// let (lock, cvar) = &*pair2; /// let mut started = lock.lock().unwrap(); /// *started = true; diff --git a/library/std/src/sync/mpsc/mod.rs b/library/std/src/sync/mpsc/mod.rs index d353c7bd5de9e..feee6948db4fd 100644 --- a/library/std/src/sync/mpsc/mod.rs +++ b/library/std/src/sync/mpsc/mod.rs @@ -51,7 +51,7 @@ //! //! // Create a simple streaming channel //! let (tx, rx) = channel(); -//! thread::spawn(move|| { +//! thread::spawn(move || { //! tx.send(10).unwrap(); //! }); //! assert_eq!(rx.recv().unwrap(), 10); @@ -69,7 +69,7 @@ //! let (tx, rx) = channel(); //! for i in 0..10 { //! let tx = tx.clone(); -//! thread::spawn(move|| { +//! thread::spawn(move || { //! tx.send(i).unwrap(); //! }); //! } @@ -99,7 +99,7 @@ //! use std::sync::mpsc::sync_channel; //! //! let (tx, rx) = sync_channel::(0); -//! thread::spawn(move|| { +//! thread::spawn(move || { //! // This will wait for the parent thread to start receiving //! tx.send(53).unwrap(); //! }); @@ -510,7 +510,7 @@ pub enum TrySendError { /// let (sender, receiver) = channel(); /// /// // Spawn off an expensive computation -/// thread::spawn(move|| { +/// thread::spawn(move || { /// # fn expensive_computation() {} /// sender.send(expensive_computation()).unwrap(); /// }); @@ -561,7 +561,7 @@ pub fn channel() -> (Sender, Receiver) { /// // this returns immediately /// sender.send(1).unwrap(); /// -/// thread::spawn(move|| { +/// thread::spawn(move || { /// // this will block until the previous message has been received /// sender.send(2).unwrap(); /// }); diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index aed185637fd1f..f147c5fdcd146 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -62,7 +62,7 @@ use crate::fmt; /// FOO.set(2); /// /// // each thread starts out with the initial value of 1 -/// let t = thread::spawn(move|| { +/// let t = thread::spawn(move || { /// assert_eq!(FOO.get(), 1); /// FOO.set(3); /// }); From de473a5a2b7e15aa45e4c2b0d070c22f7140316a Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 11 Apr 2024 08:56:10 +0000 Subject: [PATCH 14/14] Test that opaque types can't have themselves as a hidden type with incompatible lifetimes --- .../different_args_considered_equal.rs | 14 ++++++++++++ .../different_args_considered_equal.stderr | 15 +++++++++++++ .../different_args_considered_equal2.rs | 14 ++++++++++++ .../different_args_considered_equal2.stderr | 20 +++++++++++++++++ .../different_args_considered_equal3.rs | 22 +++++++++++++++++++ .../different_args_considered_equal3.stderr | 10 +++++++++ 6 files changed, 95 insertions(+) create mode 100644 tests/ui/type-alias-impl-trait/different_args_considered_equal.rs create mode 100644 tests/ui/type-alias-impl-trait/different_args_considered_equal.stderr create mode 100644 tests/ui/type-alias-impl-trait/different_args_considered_equal2.rs create mode 100644 tests/ui/type-alias-impl-trait/different_args_considered_equal2.stderr create mode 100644 tests/ui/type-alias-impl-trait/different_args_considered_equal3.rs create mode 100644 tests/ui/type-alias-impl-trait/different_args_considered_equal3.stderr diff --git a/tests/ui/type-alias-impl-trait/different_args_considered_equal.rs b/tests/ui/type-alias-impl-trait/different_args_considered_equal.rs new file mode 100644 index 0000000000000..8ce471e395681 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/different_args_considered_equal.rs @@ -0,0 +1,14 @@ +#![feature(type_alias_impl_trait)] + +pub type Opaque<'a> = impl Sized; + +fn get_one<'a>(a: *mut &'a str) -> Opaque<'a> { + a +} + +fn get_iter<'a>() -> impl IntoIterator> { + //~^ ERROR: item does not constrain + None::> +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/different_args_considered_equal.stderr b/tests/ui/type-alias-impl-trait/different_args_considered_equal.stderr new file mode 100644 index 0000000000000..f27f223452523 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/different_args_considered_equal.stderr @@ -0,0 +1,15 @@ +error: item does not constrain `Opaque::{opaque#0}`, but has it in its signature + --> $DIR/different_args_considered_equal.rs:9:4 + | +LL | fn get_iter<'a>() -> impl IntoIterator> { + | ^^^^^^^^ + | + = note: consider moving the opaque type's declaration and defining uses into a separate module +note: this opaque type is in the signature + --> $DIR/different_args_considered_equal.rs:3:23 + | +LL | pub type Opaque<'a> = impl Sized; + | ^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/type-alias-impl-trait/different_args_considered_equal2.rs b/tests/ui/type-alias-impl-trait/different_args_considered_equal2.rs new file mode 100644 index 0000000000000..43dfea97e6dbd --- /dev/null +++ b/tests/ui/type-alias-impl-trait/different_args_considered_equal2.rs @@ -0,0 +1,14 @@ +#![feature(type_alias_impl_trait)] + +pub type Opaque<'a> = impl Sized; + +fn get_one<'a>(a: *mut &'a str) -> impl IntoIterator> { + if a.is_null() { + Some(a) + } else { + None::> + //~^ ERROR hidden type for `Opaque<'static>` captures lifetime that does not appear in bounds + } +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/different_args_considered_equal2.stderr b/tests/ui/type-alias-impl-trait/different_args_considered_equal2.stderr new file mode 100644 index 0000000000000..1104c2c498a5a --- /dev/null +++ b/tests/ui/type-alias-impl-trait/different_args_considered_equal2.stderr @@ -0,0 +1,20 @@ +error[E0700]: hidden type for `Opaque<'static>` captures lifetime that does not appear in bounds + --> $DIR/different_args_considered_equal2.rs:9:9 + | +LL | pub type Opaque<'a> = impl Sized; + | ---------- opaque type defined here +LL | +LL | fn get_one<'a>(a: *mut &'a str) -> impl IntoIterator> { + | -- hidden type `*mut &'a str` captures the lifetime `'a` as defined here +... +LL | None::> + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: to declare that `impl IntoIterator>` captures `'a`, you can add an explicit `'a` lifetime bound + | +LL | fn get_one<'a>(a: *mut &'a str) -> impl IntoIterator> + 'a { + | ++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0700`. diff --git a/tests/ui/type-alias-impl-trait/different_args_considered_equal3.rs b/tests/ui/type-alias-impl-trait/different_args_considered_equal3.rs new file mode 100644 index 0000000000000..ea69175ba3108 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/different_args_considered_equal3.rs @@ -0,0 +1,22 @@ +//! Test that we don't allow coercing an opaque type with a non-static +//! lifetime to one with a static lifetime. While `get_iter` looks like +//! it would be doing the opposite, the way we're handling projections +//! makes `Opaque<'a>` the hidden type of `Opaque<'static>`. + +#![feature(type_alias_impl_trait)] + +mod defining_scope { + pub type Opaque<'a> = impl Sized; + + fn get_one<'a>(a: *mut &'a str) -> Opaque<'a> { + a + } +} +use defining_scope::Opaque; + +fn get_iter<'a>() -> impl IntoIterator> { + None::> + //~^ ERROR lifetime may not live long enough +} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/different_args_considered_equal3.stderr b/tests/ui/type-alias-impl-trait/different_args_considered_equal3.stderr new file mode 100644 index 0000000000000..d8f70e3d77825 --- /dev/null +++ b/tests/ui/type-alias-impl-trait/different_args_considered_equal3.stderr @@ -0,0 +1,10 @@ +error: lifetime may not live long enough + --> $DIR/different_args_considered_equal3.rs:18:5 + | +LL | fn get_iter<'a>() -> impl IntoIterator> { + | -- lifetime `'a` defined here +LL | None::> + | ^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'a` must outlive `'static` + +error: aborting due to 1 previous error +