From c2d2df182aae08fd9022e5e8977f63f69aa485a0 Mon Sep 17 00:00:00 2001 From: CensoredUsername Date: Fri, 26 Apr 2024 02:23:14 +0200 Subject: [PATCH 1/9] Add a warning to Delimiter::None that rustc currently does not respect it. It does not provide the behaviour it is indicated to provide when used in a proc_macro context. --- library/proc_macro/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/library/proc_macro/src/lib.rs b/library/proc_macro/src/lib.rs index 1ceff2e506ccc..50611fda9a34b 100644 --- a/library/proc_macro/src/lib.rs +++ b/library/proc_macro/src/lib.rs @@ -815,6 +815,18 @@ pub enum Delimiter { /// "macro variable" `$var`. It is important to preserve operator priorities in cases like /// `$var * 3` where `$var` is `1 + 2`. /// Invisible delimiters might not survive roundtrip of a token stream through a string. + /// + ///
+ /// + /// Note: rustc currently can ignore the grouping of tokens delimited by `None` in the output + /// of a proc_macro. Only `None`-delimited groups created by a macro_rules macro in the input + /// of a proc_macro macro are preserved, and only in very specific circumstances. + /// Any `None`-delimited groups (re)created by a proc_macro will therefore not preserve + /// operator priorities as indicated above. The other `Delimiter` variants should be used + /// instead in this context. This is a rustc bug. For details, see + /// [rust-lang/rust#67062](https://github.com/rust-lang/rust/issues/67062). + /// + ///
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] None, } From f0cb386c7de34667160b38d016323c4ca18f2ed9 Mon Sep 17 00:00:00 2001 From: Mohammad Omidvar Date: Mon, 20 May 2024 18:11:07 +0000 Subject: [PATCH 2/9] Add intrinsic definition and retrieval APIs --- compiler/rustc_smir/src/rustc_smir/context.rs | 36 ++++++++++++------- compiler/stable_mir/src/compiler_interface.rs | 15 ++++++-- compiler/stable_mir/src/mir/mono.rs | 4 ++- compiler/stable_mir/src/ty.rs | 29 +++++++++++++++ 4 files changed, 68 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_smir/context.rs b/compiler/rustc_smir/src/rustc_smir/context.rs index fd31c020f899a..7af0aabcaa93d 100644 --- a/compiler/rustc_smir/src/rustc_smir/context.rs +++ b/compiler/rustc_smir/src/rustc_smir/context.rs @@ -23,8 +23,8 @@ use stable_mir::mir::{BinOp, Body, Place}; use stable_mir::target::{MachineInfo, MachineSize}; use stable_mir::ty::{ AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, ForeignDef, - ForeignItemKind, GenericArgs, LineInfo, PolyFnSig, RigidTy, Span, Ty, TyKind, UintTy, - VariantDef, + ForeignItemKind, GenericArgs, IntrinsicDef, LineInfo, PolyFnSig, RigidTy, Span, Ty, TyKind, + UintTy, VariantDef, }; use stable_mir::{Crate, CrateDef, CrateItem, CrateNum, DefId, Error, Filename, ItemKind, Symbol}; use std::cell::RefCell; @@ -307,6 +307,28 @@ impl<'tcx> Context for TablesWrapper<'tcx> { sig.stable(&mut *tables) } + fn intrinsic(&self, def: DefId) -> Option { + let mut tables = self.0.borrow_mut(); + let tcx = tables.tcx; + let def_id = def.internal(&mut *tables, tcx); + let intrinsic = tcx.intrinsic_raw(def_id); + intrinsic.map(|_| IntrinsicDef(def)) + } + + fn intrinsic_name(&self, def: IntrinsicDef) -> Symbol { + let mut tables = self.0.borrow_mut(); + let tcx = tables.tcx; + let def_id = def.0.internal(&mut *tables, tcx); + tcx.intrinsic(def_id).unwrap().name.to_string() + } + + fn intrinsic_must_be_overridden(&self, def: IntrinsicDef) -> bool { + let mut tables = self.0.borrow_mut(); + let tcx = tables.tcx; + let def_id = def.0.internal(&mut *tables, tcx); + tcx.intrinsic_raw(def_id).unwrap().must_be_overridden + } + fn closure_sig(&self, args: &GenericArgs) -> PolyFnSig { let mut tables = self.0.borrow_mut(); let tcx = tables.tcx; @@ -645,16 +667,6 @@ impl<'tcx> Context for TablesWrapper<'tcx> { } } - /// Retrieve the plain intrinsic name of an instance. - /// - /// This assumes that the instance is an intrinsic. - fn intrinsic_name(&self, def: InstanceDef) -> Symbol { - let tables = self.0.borrow_mut(); - let instance = tables.instances[def]; - let intrinsic = tables.tcx.intrinsic(instance.def_id()).unwrap(); - intrinsic.name.to_string() - } - fn ty_layout(&self, ty: Ty) -> Result { let mut tables = self.0.borrow_mut(); let tcx = tables.tcx; diff --git a/compiler/stable_mir/src/compiler_interface.rs b/compiler/stable_mir/src/compiler_interface.rs index 231d9ba49a39c..4dd64228bbadb 100644 --- a/compiler/stable_mir/src/compiler_interface.rs +++ b/compiler/stable_mir/src/compiler_interface.rs @@ -13,8 +13,8 @@ use crate::target::MachineInfo; use crate::ty::{ AdtDef, AdtKind, Allocation, ClosureDef, ClosureKind, Const, FieldDef, FnDef, ForeignDef, ForeignItemKind, ForeignModule, ForeignModuleDef, GenericArgs, GenericPredicates, Generics, - ImplDef, ImplTrait, LineInfo, PolyFnSig, RigidTy, Span, TraitDecl, TraitDef, Ty, TyKind, - UintTy, VariantDef, + ImplDef, ImplTrait, IntrinsicDef, LineInfo, PolyFnSig, RigidTy, Span, TraitDecl, TraitDef, Ty, + TyKind, UintTy, VariantDef, }; use crate::{ mir, Crate, CrateItem, CrateItems, CrateNum, DefId, Error, Filename, ImplTraitDecls, ItemKind, @@ -88,6 +88,16 @@ pub trait Context { /// Retrieve the function signature for the given generic arguments. fn fn_sig(&self, def: FnDef, args: &GenericArgs) -> PolyFnSig; + /// Retrieve the intrinsic definition if the item corresponds one. + fn intrinsic(&self, item: DefId) -> Option; + + /// Retrieve the plain function name of an intrinsic. + fn intrinsic_name(&self, def: IntrinsicDef) -> Symbol; + + /// Returns whether the intrinsic has no meaningful body and all backends + /// need to shim all calls to it. + fn intrinsic_must_be_overridden(&self, def: IntrinsicDef) -> bool; + /// Retrieve the closure signature for the given generic arguments. fn closure_sig(&self, args: &GenericArgs) -> PolyFnSig; @@ -198,7 +208,6 @@ pub trait Context { fn vtable_allocation(&self, global_alloc: &GlobalAlloc) -> Option; fn krate(&self, def_id: DefId) -> Crate; fn instance_name(&self, def: InstanceDef, trimmed: bool) -> Symbol; - fn intrinsic_name(&self, def: InstanceDef) -> Symbol; /// Return information about the target machine. fn target_info(&self) -> MachineInfo; diff --git a/compiler/stable_mir/src/mir/mono.rs b/compiler/stable_mir/src/mir/mono.rs index 394038926f61a..572f1499c5a5c 100644 --- a/compiler/stable_mir/src/mir/mono.rs +++ b/compiler/stable_mir/src/mir/mono.rs @@ -106,7 +106,9 @@ impl Instance { /// which is more convenient to match with intrinsic symbols. pub fn intrinsic_name(&self) -> Option { match self.kind { - InstanceKind::Intrinsic => Some(with(|context| context.intrinsic_name(self.def))), + InstanceKind::Intrinsic => { + Some(with(|context| context.intrinsic(self.def.def_id()).unwrap().fn_name())) + } InstanceKind::Item | InstanceKind::Virtual { .. } | InstanceKind::Shim => None, } } diff --git a/compiler/stable_mir/src/ty.rs b/compiler/stable_mir/src/ty.rs index 562516138404b..ab4deac9fd72c 100644 --- a/compiler/stable_mir/src/ty.rs +++ b/compiler/stable_mir/src/ty.rs @@ -622,6 +622,35 @@ impl FnDef { pub fn body(&self) -> Option { with(|ctx| ctx.has_body(self.0).then(|| ctx.mir_body(self.0))) } + + /// Get the information of the intrinsic if this function is a definition of one. + pub fn as_intrinsic(&self) -> Option { + with(|cx| cx.intrinsic(self.def_id())) + } + + /// Check if the function is an intrinsic. + #[inline] + pub fn is_intrinsic(&self) -> bool { + self.as_intrinsic().is_some() + } +} + +crate_def! { + pub IntrinsicDef; +} + +impl IntrinsicDef { + /// Returns the plain name of the intrinsic. + /// e.g., `transmute` for `core::intrinsics::transmute`. + pub fn fn_name(&self) -> Symbol { + with(|cx| cx.intrinsic_name(*self)) + } + + /// Returns whether the intrinsic has no meaningful body and all backends + /// need to shim all calls to it. + pub fn must_be_overridden(&self) -> bool { + with(|cx| cx.intrinsic_must_be_overridden(*self)) + } } crate_def! { From 56ad5453f6002aaabdcab6399238a1c93254a134 Mon Sep 17 00:00:00 2001 From: Mohammad Omidvar Date: Mon, 20 May 2024 18:12:06 +0000 Subject: [PATCH 3/9] Extend tests for intrinsic definitions --- .../stable-mir/check_intrinsics.rs | 50 ++++++++++++------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/tests/ui-fulldeps/stable-mir/check_intrinsics.rs b/tests/ui-fulldeps/stable-mir/check_intrinsics.rs index 171850b89bb5e..a9c1f64f812d9 100644 --- a/tests/ui-fulldeps/stable-mir/check_intrinsics.rs +++ b/tests/ui-fulldeps/stable-mir/check_intrinsics.rs @@ -11,6 +11,7 @@ //@ ignore-windows-gnu mingw has troubles with linking https://github.com/rust-lang/rust/pull/116837 #![feature(rustc_private)] +#![feature(assert_matches)] extern crate rustc_hir; #[macro_use] @@ -23,11 +24,11 @@ use rustc_smir::rustc_internal; use stable_mir::mir::mono::{Instance, InstanceKind}; use stable_mir::mir::visit::{Location, MirVisitor}; use stable_mir::mir::{LocalDecl, Terminator, TerminatorKind}; -use stable_mir::ty::{RigidTy, TyKind}; -use std::collections::HashSet; +use stable_mir::ty::{FnDef, GenericArgs, IntrinsicDef, RigidTy, TyKind}; use std::convert::TryFrom; use std::io::Write; use std::ops::ControlFlow; +use std::assert_matches::assert_matches; /// This function tests that we can correctly get type information from binary operations. fn test_intrinsics() -> ControlFlow<()> { @@ -39,9 +40,10 @@ fn test_intrinsics() -> ControlFlow<()> { visitor.visit_body(&main_body); let calls = visitor.calls; - assert_eq!(calls.len(), 2, "Expected 2 calls, but found: {calls:?}"); - for intrinsic in &calls { - check_intrinsic(intrinsic) + assert_eq!(calls.len(), 3, "Expected 3 calls, but found: {calls:?}"); + for (fn_def, args) in &calls { + check_instance(&Instance::resolve(*fn_def, &args).unwrap()); + check_def(fn_def.as_intrinsic().unwrap()); } ControlFlow::Continue(()) @@ -53,22 +55,35 @@ fn test_intrinsics() -> ControlFlow<()> { /// /// If by any chance this test breaks because you changed how an intrinsic is implemented, please /// update the test to invoke a different intrinsic. -fn check_intrinsic(intrinsic: &Instance) { - assert_eq!(intrinsic.kind, InstanceKind::Intrinsic); - let name = intrinsic.intrinsic_name().unwrap(); - if intrinsic.has_body() { - let Some(body) = intrinsic.body() else { unreachable!("Expected a body") }; +fn check_instance(instance: &Instance) { + assert_eq!(instance.kind, InstanceKind::Intrinsic); + let name = instance.intrinsic_name().unwrap(); + if instance.has_body() { + let Some(body) = instance.body() else { unreachable!("Expected a body") }; assert!(!body.blocks.is_empty()); - assert_eq!(&name, "likely"); + assert_matches!(name.as_str(), "likely" | "vtable_size"); } else { - assert!(intrinsic.body().is_none()); + assert!(instance.body().is_none()); assert_eq!(&name, "size_of_val"); } } +fn check_def(intrinsic: IntrinsicDef) { + let name = intrinsic.fn_name(); + match name.as_str() { + "likely" | "size_of_val" => { + assert!(!intrinsic.must_be_overridden()); + } + "vtable_size" => { + assert!(intrinsic.must_be_overridden()); + } + _ => unreachable!("Unexpected intrinsic: {}", name), + } +} + struct CallsVisitor<'a> { locals: &'a [LocalDecl], - calls: HashSet, + calls: Vec<(FnDef, GenericArgs)>, } impl<'a> MirVisitor for CallsVisitor<'a> { @@ -77,10 +92,10 @@ impl<'a> MirVisitor for CallsVisitor<'a> { TerminatorKind::Call { func, .. } => { let TyKind::RigidTy(RigidTy::FnDef(def, args)) = func.ty(self.locals).unwrap().kind() - else { - return; - }; - self.calls.insert(Instance::resolve(def, &args).unwrap()); + else { + return; + }; + self.calls.push((def, args.clone())); } _ => {} } @@ -106,6 +121,7 @@ fn generate_input(path: &str) -> std::io::Result<()> { #![feature(core_intrinsics)] use std::intrinsics::*; pub fn use_intrinsics(init: bool) -> bool {{ + let vtable_sz = unsafe {{ vtable_size(0 as *const ()) }}; let sz = unsafe {{ size_of_val("hi") }}; likely(init && sz == 2) }} From fc76015dcb77ce86aa2df98c33a8e9cf36c16352 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 22 May 2024 11:47:32 +0200 Subject: [PATCH 4/9] Migrate `run-make/rustdoc-scrape-examples-macros` to `rmake.rs` --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../rustdoc-scrape-examples-macros/Makefile | 19 ------ .../rustdoc-scrape-examples-macros/rmake.rs | 64 +++++++++++++++++++ 3 files changed, 64 insertions(+), 20 deletions(-) delete mode 100644 tests/run-make/rustdoc-scrape-examples-macros/Makefile create mode 100644 tests/run-make/rustdoc-scrape-examples-macros/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 96fb8e27e6d21..54912cdd2548d 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -233,7 +233,6 @@ run-make/rlib-format-packed-bundled-libs/Makefile run-make/rmeta-preferred/Makefile run-make/rustc-macro-dep-files/Makefile run-make/rustdoc-io-error/Makefile -run-make/rustdoc-scrape-examples-macros/Makefile run-make/rustdoc-verify-output-files/Makefile run-make/rustdoc-with-output-option/Makefile run-make/rustdoc-with-short-out-dir-option/Makefile diff --git a/tests/run-make/rustdoc-scrape-examples-macros/Makefile b/tests/run-make/rustdoc-scrape-examples-macros/Makefile deleted file mode 100644 index edc19d8cb5de4..0000000000000 --- a/tests/run-make/rustdoc-scrape-examples-macros/Makefile +++ /dev/null @@ -1,19 +0,0 @@ -# ignore-cross-compile -include ../../run-make/tools.mk - -OUTPUT_DIR := "$(TMPDIR)/rustdoc" -DYLIB_NAME := $(shell echo | $(RUSTC) --crate-name foobar_macro --crate-type dylib --print file-names -) - -all: - $(RUSTC) src/proc.rs --crate-name foobar_macro --edition=2021 --crate-type proc-macro --emit=dep-info,link - - $(RUSTC) src/lib.rs --crate-name foobar --edition=2021 --crate-type lib --emit=dep-info,link - - $(RUSTDOC) examples/ex.rs --crate-name ex --crate-type bin --output $(OUTPUT_DIR) \ - --extern foobar=$(TMPDIR)/libfoobar.rlib --extern foobar_macro=$(TMPDIR)/$(DYLIB_NAME) \ - -Z unstable-options --scrape-examples-output-path $(TMPDIR)/ex.calls --scrape-examples-target-crate foobar - - $(RUSTDOC) src/lib.rs --crate-name foobar --crate-type lib --output $(OUTPUT_DIR) \ - -Z unstable-options --with-examples $(TMPDIR)/ex.calls - - $(HTMLDOCCK) $(OUTPUT_DIR) src/lib.rs diff --git a/tests/run-make/rustdoc-scrape-examples-macros/rmake.rs b/tests/run-make/rustdoc-scrape-examples-macros/rmake.rs new file mode 100644 index 0000000000000..81b7defafc6c0 --- /dev/null +++ b/tests/run-make/rustdoc-scrape-examples-macros/rmake.rs @@ -0,0 +1,64 @@ +//@ ignore-cross-compile + +use run_make_support::{htmldocck, rustc, rustdoc, tmp_dir}; + +fn main() { + let tmp_dir = tmp_dir(); + let out_dir = tmp_dir.join("rustdoc"); + let ex_dir = tmp_dir.join("ex.calls"); + let proc_crate_name = "foobar_macro"; + let crate_name = "foobar"; + + let dylib_name = String::from_utf8( + rustc() + .crate_name(proc_crate_name) + .crate_type("dylib") + .arg("--print") + .arg("file-names") + .arg("-") + .command_output() + .stdout, + ) + .unwrap(); + + rustc() + .input("src/proc.rs") + .crate_name(proc_crate_name) + .edition("2021") + .crate_type("proc-macro") + .emit("dep-info,link") + .run(); + rustc() + .input("src/lib.rs") + .crate_name(crate_name) + .edition("2021") + .crate_type("lib") + .emit("dep-info,link") + .run(); + + rustdoc() + .input("examples/ex.rs") + .crate_name("ex") + .crate_type("bin") + .output(&out_dir) + .extern_(crate_name, tmp_dir.join(format!("lib{crate_name}.rlib"))) + .extern_(proc_crate_name, tmp_dir.join(dylib_name.trim())) + .arg("-Zunstable-options") + .arg("--scrape-examples-output-path") + .arg(&ex_dir) + .arg("--scrape-examples-target-crate") + .arg(crate_name) + .run(); + + rustdoc() + .input("src/lib.rs") + .crate_name(crate_name) + .crate_type("lib") + .output(&out_dir) + .arg("-Zunstable-options") + .arg("--with-examples") + .arg(&ex_dir) + .run(); + + assert!(htmldocck().arg(out_dir).arg("src/lib.rs").status().unwrap().success()); +} From f377ea165f2d3e977731e9746876a15b2c7e48a6 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 17 May 2024 16:50:37 -0400 Subject: [PATCH 5/9] rewrite issue-30063 --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/issue-30063/Makefile | 36 ------------------ .../{issue-30063 => reset-codegen-1}/foo.rs | 0 tests/run-make/reset-codegen-1/rmake.rs | 38 +++++++++++++++++++ 4 files changed, 38 insertions(+), 37 deletions(-) delete mode 100644 tests/run-make/issue-30063/Makefile rename tests/run-make/{issue-30063 => reset-codegen-1}/foo.rs (100%) create mode 100644 tests/run-make/reset-codegen-1/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 9b879f33778fd..d7ce4ed7f21f6 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -103,7 +103,6 @@ run-make/issue-25581/Makefile run-make/issue-26006/Makefile run-make/issue-26092/Makefile run-make/issue-28595/Makefile -run-make/issue-30063/Makefile run-make/issue-33329/Makefile run-make/issue-35164/Makefile run-make/issue-36710/Makefile diff --git a/tests/run-make/issue-30063/Makefile b/tests/run-make/issue-30063/Makefile deleted file mode 100644 index 8a69ca79f5150..0000000000000 --- a/tests/run-make/issue-30063/Makefile +++ /dev/null @@ -1,36 +0,0 @@ -# ignore-cross-compile -include ../tools.mk - -all: - rm -f $(TMPDIR)/foo-output - $(RUSTC) -C codegen-units=4 -o $(TMPDIR)/foo-output foo.rs - rm $(TMPDIR)/foo-output - - rm -f $(TMPDIR)/asm-output - $(RUSTC) -C codegen-units=4 --emit=asm -o $(TMPDIR)/asm-output foo.rs - rm $(TMPDIR)/asm-output - - rm -f $(TMPDIR)/bc-output - $(RUSTC) -C codegen-units=4 --emit=llvm-bc -o $(TMPDIR)/bc-output foo.rs - rm $(TMPDIR)/bc-output - - rm -f $(TMPDIR)/ir-output - $(RUSTC) -C codegen-units=4 --emit=llvm-ir -o $(TMPDIR)/ir-output foo.rs - rm $(TMPDIR)/ir-output - - rm -f $(TMPDIR)/link-output - $(RUSTC) -C codegen-units=4 --emit=link -o $(TMPDIR)/link-output foo.rs - rm $(TMPDIR)/link-output - - rm -f $(TMPDIR)/obj-output - $(RUSTC) -C codegen-units=4 --emit=obj -o $(TMPDIR)/obj-output foo.rs - rm $(TMPDIR)/obj-output - - rm -f $(TMPDIR)/dep-output - $(RUSTC) -C codegen-units=4 --emit=dep-info -o $(TMPDIR)/dep-output foo.rs - rm $(TMPDIR)/dep-output - -# # (This case doesn't work yet, and may be fundamentally wrong-headed anyway.) -# rm -f $(TMPDIR)/multi-output -# $(RUSTC) -C codegen-units=4 --emit=asm,obj -o $(TMPDIR)/multi-output foo.rs -# rm $(TMPDIR)/multi-output diff --git a/tests/run-make/issue-30063/foo.rs b/tests/run-make/reset-codegen-1/foo.rs similarity index 100% rename from tests/run-make/issue-30063/foo.rs rename to tests/run-make/reset-codegen-1/foo.rs diff --git a/tests/run-make/reset-codegen-1/rmake.rs b/tests/run-make/reset-codegen-1/rmake.rs new file mode 100644 index 0000000000000..4b91ba7df90bd --- /dev/null +++ b/tests/run-make/reset-codegen-1/rmake.rs @@ -0,0 +1,38 @@ +// When rustc received 4 codegen-units, an output path and an emit flag all simultaneously, +// this could cause an annoying recompilation issue, uselessly lengthening the build process. +// A fix was delivered, which resets codegen-units to 1 when necessary, +// but as it directly affected the way codegen-units are manipulated, +// this test was created to check that this fix did not cause compilation failures. +// See https://github.com/rust-lang/rust/issues/30063 + +//@ ignore-cross-compile + +use run_make_support::{rustc, tmp_dir}; +use std::fs; + +fn compile(output_file: &str, emit: Option<&str>) { + let mut rustc = rustc(); + let rustc = rustc.codegen_units(4).output(tmp_dir().join(output_file)).input("foo.rs"); + if let Some(emit) = emit { + rustc.emit(emit); + } + rustc.run(); +} + +fn main() { + let flags = [ + ("foo-output", None), + ("asm-output", Some("asm")), + ("bc-output", Some("llvm-bc")), + ("ir-output", Some("llvm-ir")), + ("link-output", Some("link")), + ("obj-output", Some("obj")), + ("dep-output", Some("dep-info")), + ("multi-output", Some("asm,obj")), + ]; + for (output_file, emit) in flags { + fs::remove_file(output_file).unwrap_or_default(); + compile(output_file, emit); + fs::remove_file(output_file); + } +} From dd7e68b3566c9417eb13ba8b09c74481727ff977 Mon Sep 17 00:00:00 2001 From: Oneirical Date: Fri, 17 May 2024 15:37:16 -0400 Subject: [PATCH 6/9] rewrite and rename issue-53964 --- src/tools/tidy/src/allowed_run_make_makefiles.txt | 1 - .../app.rs | 0 .../panic.rs | 0 .../external-crate-panic-handle-no-lint/rmake.rs | 12 ++++++++++++ tests/run-make/issue-53964/Makefile | 5 ----- 5 files changed, 12 insertions(+), 6 deletions(-) rename tests/run-make/{issue-53964 => external-crate-panic-handle-no-lint}/app.rs (100%) rename tests/run-make/{issue-53964 => external-crate-panic-handle-no-lint}/panic.rs (100%) create mode 100644 tests/run-make/external-crate-panic-handle-no-lint/rmake.rs delete mode 100644 tests/run-make/issue-53964/Makefile diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index 1a3d6f8d81360..4aed36c9e54f6 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -116,7 +116,6 @@ run-make/issue-46239/Makefile run-make/issue-47384/Makefile run-make/issue-47551/Makefile run-make/issue-51671/Makefile -run-make/issue-53964/Makefile run-make/issue-64153/Makefile run-make/issue-68794-textrel-on-minimal-lib/Makefile run-make/issue-69368/Makefile diff --git a/tests/run-make/issue-53964/app.rs b/tests/run-make/external-crate-panic-handle-no-lint/app.rs similarity index 100% rename from tests/run-make/issue-53964/app.rs rename to tests/run-make/external-crate-panic-handle-no-lint/app.rs diff --git a/tests/run-make/issue-53964/panic.rs b/tests/run-make/external-crate-panic-handle-no-lint/panic.rs similarity index 100% rename from tests/run-make/issue-53964/panic.rs rename to tests/run-make/external-crate-panic-handle-no-lint/panic.rs diff --git a/tests/run-make/external-crate-panic-handle-no-lint/rmake.rs b/tests/run-make/external-crate-panic-handle-no-lint/rmake.rs new file mode 100644 index 0000000000000..de4023282ef6e --- /dev/null +++ b/tests/run-make/external-crate-panic-handle-no-lint/rmake.rs @@ -0,0 +1,12 @@ +// Defining a crate that provides panic handling as an external crate +// could uselessly trigger the "unused external crate" lint. In this test, +// if the lint is triggered, it will trip #![deny(unused_extern_crates)], +// and cause the test to fail. +// See https://github.com/rust-lang/rust/issues/53964 + +use run_make_support::{rustc, tmp_dir}; + +fn main() { + rustc().input("panic.rs").run(); + rustc().input("app.rs").panic("abort").emit("obj").library_search_path(tmp_dir()).run(); +} diff --git a/tests/run-make/issue-53964/Makefile b/tests/run-make/issue-53964/Makefile deleted file mode 100644 index 6bd83021374d9..0000000000000 --- a/tests/run-make/issue-53964/Makefile +++ /dev/null @@ -1,5 +0,0 @@ -include ../tools.mk - -all: - $(RUSTC) panic.rs - $(RUSTC) -C panic=abort --emit=obj app.rs -L $(TMPDIR) From 96968350e17efdbb9958dbeaec4982d8cca0019d Mon Sep 17 00:00:00 2001 From: r0cky Date: Thu, 23 May 2024 09:06:43 +0800 Subject: [PATCH 7/9] Detect unused structs which implement private traits --- compiler/rustc_passes/src/dead.rs | 49 +++++++++++++------ .../lint/dead-code/unused-adt-impls-trait.rs | 34 +++++++++++++ .../dead-code/unused-adt-impls-trait.stderr | 14 ++++++ .../ui/rust-2021/inherent-dyn-collision.fixed | 1 + tests/ui/rust-2021/inherent-dyn-collision.rs | 1 + .../rust-2021/inherent-dyn-collision.stderr | 2 +- 6 files changed, 84 insertions(+), 17 deletions(-) create mode 100644 tests/ui/lint/dead-code/unused-adt-impls-trait.rs create mode 100644 tests/ui/lint/dead-code/unused-adt-impls-trait.stderr diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 933412f677ce8..ddc50e2b811a2 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -425,10 +425,11 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { && let ItemKind::Impl(impl_ref) = self.tcx.hir().expect_item(local_impl_id).kind { - if self.tcx.visibility(trait_id).is_public() - && matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) + if matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) && !ty_ref_to_pub_struct(self.tcx, impl_ref.self_ty) { + // skip methods of private ty, + // they would be solved in `solve_rest_impl_items` continue; } @@ -485,32 +486,46 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { fn solve_rest_impl_items(&mut self, mut unsolved_impl_items: Vec<(hir::ItemId, LocalDefId)>) { let mut ready; - (ready, unsolved_impl_items) = unsolved_impl_items - .into_iter() - .partition(|&(impl_id, _)| self.impl_item_with_used_self(impl_id)); + (ready, unsolved_impl_items) = + unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| { + self.impl_item_with_used_self(impl_id, impl_item_id) + }); while !ready.is_empty() { self.worklist = ready.into_iter().map(|(_, id)| (id, ComesFromAllowExpect::No)).collect(); self.mark_live_symbols(); - (ready, unsolved_impl_items) = unsolved_impl_items - .into_iter() - .partition(|&(impl_id, _)| self.impl_item_with_used_self(impl_id)); + (ready, unsolved_impl_items) = + unsolved_impl_items.into_iter().partition(|&(impl_id, impl_item_id)| { + self.impl_item_with_used_self(impl_id, impl_item_id) + }); } } - fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId) -> bool { + fn impl_item_with_used_self(&mut self, impl_id: hir::ItemId, impl_item_id: LocalDefId) -> bool { if let TyKind::Path(hir::QPath::Resolved(_, path)) = self.tcx.hir().item(impl_id).expect_impl().self_ty.kind && let Res::Def(def_kind, def_id) = path.res && let Some(local_def_id) = def_id.as_local() && matches!(def_kind, DefKind::Struct | DefKind::Enum | DefKind::Union) { - self.live_symbols.contains(&local_def_id) - } else { - false + if self.tcx.visibility(impl_item_id).is_public() { + // for the public method, we don't know the trait item is used or not, + // so we mark the method live if the self is used + return self.live_symbols.contains(&local_def_id); + } + + if let Some(trait_item_id) = self.tcx.associated_item(impl_item_id).trait_item_def_id + && let Some(local_id) = trait_item_id.as_local() + { + // for the private method, we can know the trait item is used or not, + // so we mark the method live if the self is used and the trait item is used + return self.live_symbols.contains(&local_id) + && self.live_symbols.contains(&local_def_id); + } } + false } } @@ -745,20 +760,22 @@ fn check_item<'tcx>( matches!(fn_sig.decl.implicit_self, hir::ImplicitSelfKind::None); } - // for impl trait blocks, mark associate functions live if the trait is public + // for trait impl blocks, + // mark the method live if the self_ty is public, + // or the method is public and may construct self if of_trait && (!matches!(tcx.def_kind(local_def_id), DefKind::AssocFn) || tcx.visibility(local_def_id).is_public() && (ty_is_pub || may_construct_self)) { worklist.push((local_def_id, ComesFromAllowExpect::No)); - } else if of_trait && tcx.visibility(local_def_id).is_public() { - // pub method && private ty & methods not construct self - unsolved_impl_items.push((id, local_def_id)); } else if let Some(comes_from_allow) = has_allow_dead_code_or_lang_attr(tcx, local_def_id) { worklist.push((local_def_id, comes_from_allow)); + } else if of_trait { + // private method || public method not constructs self + unsolved_impl_items.push((id, local_def_id)); } } } diff --git a/tests/ui/lint/dead-code/unused-adt-impls-trait.rs b/tests/ui/lint/dead-code/unused-adt-impls-trait.rs new file mode 100644 index 0000000000000..4714859afac4b --- /dev/null +++ b/tests/ui/lint/dead-code/unused-adt-impls-trait.rs @@ -0,0 +1,34 @@ +#![deny(dead_code)] + +struct Used; +struct Unused; //~ ERROR struct `Unused` is never constructed + +pub trait PubTrait { + fn foo(&self) -> Self; +} + +impl PubTrait for Used { + fn foo(&self) -> Self { Used } +} + +impl PubTrait for Unused { + fn foo(&self) -> Self { Unused } +} + +trait PriTrait { + fn foo(&self) -> Self; +} + +impl PriTrait for Used { + fn foo(&self) -> Self { Used } +} + +impl PriTrait for Unused { + fn foo(&self) -> Self { Unused } +} + +fn main() { + let t = Used; + let _t = ::foo(&t); + let _t = ::foo(&t); +} diff --git a/tests/ui/lint/dead-code/unused-adt-impls-trait.stderr b/tests/ui/lint/dead-code/unused-adt-impls-trait.stderr new file mode 100644 index 0000000000000..28bae5c2af09c --- /dev/null +++ b/tests/ui/lint/dead-code/unused-adt-impls-trait.stderr @@ -0,0 +1,14 @@ +error: struct `Unused` is never constructed + --> $DIR/unused-adt-impls-trait.rs:4:8 + | +LL | struct Unused; + | ^^^^^^ + | +note: the lint level is defined here + --> $DIR/unused-adt-impls-trait.rs:1:9 + | +LL | #![deny(dead_code)] + | ^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/rust-2021/inherent-dyn-collision.fixed b/tests/ui/rust-2021/inherent-dyn-collision.fixed index f5702613af032..8fe1bf811dbf3 100644 --- a/tests/ui/rust-2021/inherent-dyn-collision.fixed +++ b/tests/ui/rust-2021/inherent-dyn-collision.fixed @@ -25,6 +25,7 @@ mod inner { // having a struct of the same name as the trait in-scope, while *also* // implementing the trait for that struct but **without** importing the // trait itself into scope + #[allow(dead_code)] struct TryIntoU32; impl super::TryIntoU32 for TryIntoU32 { diff --git a/tests/ui/rust-2021/inherent-dyn-collision.rs b/tests/ui/rust-2021/inherent-dyn-collision.rs index 0bcb34e5708bb..47c8446241988 100644 --- a/tests/ui/rust-2021/inherent-dyn-collision.rs +++ b/tests/ui/rust-2021/inherent-dyn-collision.rs @@ -25,6 +25,7 @@ mod inner { // having a struct of the same name as the trait in-scope, while *also* // implementing the trait for that struct but **without** importing the // trait itself into scope + #[allow(dead_code)] struct TryIntoU32; impl super::TryIntoU32 for TryIntoU32 { diff --git a/tests/ui/rust-2021/inherent-dyn-collision.stderr b/tests/ui/rust-2021/inherent-dyn-collision.stderr index f5905574ac39d..d9e720dd9af8f 100644 --- a/tests/ui/rust-2021/inherent-dyn-collision.stderr +++ b/tests/ui/rust-2021/inherent-dyn-collision.stderr @@ -1,5 +1,5 @@ warning: trait method `try_into` will become ambiguous in Rust 2021 - --> $DIR/inherent-dyn-collision.rs:41:9 + --> $DIR/inherent-dyn-collision.rs:42:9 | LL | get_dyn_trait().try_into().unwrap() | ^^^^^^^^^^^^^^^ help: disambiguate the method call: `(&*get_dyn_trait())` From 6743fc7704d28a0be61643b1cada2fd31be132b5 Mon Sep 17 00:00:00 2001 From: Mohammad Omidvar Date: Thu, 23 May 2024 15:35:18 +0000 Subject: [PATCH 8/9] Add conversion from IntrinsicDef to FnDef --- compiler/stable_mir/src/ty.rs | 6 ++++++ tests/ui-fulldeps/stable-mir/check_intrinsics.rs | 16 ++++++++++------ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/compiler/stable_mir/src/ty.rs b/compiler/stable_mir/src/ty.rs index ab4deac9fd72c..ebfc6e026ebc7 100644 --- a/compiler/stable_mir/src/ty.rs +++ b/compiler/stable_mir/src/ty.rs @@ -653,6 +653,12 @@ impl IntrinsicDef { } } +impl From for FnDef { + fn from(def: IntrinsicDef) -> Self { + FnDef(def.0) + } +} + crate_def! { pub ClosureDef; } diff --git a/tests/ui-fulldeps/stable-mir/check_intrinsics.rs b/tests/ui-fulldeps/stable-mir/check_intrinsics.rs index a9c1f64f812d9..7e247ce0c75d2 100644 --- a/tests/ui-fulldeps/stable-mir/check_intrinsics.rs +++ b/tests/ui-fulldeps/stable-mir/check_intrinsics.rs @@ -24,11 +24,11 @@ use rustc_smir::rustc_internal; use stable_mir::mir::mono::{Instance, InstanceKind}; use stable_mir::mir::visit::{Location, MirVisitor}; use stable_mir::mir::{LocalDecl, Terminator, TerminatorKind}; -use stable_mir::ty::{FnDef, GenericArgs, IntrinsicDef, RigidTy, TyKind}; +use stable_mir::ty::{FnDef, GenericArgs, RigidTy, TyKind}; +use std::assert_matches::assert_matches; use std::convert::TryFrom; use std::io::Write; use std::ops::ControlFlow; -use std::assert_matches::assert_matches; /// This function tests that we can correctly get type information from binary operations. fn test_intrinsics() -> ControlFlow<()> { @@ -41,9 +41,9 @@ fn test_intrinsics() -> ControlFlow<()> { let calls = visitor.calls; assert_eq!(calls.len(), 3, "Expected 3 calls, but found: {calls:?}"); - for (fn_def, args) in &calls { - check_instance(&Instance::resolve(*fn_def, &args).unwrap()); - check_def(fn_def.as_intrinsic().unwrap()); + for (fn_def, args) in calls.into_iter() { + check_instance(&Instance::resolve(fn_def, &args).unwrap()); + check_def(fn_def); } ControlFlow::Continue(()) @@ -68,7 +68,11 @@ fn check_instance(instance: &Instance) { } } -fn check_def(intrinsic: IntrinsicDef) { +fn check_def(fn_def: FnDef) { + assert!(fn_def.is_intrinsic()); + let intrinsic = fn_def.as_intrinsic().unwrap(); + assert_eq!(fn_def, intrinsic.into()); + let name = intrinsic.fn_name(); match name.as_str() { "likely" | "size_of_val" => { From b1fa845d426bb9866851c8ea610c34b68a4111b4 Mon Sep 17 00:00:00 2001 From: r0cky Date: Thu, 23 May 2024 23:50:39 +0800 Subject: [PATCH 9/9] Improve the doc of query associated_item --- compiler/rustc_middle/src/query/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 0654d82838c9d..5a6decc4f9497 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -780,7 +780,7 @@ rustc_queries! { separate_provide_extern } - /// Maps from a trait item to the trait item "descriptor". + /// Maps from a trait/impl item to the trait/impl item "descriptor". query associated_item(key: DefId) -> ty::AssocItem { desc { |tcx| "computing associated item data for `{}`", tcx.def_path_str(key) } cache_on_disk_if { key.is_local() }