From 80faf203515bf3be214ad3ffc019b55b026b1220 Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Sun, 26 Jan 2025 03:23:02 -0500 Subject: [PATCH 01/23] add test for Box::default's stack usage in debug mode --- tests/codegen/box-default-debug-copies.rs | 25 +++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/codegen/box-default-debug-copies.rs diff --git a/tests/codegen/box-default-debug-copies.rs b/tests/codegen/box-default-debug-copies.rs new file mode 100644 index 0000000000000..ed4e0c416b8dc --- /dev/null +++ b/tests/codegen/box-default-debug-copies.rs @@ -0,0 +1,25 @@ +//@ compile-flags: -Copt-level=0 + +// Test to make sure that `>::default` does not create too many copies of `T` on the stack. +// in debug mode. This regressed in dd0620b86721ae8cae86736443acd3f72ba6fc32 to +// four `T` allocas. +// +// See https://github.com/rust-lang/rust/issues/136043 for more context. + +#![crate_type = "lib"] + +#[allow(dead_code)] +pub struct Thing([u8; 1000000]); + +impl Default for Thing { + fn default() -> Self { + Thing([0; 1000000]) + } +} + +// CHECK-COUNT-4: %{{.*}} = alloca {{.*}}1000000 +// CHECK-NOT: %{{.*}} = alloca {{.*}}1000000 +#[no_mangle] +pub fn box_default_single_copy() -> Box { + Box::default() +} From 97005678c38fd391c9b502d011cc3f3d4434a18a Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Sun, 26 Jan 2025 03:36:50 -0500 Subject: [PATCH 02/23] reduce `Box::default` stack copies in debug mode The `Box::new(T::default())` implementation of `Box::default` only had two stack copies in debug mode, compared to the current version, which has four. By avoiding creating any `MaybeUninit`'s and just writing `T` directly to the `Box` pointer, the stack usage in debug mode remains the same as the old version. --- library/alloc/src/boxed.rs | 15 ++++++++++++++- tests/codegen/box-default-debug-copies.rs | 5 ++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 1b5e44a913467..8f43ebc388889 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1730,7 +1730,20 @@ impl Default for Box { /// Creates a `Box`, with the `Default` value for T. #[inline] fn default() -> Self { - Box::write(Box::new_uninit(), T::default()) + let mut x: Box> = Box::new_uninit(); + unsafe { + // SAFETY: `x` is valid for writing and has the same layout as `T`. + // If `T::default()` panics, dropping `x` will just deallocate the Box as `MaybeUninit` + // does not have a destructor. + // + // We use `ptr::write` as `MaybeUninit::write` creates + // extra stack copies of `T` in debug mode. + // + // See https://github.com/rust-lang/rust/issues/136043 for more context. + ptr::write(&raw mut *x as *mut T, T::default()); + // SAFETY: `x` was just initialized above. + x.assume_init() + } } } diff --git a/tests/codegen/box-default-debug-copies.rs b/tests/codegen/box-default-debug-copies.rs index ed4e0c416b8dc..06cc41b21c06f 100644 --- a/tests/codegen/box-default-debug-copies.rs +++ b/tests/codegen/box-default-debug-copies.rs @@ -5,6 +5,9 @@ // four `T` allocas. // // See https://github.com/rust-lang/rust/issues/136043 for more context. +// +// FIXME: This test only wants to ensure that there are at most two allocas of `T` created, instead +// of checking for exactly two. #![crate_type = "lib"] @@ -17,7 +20,7 @@ impl Default for Thing { } } -// CHECK-COUNT-4: %{{.*}} = alloca {{.*}}1000000 +// CHECK-COUNT-2: %{{.*}} = alloca {{.*}}1000000 // CHECK-NOT: %{{.*}} = alloca {{.*}}1000000 #[no_mangle] pub fn box_default_single_copy() -> Box { From fe7ed278b7cfc8a152dcee29ca6ba583ff0d876d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jul 2024 00:23:54 +0000 Subject: [PATCH 03/23] Specify scope in `out_of_scope_macro_calls` lint ``` warning: cannot find macro `in_root` in the crate root --> $DIR/key-value-expansion-scope.rs:1:10 | LL | #![doc = in_root!()] | ^^^^^^^ not found in the crate root | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition = note: `#[warn(out_of_scope_macro_calls)]` on by default ``` --- compiler/rustc_lint/messages.ftl | 3 ++- compiler/rustc_lint/src/early/diagnostics.rs | 4 +-- compiler/rustc_lint/src/lints.rs | 3 +++ compiler/rustc_lint_defs/src/lib.rs | 2 ++ compiler/rustc_resolve/src/macros.rs | 26 +++++++++++++++---- .../attributes/key-value-expansion-scope.rs | 12 ++++----- .../key-value-expansion-scope.stderr | 24 ++++++++--------- 7 files changed, 48 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index dbe24c9cdf24c..1d8bdb9489917 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -630,7 +630,8 @@ lint_opaque_hidden_inferred_bound_sugg = add this bound lint_or_patterns_back_compat = the meaning of the `pat` fragment specifier is changing in Rust 2021, which may affect this macro .suggestion = use pat_param to preserve semantics -lint_out_of_scope_macro_calls = cannot find macro `{$path}` in this scope +lint_out_of_scope_macro_calls = cannot find macro `{$path}` in {$scope} + .label = not found in {$scope} .help = import `macro_rules` with `use` to make it callable above its definition lint_overflowing_bin_hex = literal out of range for `{$ty}` diff --git a/compiler/rustc_lint/src/early/diagnostics.rs b/compiler/rustc_lint/src/early/diagnostics.rs index aeb5a03a4f7e3..d1345467d1034 100644 --- a/compiler/rustc_lint/src/early/diagnostics.rs +++ b/compiler/rustc_lint/src/early/diagnostics.rs @@ -444,8 +444,8 @@ pub(super) fn decorate_lint( lints::InnerAttributeUnstable::CustomInnerAttribute } .decorate_lint(diag), - BuiltinLintDiag::OutOfScopeMacroCalls { path } => { - lints::OutOfScopeMacroCalls { path }.decorate_lint(diag) + BuiltinLintDiag::OutOfScopeMacroCalls { span, path, scope } => { + lints::OutOfScopeMacroCalls { span, path, scope }.decorate_lint(diag) } BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by } => { lints::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by }.decorate_lint(diag) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 368d36bfdd0b2..afd3117d6bc12 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -3108,7 +3108,10 @@ pub(crate) struct UnsafeAttrOutsideUnsafeSuggestion { #[diag(lint_out_of_scope_macro_calls)] #[help] pub(crate) struct OutOfScopeMacroCalls { + #[label] + pub span: Span, pub path: String, + pub scope: String, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 7ffe4e4e4901c..efa6a934b0065 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -819,7 +819,9 @@ pub enum BuiltinLintDiag { is_macro: bool, }, OutOfScopeMacroCalls { + span: Span, path: String, + scope: String, }, UnexpectedBuiltinCfg { cfg: String, diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index cca01a01e9877..07d1d377b4196 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -857,8 +857,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { ), path_res @ (PathResult::NonModule(..) | PathResult::Failed { .. }) => { let mut suggestion = None; - let (span, label, module) = - if let PathResult::Failed { span, label, module, .. } = path_res { + let (span, label, module, segment) = + if let PathResult::Failed { span, label, module, segment_name, .. } = + path_res + { // try to suggest if it's not a macro, maybe a function if let PathResult::NonModule(partial_res) = self.maybe_resolve_path(&path, Some(ValueNS), &parent_scope, None) @@ -876,7 +878,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { Applicability::MaybeIncorrect, )); } - (span, label, module) + (span, label, module, segment_name) } else { ( path_span, @@ -886,12 +888,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { kind.descr() ), None, + path.last().map(|segment| segment.ident.name).unwrap(), ) }; self.report_error( span, ResolutionError::FailedToResolve { - segment: path.last().map(|segment| segment.ident.name), + segment: Some(segment), label, suggestion, module, @@ -1067,11 +1070,24 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { None, ); if fallback_binding.ok().and_then(|b| b.res().opt_def_id()) != Some(def_id) { + let scope = match parent_scope.module.kind { + ModuleKind::Def(_, _, name) if name == kw::Empty => { + "the crate root".to_string() + } + ModuleKind::Def(kind, def_id, name) => { + format!("{} `{name}`", kind.descr(def_id)) + } + ModuleKind::Block => "this scope".to_string(), + }; self.tcx.sess.psess.buffer_lint( OUT_OF_SCOPE_MACRO_CALLS, path.span, node_id, - BuiltinLintDiag::OutOfScopeMacroCalls { path: pprust::path_to_string(path) }, + BuiltinLintDiag::OutOfScopeMacroCalls { + span: path.span, + path: pprust::path_to_string(path), + scope, + }, ); } } diff --git a/tests/ui/attributes/key-value-expansion-scope.rs b/tests/ui/attributes/key-value-expansion-scope.rs index b6eab1571d490..49a59502377f5 100644 --- a/tests/ui/attributes/key-value-expansion-scope.rs +++ b/tests/ui/attributes/key-value-expansion-scope.rs @@ -1,7 +1,7 @@ -#![doc = in_root!()] //~ WARN cannot find macro `in_root` in this scope +#![doc = in_root!()] //~ WARN cannot find macro `in_root` //~| WARN this was previously accepted by the compiler #![doc = in_mod!()] //~ ERROR cannot find macro `in_mod` in this scope -#![doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` in this scope +#![doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` //~| WARN this was previously accepted by the compiler #![doc = in_block!()] //~ ERROR cannot find macro `in_block` in this scope @@ -18,10 +18,10 @@ fn before() { macro_rules! in_root { () => { "" } } -#[doc = in_mod!()] //~ WARN cannot find macro `in_mod` in this scope +#[doc = in_mod!()] //~ WARN cannot find macro `in_mod` //~| WARN this was previously accepted by the compiler mod macros_stay { - #![doc = in_mod!()] //~ WARN cannot find macro `in_mod` in this scope + #![doc = in_mod!()] //~ WARN cannot find macro `in_mod` //~| WARN this was previously accepted by the compiler macro_rules! in_mod { () => { "" } } @@ -33,10 +33,10 @@ mod macros_stay { } #[macro_use] -#[doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` in this scope +#[doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` //~| WARN this was previously accepted by the compiler mod macros_escape { - #![doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` in this scope + #![doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` //~| WARN this was previously accepted by the compiler macro_rules! in_mod_escape { () => { "" } } diff --git a/tests/ui/attributes/key-value-expansion-scope.stderr b/tests/ui/attributes/key-value-expansion-scope.stderr index d22fef7dd251e..172b2c54cc6b7 100644 --- a/tests/ui/attributes/key-value-expansion-scope.stderr +++ b/tests/ui/attributes/key-value-expansion-scope.stderr @@ -126,62 +126,62 @@ LL | #![doc = in_block!()] | = help: have you added the `#[macro_use]` on the module/import? -warning: cannot find macro `in_root` in this scope +warning: cannot find macro `in_root` in the crate root --> $DIR/key-value-expansion-scope.rs:1:10 | LL | #![doc = in_root!()] - | ^^^^^^^ + | ^^^^^^^ not found in the crate root | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition = note: `#[warn(out_of_scope_macro_calls)]` on by default -warning: cannot find macro `in_mod_escape` in this scope +warning: cannot find macro `in_mod_escape` in the crate root --> $DIR/key-value-expansion-scope.rs:4:10 | LL | #![doc = in_mod_escape!()] - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ not found in the crate root | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod` in this scope +warning: cannot find macro `in_mod` in module `macros_stay` --> $DIR/key-value-expansion-scope.rs:21:9 | LL | #[doc = in_mod!()] - | ^^^^^^ + | ^^^^^^ not found in module `macros_stay` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod` in this scope +warning: cannot find macro `in_mod` in module `macros_stay` --> $DIR/key-value-expansion-scope.rs:24:14 | LL | #![doc = in_mod!()] - | ^^^^^^ + | ^^^^^^ not found in module `macros_stay` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod_escape` in this scope +warning: cannot find macro `in_mod_escape` in module `macros_escape` --> $DIR/key-value-expansion-scope.rs:36:9 | LL | #[doc = in_mod_escape!()] - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ not found in module `macros_escape` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod_escape` in this scope +warning: cannot find macro `in_mod_escape` in module `macros_escape` --> $DIR/key-value-expansion-scope.rs:39:14 | LL | #![doc = in_mod_escape!()] - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ not found in module `macros_escape` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 From c9fbaab4533f109ce4e7b56ab8d3df0a49c46ffb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 14:16:22 +1100 Subject: [PATCH 04/23] Reflow `MirPhase` comments. Currently many of them exceed 100 chars, which makes them painful to read on a terminal that is 100 chars wide. --- compiler/rustc_middle/src/mir/syntax.rs | 47 +++++++++++++------------ 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 389792a04f8a0..69c19ae7b9c2a 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -53,7 +53,8 @@ pub enum MirPhase { /// sequences of statements that would generally be subject to constant promotion are /// semantically constants, while in analysis MIR all constants are explicit. /// - /// The result of const promotion is available from the `mir_promoted` and `promoted_mir` queries. + /// The result of const promotion is available from the `mir_promoted` and `promoted_mir` + /// queries. /// /// This is the version of MIR used by borrowck and friends. Analysis(AnalysisPhase), @@ -61,26 +62,27 @@ pub enum MirPhase { /// /// The semantic changes that occur in the lowering from analysis to runtime MIR are as follows: /// - /// - Drops: In analysis MIR, `Drop` terminators represent *conditional* drops; roughly speaking, - /// if dataflow analysis determines that the place being dropped is uninitialized, the drop will - /// not be executed. The exact semantics of this aren't written down anywhere, which means they - /// are essentially "what drop elaboration does." In runtime MIR, the drops are unconditional; - /// when a `Drop` terminator is reached, if the type has drop glue that drop glue is always - /// executed. This may be UB if the underlying place is not initialized. - /// - Packed drops: Places might in general be misaligned - in most cases this is UB, the exception - /// is fields of packed structs. In analysis MIR, `Drop(P)` for a `P` that might be misaligned - /// for this reason implicitly moves `P` to a temporary before dropping. Runtime MIR has no such - /// rules, and dropping a misaligned place is simply UB. - /// - Unwinding: in analysis MIR, unwinding from a function which may not unwind aborts. In runtime - /// MIR, this is UB. - /// - Retags: If `-Zmir-emit-retag` is enabled, analysis MIR has "implicit" retags in the same way - /// that Rust itself has them. Where exactly these are is generally subject to change, and so we - /// don't document this here. Runtime MIR has most retags explicit (though implicit retags - /// can still occur at `Rvalue::{Ref,AddrOf}`). - /// - Coroutine bodies: In analysis MIR, locals may actually be behind a pointer that user code has - /// access to. This occurs in coroutine bodies. Such locals do not behave like other locals, - /// because they eg may be aliased in surprising ways. Runtime MIR has no such special locals - - /// all coroutine bodies are lowered and so all places that look like locals really are locals. + /// - Drops: In analysis MIR, `Drop` terminators represent *conditional* drops; roughly + /// speaking, if dataflow analysis determines that the place being dropped is uninitialized, + /// the drop will not be executed. The exact semantics of this aren't written down anywhere, + /// which means they are essentially "what drop elaboration does." In runtime MIR, the drops + /// are unconditional; when a `Drop` terminator is reached, if the type has drop glue that + /// drop glue is always executed. This may be UB if the underlying place is not initialized. + /// - Packed drops: Places might in general be misaligned - in most cases this is UB, the + /// exception is fields of packed structs. In analysis MIR, `Drop(P)` for a `P` that might be + /// misaligned for this reason implicitly moves `P` to a temporary before dropping. Runtime + /// MIR has no such rules, and dropping a misaligned place is simply UB. + /// - Unwinding: in analysis MIR, unwinding from a function which may not unwind aborts. In + /// runtime MIR, this is UB. + /// - Retags: If `-Zmir-emit-retag` is enabled, analysis MIR has "implicit" retags in the same + /// way that Rust itself has them. Where exactly these are is generally subject to change, + /// and so we don't document this here. Runtime MIR has most retags explicit (though implicit + /// retags can still occur at `Rvalue::{Ref,AddrOf}`). + /// - Coroutine bodies: In analysis MIR, locals may actually be behind a pointer that user code + /// has access to. This occurs in coroutine bodies. Such locals do not behave like other + /// locals, because they eg may be aliased in surprising ways. Runtime MIR has no such + /// special locals. All coroutine bodies are lowered and so all places that look like locals + /// really are locals. /// /// Also note that the lint pass which reports eg `200_u8 + 200_u8` as an error is run as a part /// of analysis to runtime MIR lowering. To ensure lints are reported reliably, this means that @@ -111,7 +113,8 @@ pub enum AnalysisPhase { /// * [`TerminatorKind::FalseEdge`] /// * [`StatementKind::FakeRead`] /// * [`StatementKind::AscribeUserType`] - /// * [`StatementKind::Coverage`] with [`CoverageKind::BlockMarker`] or [`CoverageKind::SpanMarker`] + /// * [`StatementKind::Coverage`] with [`CoverageKind::BlockMarker`] or + /// [`CoverageKind::SpanMarker`] /// * [`Rvalue::Ref`] with `BorrowKind::Fake` /// * [`CastKind::PointerCoercion`] with any of the following: /// * [`PointerCoercion::ArrayToPointer`] From c039533656d344b465cf95da721197a6fdd3d7f7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 14:33:59 +1100 Subject: [PATCH 05/23] Improve MIR phase comments. I found the dialect/phase distinction quite confusing when I first read these comments. This commit clarifies things a bit. --- compiler/rustc_middle/src/mir/syntax.rs | 51 ++++++++++++++----------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 69c19ae7b9c2a..b63c4d172f2e7 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -23,44 +23,49 @@ use crate::ty::{self, GenericArgsRef, List, Region, Ty, UserTypeAnnotationIndex} /// Represents the "flavors" of MIR. /// -/// All flavors of MIR use the same data structure, but there are some important differences. These -/// differences come in two forms: Dialects and phases. +/// The MIR pipeline is structured into a few major dialects, with one or more phases within each +/// dialect. A MIR flavor is identified by a dialect-phase pair. A single `MirPhase` value +/// specifies such a pair. All flavors of MIR use the same data structure to represent the program. /// -/// Dialects represent a stronger distinction than phases. This is because the transitions between -/// dialects are semantic changes, and therefore technically *lowerings* between distinct IRs. In -/// other words, the same [`Body`](crate::mir::Body) might be well-formed for multiple dialects, but -/// have different semantic meaning and different behavior at runtime. +/// Different MIR dialects have different semantics. (The differences between dialects are small, +/// but they do exist.) The progression from one MIR dialect to the next is technically a lowering +/// from one IR to another. In other words, a single well-formed [`Body`](crate::mir::Body) might +/// have different semantic meaning and different behavior at runtime in the different dialects. +/// The specific differences between dialects are described on the variants below. /// -/// Each dialect additionally has a number of phases. However, phase changes never involve semantic -/// changes. If some MIR is well-formed both before and after a phase change, it is also guaranteed -/// that it has the same semantic meaning. In this sense, phase changes can only add additional -/// restrictions on what MIR is well-formed. +/// Phases exist only to place restrictions on what language constructs are permitted in +/// well-formed MIR, and subsequent phases mostly increase those restrictions. I.e. to convert MIR +/// from one phase to the next might require removing/replacing certain MIR constructs. /// -/// When adding phases, remember to update [`MirPhase::phase_index`]. +/// When adding dialects or phases, remember to update [`MirPhase::phase_index`]. #[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)] #[derive(HashStable)] pub enum MirPhase { - /// The MIR that is generated by MIR building. + /// The "built MIR" dialect, as generated by MIR building. /// /// The only things that operate on this dialect are unsafeck, the various MIR lints, and const /// qualifs. /// - /// This has no distinct phases. + /// This dialect has just the one (implicit) phase, which places few restrictions on what MIR + /// constructs are allowed. Built, - /// The MIR used for most analysis. + + /// The "analysis MIR" dialect, used for borrowck and friends. /// - /// The only semantic change between analysis and built MIR is constant promotion. In built MIR, - /// sequences of statements that would generally be subject to constant promotion are - /// semantically constants, while in analysis MIR all constants are explicit. + /// The only semantic difference between built MIR and analysis MIR relates to constant + /// promotion. In built MIR, sequences of statements that would generally be subject to + /// constant promotion are semantically constants, while in analysis MIR all constants are + /// explicit. /// /// The result of const promotion is available from the `mir_promoted` and `promoted_mir` /// queries. /// - /// This is the version of MIR used by borrowck and friends. + /// The phases of this dialect are described in `AnalysisPhase`. Analysis(AnalysisPhase), - /// The MIR used for CTFE, optimizations, and codegen. + + /// The "runtime MIR" dialect, used for CTFE, optimizations, and codegen. /// - /// The semantic changes that occur in the lowering from analysis to runtime MIR are as follows: + /// The semantic differences between analysis MIR and runtime MIR are as follows. /// /// - Drops: In analysis MIR, `Drop` terminators represent *conditional* drops; roughly /// speaking, if dataflow analysis determines that the place being dropped is uninitialized, @@ -80,13 +85,15 @@ pub enum MirPhase { /// retags can still occur at `Rvalue::{Ref,AddrOf}`). /// - Coroutine bodies: In analysis MIR, locals may actually be behind a pointer that user code /// has access to. This occurs in coroutine bodies. Such locals do not behave like other - /// locals, because they eg may be aliased in surprising ways. Runtime MIR has no such + /// locals, because they e.g. may be aliased in surprising ways. Runtime MIR has no such /// special locals. All coroutine bodies are lowered and so all places that look like locals /// really are locals. /// /// Also note that the lint pass which reports eg `200_u8 + 200_u8` as an error is run as a part /// of analysis to runtime MIR lowering. To ensure lints are reported reliably, this means that - /// transformations which may suppress such errors should not run on analysis MIR. + /// transformations that can suppress such errors should not run on analysis MIR. + /// + /// The phases of this dialect are described in `RuntimePhase`. Runtime(RuntimePhase), } From 83a7fb61fbf2377961d05cad10a699fd954a4bb7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Feb 2025 15:17:50 +1100 Subject: [PATCH 06/23] Improve how the MIR dialect/phase index is reported. The only visible change is to the filenames produce by `-Zdump-mir`. E.g. before and after: ``` h.main.003-000.analysis-post-cleanup.after.mir h.main.2-2-000.analysis-post-cleanup.after.mir ``` It also fixes a FIXME comment. --- compiler/rustc_middle/src/mir/mod.rs | 21 +++++++-------------- compiler/rustc_middle/src/mir/pretty.rs | 3 ++- compiler/rustc_middle/src/mir/syntax.rs | 2 +- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 528da4ca05776..096422369dc05 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -99,20 +99,13 @@ impl<'tcx> HasLocalDecls<'tcx> for Body<'tcx> { } impl MirPhase { - /// Gets the index of the current MirPhase within the set of all `MirPhase`s. - /// - /// FIXME(JakobDegen): Return a `(usize, usize)` instead. - pub fn phase_index(&self) -> usize { - const BUILT_PHASE_COUNT: usize = 1; - const ANALYSIS_PHASE_COUNT: usize = 2; - match self { - MirPhase::Built => 1, - MirPhase::Analysis(analysis_phase) => { - 1 + BUILT_PHASE_COUNT + (*analysis_phase as usize) - } - MirPhase::Runtime(runtime_phase) => { - 1 + BUILT_PHASE_COUNT + ANALYSIS_PHASE_COUNT + (*runtime_phase as usize) - } + /// Gets the (dialect, phase) index of the current `MirPhase`. Both numbers + /// are 1-indexed. + pub fn index(&self) -> (usize, usize) { + match *self { + MirPhase::Built => (1, 1), + MirPhase::Analysis(analysis_phase) => (2, 1 + analysis_phase as usize), + MirPhase::Runtime(runtime_phase) => (3, 1 + runtime_phase as usize), } } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 1e3b8d029e1b3..dd3de48fc14fd 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -231,7 +231,8 @@ fn dump_path<'tcx>( let pass_num = if tcx.sess.opts.unstable_opts.dump_mir_exclude_pass_number { String::new() } else if pass_num { - format!(".{:03}-{:03}", body.phase.phase_index(), body.pass_count) + let (dialect_index, phase_index) = body.phase.index(); + format!(".{}-{}-{:03}", dialect_index, phase_index, body.pass_count) } else { ".-------".to_string() }; diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index b63c4d172f2e7..af6f0e4c55183 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -37,7 +37,7 @@ use crate::ty::{self, GenericArgsRef, List, Region, Ty, UserTypeAnnotationIndex} /// well-formed MIR, and subsequent phases mostly increase those restrictions. I.e. to convert MIR /// from one phase to the next might require removing/replacing certain MIR constructs. /// -/// When adding dialects or phases, remember to update [`MirPhase::phase_index`]. +/// When adding dialects or phases, remember to update [`MirPhase::index`]. #[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, PartialEq, Eq, PartialOrd, Ord)] #[derive(HashStable)] pub enum MirPhase { From dc4f948299b58e1d34c0267c17eea09a1119e8c7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2025 15:39:15 +1100 Subject: [PATCH 07/23] Move `StatementAsExpression` to where it's actually used. Also minimize some visibilities in the destination file. --- compiler/rustc_middle/src/traits/mod.rs | 6 ------ .../src/error_reporting/infer/suggest.rs | 21 ++++++++++++------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index f039da772fd4d..23c06f712322a 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -514,12 +514,6 @@ impl<'tcx> ObligationCauseCode<'tcx> { #[cfg(target_pointer_width = "64")] rustc_data_structures::static_assert_size!(ObligationCauseCode<'_>, 48); -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] -pub enum StatementAsExpression { - CorrectType, - NeedsBoxing, -} - #[derive(Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)] #[derive(TypeVisitable, TypeFoldable)] pub struct MatchExpressionArmCause<'tcx> { diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/suggest.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/suggest.rs index 628888c8d45c0..a87a449daf1c7 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/suggest.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/infer/suggest.rs @@ -10,7 +10,6 @@ use rustc_hir::def::Res; use rustc_hir::{MatchSource, Node}; use rustc_middle::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, - StatementAsExpression, }; use rustc_middle::ty::error::TypeError; use rustc_middle::ty::print::with_no_trimmed_paths; @@ -26,8 +25,14 @@ use crate::errors::{ SuggestTuplePatternMany, SuggestTuplePatternOne, TypeErrorAdditionalDiags, }; +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +enum StatementAsExpression { + CorrectType, + NeedsBoxing, +} + #[derive(Clone, Copy)] -pub enum SuggestAsRefKind { +enum SuggestAsRefKind { Option, Result, } @@ -382,7 +387,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } - pub fn suggest_function_pointers_impl( + pub(crate) fn suggest_function_pointers_impl( &self, span: Option, exp_found: &ty::error::ExpectedFound>, @@ -518,7 +523,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } - pub fn should_suggest_as_ref_kind( + fn should_suggest_as_ref_kind( &self, expected: Ty<'tcx>, found: Ty<'tcx>, @@ -588,8 +593,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ) -> Option { /// Find the if expression with given span struct IfVisitor { - pub found_if: bool, - pub err_span: Span, + found_if: bool, + err_span: Span, } impl<'v> Visitor<'v> for IfVisitor { @@ -736,7 +741,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { impl<'tcx> TypeErrCtxt<'_, 'tcx> { /// Be helpful when the user wrote `{... expr; }` and taking the `;` off /// is enough to fix the error. - pub fn could_remove_semicolon( + fn could_remove_semicolon( &self, blk: &'tcx hir::Block<'tcx>, expected_ty: Ty<'tcx>, @@ -816,7 +821,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { /// Suggest returning a local binding with a compatible type if the block /// has no return expression. - pub fn consider_returning_binding_diag( + fn consider_returning_binding_diag( &self, blk: &'tcx hir::Block<'tcx>, expected_ty: Ty<'tcx>, From 0895fe20e2f8ea4603fddae35412e0bc00511457 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2025 16:31:16 +1100 Subject: [PATCH 08/23] Remove unused items from `query.rs`. --- compiler/rustc_middle/src/traits/query.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/compiler/rustc_middle/src/traits/query.rs b/compiler/rustc_middle/src/traits/query.rs index 4203c8fd86149..76f3d2bab9cf4 100644 --- a/compiler/rustc_middle/src/traits/query.rs +++ b/compiler/rustc_middle/src/traits/query.rs @@ -75,12 +75,6 @@ pub type CanonicalPredicateGoal<'tcx> = pub type CanonicalTypeOpAscribeUserTypeGoal<'tcx> = CanonicalQueryInput<'tcx, ty::ParamEnvAnd<'tcx, type_op::AscribeUserType<'tcx>>>; -pub type CanonicalTypeOpEqGoal<'tcx> = - CanonicalQueryInput<'tcx, ty::ParamEnvAnd<'tcx, type_op::Eq<'tcx>>>; - -pub type CanonicalTypeOpSubtypeGoal<'tcx> = - CanonicalQueryInput<'tcx, ty::ParamEnvAnd<'tcx, type_op::Subtype<'tcx>>>; - pub type CanonicalTypeOpProvePredicateGoal<'tcx> = CanonicalQueryInput<'tcx, ty::ParamEnvAnd<'tcx, type_op::ProvePredicate<'tcx>>>; From cae9ebbe1e09c2878d56ab1425d3765f450fd5fe Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Feb 2025 14:08:52 +1100 Subject: [PATCH 09/23] Simplify `Postorder` customization. `Postorder` has a `C: Customization<'tcx>` parameter, that gives it flexibility about how it computes successors. But in practice, there are only two `impls` of `Customization`, and one is for the unit type. This commit simplifies things by removing the generic parameter and replacing it with an `Option`. --- compiler/rustc_middle/src/mir/basic_blocks.rs | 2 +- compiler/rustc_middle/src/mir/traversal.rs | 47 +++++-------------- src/tools/clippy/clippy_utils/src/mir/mod.rs | 2 +- 3 files changed, 15 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_middle/src/mir/basic_blocks.rs b/compiler/rustc_middle/src/mir/basic_blocks.rs index 107c3198525a1..171542d1279c7 100644 --- a/compiler/rustc_middle/src/mir/basic_blocks.rs +++ b/compiler/rustc_middle/src/mir/basic_blocks.rs @@ -79,7 +79,7 @@ impl<'tcx> BasicBlocks<'tcx> { #[inline] pub fn reverse_postorder(&self) -> &[BasicBlock] { self.cache.reverse_postorder.get_or_init(|| { - let mut rpo: Vec<_> = Postorder::new(&self.basic_blocks, START_BLOCK, ()).collect(); + let mut rpo: Vec<_> = Postorder::new(&self.basic_blocks, START_BLOCK, None).collect(); rpo.reverse(); rpo }) diff --git a/compiler/rustc_middle/src/mir/traversal.rs b/compiler/rustc_middle/src/mir/traversal.rs index 0e7dcc24dafa6..5950ac295af19 100644 --- a/compiler/rustc_middle/src/mir/traversal.rs +++ b/compiler/rustc_middle/src/mir/traversal.rs @@ -104,23 +104,21 @@ impl<'a, 'tcx> Iterator for Preorder<'a, 'tcx> { /// ``` /// /// A Postorder traversal of this graph is `D B C A` or `D C B A` -pub struct Postorder<'a, 'tcx, C> { +pub struct Postorder<'a, 'tcx> { basic_blocks: &'a IndexSlice>, visited: DenseBitSet, visit_stack: Vec<(BasicBlock, Successors<'a>)>, root_is_start_block: bool, - extra: C, + /// A non-empty `extra` allows for a precise calculation of the successors. + extra: Option<(TyCtxt<'tcx>, Instance<'tcx>)>, } -impl<'a, 'tcx, C> Postorder<'a, 'tcx, C> -where - C: Customization<'tcx>, -{ +impl<'a, 'tcx> Postorder<'a, 'tcx> { pub fn new( basic_blocks: &'a IndexSlice>, root: BasicBlock, - extra: C, - ) -> Postorder<'a, 'tcx, C> { + extra: Option<(TyCtxt<'tcx>, Instance<'tcx>)>, + ) -> Postorder<'a, 'tcx> { let mut po = Postorder { basic_blocks, visited: DenseBitSet::new_empty(basic_blocks.len()), @@ -140,7 +138,11 @@ where return; } let data = &self.basic_blocks[bb]; - let successors = C::successors(data, self.extra); + let successors = if let Some(extra) = self.extra { + data.mono_successors(extra.0, extra.1) + } else { + data.terminator().successors() + }; self.visit_stack.push((bb, successors)); } @@ -198,10 +200,7 @@ where } } -impl<'tcx, C> Iterator for Postorder<'_, 'tcx, C> -where - C: Customization<'tcx>, -{ +impl<'tcx> Iterator for Postorder<'_, 'tcx> { type Item = BasicBlock; fn next(&mut self) -> Option { @@ -241,32 +240,12 @@ pub fn postorder<'a, 'tcx>( reverse_postorder(body).rev() } -/// Lets us plug in some additional logic and data into a Postorder traversal. Or not. -pub trait Customization<'tcx>: Copy { - fn successors<'a>(_: &'a BasicBlockData<'tcx>, _: Self) -> Successors<'a>; -} - -impl<'tcx> Customization<'tcx> for () { - fn successors<'a>(data: &'a BasicBlockData<'tcx>, _: ()) -> Successors<'a> { - data.terminator().successors() - } -} - -impl<'tcx> Customization<'tcx> for (TyCtxt<'tcx>, Instance<'tcx>) { - fn successors<'a>( - data: &'a BasicBlockData<'tcx>, - (tcx, instance): (TyCtxt<'tcx>, Instance<'tcx>), - ) -> Successors<'a> { - data.mono_successors(tcx, instance) - } -} - pub fn mono_reachable_reverse_postorder<'a, 'tcx>( body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, ) -> Vec { - let mut iter = Postorder::new(&body.basic_blocks, START_BLOCK, (tcx, instance)); + let mut iter = Postorder::new(&body.basic_blocks, START_BLOCK, Some((tcx, instance))); let mut items = Vec::with_capacity(body.basic_blocks.len()); while let Some(block) = iter.next() { items.push(block); diff --git a/src/tools/clippy/clippy_utils/src/mir/mod.rs b/src/tools/clippy/clippy_utils/src/mir/mod.rs index 637c0bafd964a..ffcfcd240ea5a 100644 --- a/src/tools/clippy/clippy_utils/src/mir/mod.rs +++ b/src/tools/clippy/clippy_utils/src/mir/mod.rs @@ -30,7 +30,7 @@ pub fn visit_local_usage(locals: &[Local], mir: &Body<'_>, location: Location) - locals.len() ]; - traversal::Postorder::new(&mir.basic_blocks, location.block, ()) + traversal::Postorder::new(&mir.basic_blocks, location.block, None) .collect::>() .into_iter() .rev() From d4609f8d4298ec3068b7dc957bad1df91031f347 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 20 Feb 2025 03:36:58 +0000 Subject: [PATCH 10/23] Use a probe to avoid registering stray region obligations when re-checking drops in MIR typeck --- .../src/type_check/liveness/trace.rs | 22 ++++++++++++------- compiler/rustc_infer/src/infer/mod.rs | 2 +- tests/ui/borrowck/bad-drop-side-effects.rs | 18 +++++++++++++++ .../ui/borrowck/bad-drop-side-effects.stderr | 9 ++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) create mode 100644 tests/ui/borrowck/bad-drop-side-effects.rs create mode 100644 tests/ui/borrowck/bad-drop-side-effects.stderr diff --git a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs index 148e75aa84cc7..12e8be41614bd 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/trace.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/trace.rs @@ -613,9 +613,14 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> { // types, so there's no guarantee that it succeeds. We also // can't rely on the the `ErrorGuaranteed` from `fully_perform` here // because it comes from delay_span_bug. - let ocx = ObligationCtxt::new_with_diagnostics(&typeck.infcx); - let errors = - match dropck_outlives::compute_dropck_outlives_with_errors(&ocx, op, span) { + // + // Do this inside of a probe because we don't particularly care (or want) + // any region side-effects of this operation in our infcx. + typeck.infcx.probe(|_| { + let ocx = ObligationCtxt::new_with_diagnostics(&typeck.infcx); + let errors = match dropck_outlives::compute_dropck_outlives_with_errors( + &ocx, op, span, + ) { Ok(_) => ocx.select_all_or_error(), Err(e) => { if e.is_empty() { @@ -626,11 +631,12 @@ impl<'tcx> LivenessContext<'_, '_, '_, 'tcx> { } }; - if !errors.is_empty() { - typeck.infcx.err_ctxt().report_fulfillment_errors(errors); - } else { - span_bug!(span, "Rerunning drop data query produced no error."); - } + if !errors.is_empty() { + typeck.infcx.err_ctxt().report_fulfillment_errors(errors); + } else { + span_bug!(span, "Rerunning drop data query produced no error."); + } + }); DropData { dropck_result: Default::default(), region_constraint_data: None } } } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index c2513a1af1988..88a325a39be52 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -950,7 +950,7 @@ impl<'tcx> InferCtxt<'tcx> { let inner = self.inner.borrow(); assert!(!UndoLogs::>::in_snapshot(&inner.undo_log)); let storage = inner.region_constraint_storage.as_ref().expect("regions already resolved"); - assert!(storage.data.is_empty()); + assert!(storage.data.is_empty(), "{:#?}", storage.data); // We clone instead of taking because borrowck still wants to use the // inference context after calling this for diagnostics and the new // trait solver. diff --git a/tests/ui/borrowck/bad-drop-side-effects.rs b/tests/ui/borrowck/bad-drop-side-effects.rs new file mode 100644 index 0000000000000..a09b7087608f6 --- /dev/null +++ b/tests/ui/borrowck/bad-drop-side-effects.rs @@ -0,0 +1,18 @@ +// Regression test for . + +trait B { + type C; +} + +impl B for &Missing { +//~^ ERROR cannot find type `Missing` in this scope + type C = (); +} + +struct E { + g: ::C, +} + +fn h(i: Box>) {} + +fn main() {} diff --git a/tests/ui/borrowck/bad-drop-side-effects.stderr b/tests/ui/borrowck/bad-drop-side-effects.stderr new file mode 100644 index 0000000000000..0a5998c7e4830 --- /dev/null +++ b/tests/ui/borrowck/bad-drop-side-effects.stderr @@ -0,0 +1,9 @@ +error[E0412]: cannot find type `Missing` in this scope + --> $DIR/bad-drop-side-effects.rs:7:16 + | +LL | impl B for &Missing { + | ^^^^^^^ not found in this scope + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0412`. From ea7180813b4dface2bc02a9095ce1ebb8c64d4de Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 14 Feb 2025 17:27:49 +0000 Subject: [PATCH 11/23] Create safe helper for LLVMSetDLLStorageClass --- compiler/rustc_codegen_llvm/src/callee.rs | 6 ++---- compiler/rustc_codegen_llvm/src/consts.rs | 8 ++------ compiler/rustc_codegen_llvm/src/llvm/mod.rs | 6 ++++++ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/callee.rs b/compiler/rustc_codegen_llvm/src/callee.rs index aa9a0f34f55e9..90075e1c10552 100644 --- a/compiler/rustc_codegen_llvm/src/callee.rs +++ b/compiler/rustc_codegen_llvm/src/callee.rs @@ -66,9 +66,7 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t // LLVM will prefix the name with `__imp_`. Ideally, we'd like the // existing logic below to set the Storage Class, but it has an // exemption for MinGW for backwards compatibility. - unsafe { - llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport); - } + llvm::set_dllimport_storage_class(llfn); llfn } else { cx.declare_fn(sym, fn_abi, Some(instance)) @@ -150,7 +148,7 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t && library.kind.is_dllimport() && !matches!(tcx.sess.target.env.as_ref(), "gnu" | "uclibc") { - llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport); + llvm::set_dllimport_storage_class(llfn); } if cx.should_assume_dso_local(llfn, true) { diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 4a5491ec7a18c..5ab437d27c208 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -375,9 +375,7 @@ impl<'ll> CodegenCx<'ll, '_> { // is actually present in the current crate. We can find out via the // is_codegened_item query. if !self.tcx.is_codegened_item(def_id) { - unsafe { - llvm::LLVMSetDLLStorageClass(g, llvm::DLLStorageClass::DllImport); - } + llvm::set_dllimport_storage_class(g); } } } @@ -387,9 +385,7 @@ impl<'ll> CodegenCx<'ll, '_> { && library.kind.is_dllimport() { // For foreign (native) libs we know the exact storage type to use. - unsafe { - llvm::LLVMSetDLLStorageClass(g, llvm::DLLStorageClass::DllImport); - } + llvm::set_dllimport_storage_class(g); } self.instances.borrow_mut().insert(instance, g); diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index efc9cf2ef691b..7f57e64063794 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -403,3 +403,9 @@ pub(crate) fn add_module_flag_str( ); } } + +pub(crate) fn set_dllimport_storage_class<'ll>(v: &'ll Value) { + unsafe { + LLVMSetDLLStorageClass(v, DLLStorageClass::DllImport); + } +} From ce7f58bd91548fd8674aaceb670f827de26d4216 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 14 Feb 2025 17:27:49 +0000 Subject: [PATCH 12/23] Merge two operations that were always performed together --- compiler/rustc_codegen_llvm/src/back/lto.rs | 2 +- compiler/rustc_codegen_llvm/src/callee.rs | 102 +++++++++---------- compiler/rustc_codegen_llvm/src/consts.rs | 11 +- compiler/rustc_codegen_llvm/src/llvm/mod.rs | 6 ++ compiler/rustc_codegen_llvm/src/mono_item.rs | 25 +++-- 5 files changed, 70 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs index 7262fce491191..3e25b94961b00 100644 --- a/compiler/rustc_codegen_llvm/src/back/lto.rs +++ b/compiler/rustc_codegen_llvm/src/back/lto.rs @@ -362,8 +362,8 @@ fn fat_lto( ptr as *const *const libc::c_char, symbols_below_threshold.len() as libc::size_t, ); - save_temp_bitcode(cgcx, &module, "lto.after-restriction"); } + save_temp_bitcode(cgcx, &module, "lto.after-restriction"); } Ok(LtoModuleCodegen::Fat(module)) diff --git a/compiler/rustc_codegen_llvm/src/callee.rs b/compiler/rustc_codegen_llvm/src/callee.rs index 90075e1c10552..ea9ab5c02bd6c 100644 --- a/compiler/rustc_codegen_llvm/src/callee.rs +++ b/compiler/rustc_codegen_llvm/src/callee.rs @@ -97,65 +97,61 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t // has been applied to the definition (wherever that definition may be). llvm::set_linkage(llfn, llvm::Linkage::ExternalLinkage); - unsafe { - let is_generic = instance.args.non_erasable_generics().next().is_some(); - - let is_hidden = if is_generic { - // This is a monomorphization of a generic function. - if !(cx.tcx.sess.opts.share_generics() - || tcx.codegen_fn_attrs(instance_def_id).inline - == rustc_attr_parsing::InlineAttr::Never) - { - // When not sharing generics, all instances are in the same - // crate and have hidden visibility. - true - } else { - if let Some(instance_def_id) = instance_def_id.as_local() { - // This is a monomorphization of a generic function - // defined in the current crate. It is hidden if: - // - the definition is unreachable for downstream - // crates, or - // - the current crate does not re-export generics - // (because the crate is a C library or executable) - cx.tcx.is_unreachable_local_definition(instance_def_id) - || !cx.tcx.local_crate_exports_generics() - } else { - // This is a monomorphization of a generic function - // defined in an upstream crate. It is hidden if: - // - it is instantiated in this crate, and - // - the current crate does not re-export generics - instance.upstream_monomorphization(tcx).is_none() - && !cx.tcx.local_crate_exports_generics() - } - } - } else { - // This is a non-generic function. It is hidden if: - // - it is instantiated in the local crate, and - // - it is defined an upstream crate (non-local), or - // - it is not reachable - cx.tcx.is_codegened_item(instance_def_id) - && (!instance_def_id.is_local() - || !cx.tcx.is_reachable_non_generic(instance_def_id)) - }; - if is_hidden { - llvm::set_visibility(llfn, llvm::Visibility::Hidden); - } + let is_generic = instance.args.non_erasable_generics().next().is_some(); - // MinGW: For backward compatibility we rely on the linker to decide whether it - // should use dllimport for functions. - if cx.use_dll_storage_attrs - && let Some(library) = tcx.native_library(instance_def_id) - && library.kind.is_dllimport() - && !matches!(tcx.sess.target.env.as_ref(), "gnu" | "uclibc") + let is_hidden = if is_generic { + // This is a monomorphization of a generic function. + if !(cx.tcx.sess.opts.share_generics() + || tcx.codegen_fn_attrs(instance_def_id).inline + == rustc_attr_parsing::InlineAttr::Never) { - llvm::set_dllimport_storage_class(llfn); + // When not sharing generics, all instances are in the same + // crate and have hidden visibility. + true + } else { + if let Some(instance_def_id) = instance_def_id.as_local() { + // This is a monomorphization of a generic function + // defined in the current crate. It is hidden if: + // - the definition is unreachable for downstream + // crates, or + // - the current crate does not re-export generics + // (because the crate is a C library or executable) + cx.tcx.is_unreachable_local_definition(instance_def_id) + || !cx.tcx.local_crate_exports_generics() + } else { + // This is a monomorphization of a generic function + // defined in an upstream crate. It is hidden if: + // - it is instantiated in this crate, and + // - the current crate does not re-export generics + instance.upstream_monomorphization(tcx).is_none() + && !cx.tcx.local_crate_exports_generics() + } } + } else { + // This is a non-generic function. It is hidden if: + // - it is instantiated in the local crate, and + // - it is defined an upstream crate (non-local), or + // - it is not reachable + cx.tcx.is_codegened_item(instance_def_id) + && (!instance_def_id.is_local() + || !cx.tcx.is_reachable_non_generic(instance_def_id)) + }; + if is_hidden { + llvm::set_visibility(llfn, llvm::Visibility::Hidden); + } - if cx.should_assume_dso_local(llfn, true) { - llvm::LLVMRustSetDSOLocal(llfn, true); - } + // MinGW: For backward compatibility we rely on the linker to decide whether it + // should use dllimport for functions. + if cx.use_dll_storage_attrs + && let Some(library) = tcx.native_library(instance_def_id) + && library.kind.is_dllimport() + && !matches!(tcx.sess.target.env.as_ref(), "gnu" | "uclibc") + { + llvm::set_dllimport_storage_class(llfn); } + cx.assume_dso_local(llfn, true); + llfn }; diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 5ab437d27c208..330e8a8f4069d 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -336,12 +336,7 @@ impl<'ll> CodegenCx<'ll, '_> { llvm::set_thread_local_mode(g, self.tls_model); } - let dso_local = self.should_assume_dso_local(g, true); - if dso_local { - unsafe { - llvm::LLVMRustSetDSOLocal(g, true); - } - } + let dso_local = self.assume_dso_local(g, true); if !def_id.is_local() { let needs_dll_storage_attr = self.use_dll_storage_attrs @@ -456,9 +451,7 @@ impl<'ll> CodegenCx<'ll, '_> { set_global_alignment(self, g, alloc.align); llvm::set_initializer(g, v); - if self.should_assume_dso_local(g, true) { - llvm::LLVMRustSetDSOLocal(g, true); - } + self.assume_dso_local(g, true); // Forward the allocation's mutability (picked by the const interner) to LLVM. if alloc.mutability.is_not() { diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index 7f57e64063794..5ec934241314c 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -409,3 +409,9 @@ pub(crate) fn set_dllimport_storage_class<'ll>(v: &'ll Value) { LLVMSetDLLStorageClass(v, DLLStorageClass::DllImport); } } + +pub(crate) fn set_dso_local<'ll>(v: &'ll Value) { + unsafe { + LLVMRustSetDSOLocal(v, true); + } +} diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index 70edee21bd63a..a64627eaf5986 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -38,11 +38,7 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { llvm::set_linkage(g, base::linkage_to_llvm(linkage)); llvm::set_visibility(g, base::visibility_to_llvm(visibility)); - unsafe { - if self.should_assume_dso_local(g, false) { - llvm::LLVMRustSetDSOLocal(g, true); - } - } + self.assume_dso_local(g, false); self.instances.borrow_mut().insert(instance, g); } @@ -79,9 +75,7 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { debug!("predefine_fn: instance = {:?}", instance); - if self.should_assume_dso_local(lldecl, false) { - unsafe { llvm::LLVMRustSetDSOLocal(lldecl, true) }; - } + self.assume_dso_local(lldecl, false); self.instances.borrow_mut().insert(instance, lldecl); } @@ -90,11 +84,16 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { impl CodegenCx<'_, '_> { /// Whether a definition or declaration can be assumed to be local to a group of /// libraries that form a single DSO or executable. - pub(crate) fn should_assume_dso_local( - &self, - llval: &llvm::Value, - is_declaration: bool, - ) -> bool { + /// Marks the local as DSO if so. + pub(crate) fn assume_dso_local(&self, llval: &llvm::Value, is_declaration: bool) -> bool { + let assume = self.should_assume_dso_local(llval, is_declaration); + if assume { + llvm::set_dso_local(llval); + } + assume + } + + fn should_assume_dso_local(&self, llval: &llvm::Value, is_declaration: bool) -> bool { let linkage = llvm::get_linkage(llval); let visibility = llvm::get_visibility(llval); From 835d434c79559f68e22a6ddf6394b84f4fd353cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 19 Feb 2025 18:52:53 +0000 Subject: [PATCH 13/23] Reword message --- compiler/rustc_lint/messages.ftl | 4 ++-- compiler/rustc_lint/src/early/diagnostics.rs | 4 ++-- compiler/rustc_lint/src/lints.rs | 2 +- compiler/rustc_lint_defs/src/lib.rs | 2 +- compiler/rustc_resolve/src/macros.rs | 4 ++-- .../key-value-expansion-scope.stderr | 24 +++++++++---------- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 1d8bdb9489917..679367634276d 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -630,8 +630,8 @@ lint_opaque_hidden_inferred_bound_sugg = add this bound lint_or_patterns_back_compat = the meaning of the `pat` fragment specifier is changing in Rust 2021, which may affect this macro .suggestion = use pat_param to preserve semantics -lint_out_of_scope_macro_calls = cannot find macro `{$path}` in {$scope} - .label = not found in {$scope} +lint_out_of_scope_macro_calls = cannot find macro `{$path}` in the current scope when looking from {$location} + .label = not found from {$location} .help = import `macro_rules` with `use` to make it callable above its definition lint_overflowing_bin_hex = literal out of range for `{$ty}` diff --git a/compiler/rustc_lint/src/early/diagnostics.rs b/compiler/rustc_lint/src/early/diagnostics.rs index d1345467d1034..40ca9e05d95d6 100644 --- a/compiler/rustc_lint/src/early/diagnostics.rs +++ b/compiler/rustc_lint/src/early/diagnostics.rs @@ -444,8 +444,8 @@ pub(super) fn decorate_lint( lints::InnerAttributeUnstable::CustomInnerAttribute } .decorate_lint(diag), - BuiltinLintDiag::OutOfScopeMacroCalls { span, path, scope } => { - lints::OutOfScopeMacroCalls { span, path, scope }.decorate_lint(diag) + BuiltinLintDiag::OutOfScopeMacroCalls { span, path, location } => { + lints::OutOfScopeMacroCalls { span, path, location }.decorate_lint(diag) } BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by } => { lints::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by }.decorate_lint(diag) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index afd3117d6bc12..0058630957291 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -3111,7 +3111,7 @@ pub(crate) struct OutOfScopeMacroCalls { #[label] pub span: Span, pub path: String, - pub scope: String, + pub location: String, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index efa6a934b0065..66fdba28502bd 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -821,7 +821,7 @@ pub enum BuiltinLintDiag { OutOfScopeMacroCalls { span: Span, path: String, - scope: String, + location: String, }, UnexpectedBuiltinCfg { cfg: String, diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 07d1d377b4196..a70def2f6c93c 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -1070,7 +1070,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { None, ); if fallback_binding.ok().and_then(|b| b.res().opt_def_id()) != Some(def_id) { - let scope = match parent_scope.module.kind { + let location = match parent_scope.module.kind { ModuleKind::Def(_, _, name) if name == kw::Empty => { "the crate root".to_string() } @@ -1086,7 +1086,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> { BuiltinLintDiag::OutOfScopeMacroCalls { span: path.span, path: pprust::path_to_string(path), - scope, + location, }, ); } diff --git a/tests/ui/attributes/key-value-expansion-scope.stderr b/tests/ui/attributes/key-value-expansion-scope.stderr index 172b2c54cc6b7..91a602e57d9e4 100644 --- a/tests/ui/attributes/key-value-expansion-scope.stderr +++ b/tests/ui/attributes/key-value-expansion-scope.stderr @@ -126,62 +126,62 @@ LL | #![doc = in_block!()] | = help: have you added the `#[macro_use]` on the module/import? -warning: cannot find macro `in_root` in the crate root +warning: cannot find macro `in_root` in the current scope when looking from the crate root --> $DIR/key-value-expansion-scope.rs:1:10 | LL | #![doc = in_root!()] - | ^^^^^^^ not found in the crate root + | ^^^^^^^ not found from the crate root | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition = note: `#[warn(out_of_scope_macro_calls)]` on by default -warning: cannot find macro `in_mod_escape` in the crate root +warning: cannot find macro `in_mod_escape` in the current scope when looking from the crate root --> $DIR/key-value-expansion-scope.rs:4:10 | LL | #![doc = in_mod_escape!()] - | ^^^^^^^^^^^^^ not found in the crate root + | ^^^^^^^^^^^^^ not found from the crate root | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod` in module `macros_stay` +warning: cannot find macro `in_mod` in the current scope when looking from module `macros_stay` --> $DIR/key-value-expansion-scope.rs:21:9 | LL | #[doc = in_mod!()] - | ^^^^^^ not found in module `macros_stay` + | ^^^^^^ not found from module `macros_stay` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod` in module `macros_stay` +warning: cannot find macro `in_mod` in the current scope when looking from module `macros_stay` --> $DIR/key-value-expansion-scope.rs:24:14 | LL | #![doc = in_mod!()] - | ^^^^^^ not found in module `macros_stay` + | ^^^^^^ not found from module `macros_stay` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod_escape` in module `macros_escape` +warning: cannot find macro `in_mod_escape` in the current scope when looking from module `macros_escape` --> $DIR/key-value-expansion-scope.rs:36:9 | LL | #[doc = in_mod_escape!()] - | ^^^^^^^^^^^^^ not found in module `macros_escape` + | ^^^^^^^^^^^^^ not found from module `macros_escape` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 = help: import `macro_rules` with `use` to make it callable above its definition -warning: cannot find macro `in_mod_escape` in module `macros_escape` +warning: cannot find macro `in_mod_escape` in the current scope when looking from module `macros_escape` --> $DIR/key-value-expansion-scope.rs:39:14 | LL | #![doc = in_mod_escape!()] - | ^^^^^^^^^^^^^ not found in module `macros_escape` + | ^^^^^^^^^^^^^ not found from module `macros_escape` | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #124535 From c2dba9ce787c088066ae81f51c4088714bb5e69d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 6 Feb 2025 16:33:55 +1100 Subject: [PATCH 14/23] Rename `InternedObligationCauseCode`. It's a misleading name, because it's not interned. --- compiler/rustc_middle/src/traits/mod.rs | 32 +++++++++++++------------ 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 23c06f712322a..53bc9eb7e466c 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -51,7 +51,7 @@ pub struct ObligationCause<'tcx> { /// information. pub body_id: LocalDefId, - code: InternedObligationCauseCode<'tcx>, + code: ObligationCauseCodeHandle<'tcx>, } // This custom hash function speeds up hashing for `Obligation` deduplication @@ -97,7 +97,7 @@ impl<'tcx> ObligationCause<'tcx> { pub fn map_code( &mut self, - f: impl FnOnce(InternedObligationCauseCode<'tcx>) -> ObligationCauseCode<'tcx>, + f: impl FnOnce(ObligationCauseCodeHandle<'tcx>) -> ObligationCauseCode<'tcx>, ) { self.code = f(std::mem::take(&mut self.code)).into(); } @@ -152,15 +152,16 @@ pub struct UnifyReceiverContext<'tcx> { pub args: GenericArgsRef<'tcx>, } +/// A compact form of `ObligationCauseCode`. #[derive(Clone, PartialEq, Eq, Default, HashStable)] #[derive(TypeVisitable, TypeFoldable, TyEncodable, TyDecodable)] -pub struct InternedObligationCauseCode<'tcx> { +pub struct ObligationCauseCodeHandle<'tcx> { /// `None` for `ObligationCauseCode::Misc` (a common case, occurs ~60% of /// the time). `Some` otherwise. code: Option>>, } -impl<'tcx> std::fmt::Debug for InternedObligationCauseCode<'tcx> { +impl<'tcx> std::fmt::Debug for ObligationCauseCodeHandle<'tcx> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let cause: &ObligationCauseCode<'_> = self; cause.fmt(f) @@ -169,14 +170,14 @@ impl<'tcx> std::fmt::Debug for InternedObligationCauseCode<'tcx> { impl<'tcx> ObligationCauseCode<'tcx> { #[inline(always)] - fn into(self) -> InternedObligationCauseCode<'tcx> { - InternedObligationCauseCode { + fn into(self) -> ObligationCauseCodeHandle<'tcx> { + ObligationCauseCodeHandle { code: if let ObligationCauseCode::Misc = self { None } else { Some(Arc::new(self)) }, } } } -impl<'tcx> std::ops::Deref for InternedObligationCauseCode<'tcx> { +impl<'tcx> std::ops::Deref for ObligationCauseCodeHandle<'tcx> { type Target = ObligationCauseCode<'tcx>; fn deref(&self) -> &Self::Target { @@ -305,7 +306,7 @@ pub enum ObligationCauseCode<'tcx> { /// The node of the function call. call_hir_id: HirId, /// The obligation introduced by this argument. - parent_code: InternedObligationCauseCode<'tcx>, + parent_code: ObligationCauseCodeHandle<'tcx>, }, /// Error derived when checking an impl item is compatible with @@ -390,7 +391,8 @@ pub enum ObligationCauseCode<'tcx> { /// `WellFormed(None)`. WellFormed(Option), - /// From `match_impl`. The cause for us having to match an impl, and the DefId we are matching against. + /// From `match_impl`. The cause for us having to match an impl, and the DefId we are matching + /// against. MatchImpl(ObligationCause<'tcx>, DefId), BinOp { @@ -413,7 +415,7 @@ pub enum ObligationCauseCode<'tcx> { ConstParam(Ty<'tcx>), /// Obligations emitted during the normalization of a weak type alias. - TypeAlias(InternedObligationCauseCode<'tcx>, Span, DefId), + TypeAlias(ObligationCauseCodeHandle<'tcx>, Span, DefId), } /// Whether a value can be extracted into a const. @@ -578,7 +580,7 @@ pub struct DerivedCause<'tcx> { pub parent_trait_pred: ty::PolyTraitPredicate<'tcx>, /// The parent trait had this cause. - pub parent_code: InternedObligationCauseCode<'tcx>, + pub parent_code: ObligationCauseCodeHandle<'tcx>, } #[derive(Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)] @@ -586,9 +588,9 @@ pub struct DerivedCause<'tcx> { pub struct ImplDerivedCause<'tcx> { pub derived: DerivedCause<'tcx>, /// The `DefId` of the `impl` that gave rise to the `derived` obligation. - /// If the `derived` obligation arose from a trait alias, which conceptually has a synthetic impl, - /// then this will be the `DefId` of that trait alias. Care should therefore be taken to handle - /// that exceptional case where appropriate. + /// If the `derived` obligation arose from a trait alias, which conceptually has a synthetic + /// impl, then this will be the `DefId` of that trait alias. Care should therefore be taken to + /// handle that exceptional case where appropriate. pub impl_or_alias_def_id: DefId, /// The index of the derived predicate in the parent impl's predicates. pub impl_def_predicate_index: Option, @@ -605,7 +607,7 @@ pub struct DerivedHostCause<'tcx> { pub parent_host_pred: ty::Binder<'tcx, ty::HostEffectPredicate<'tcx>>, /// The parent trait had this cause. - pub parent_code: InternedObligationCauseCode<'tcx>, + pub parent_code: ObligationCauseCodeHandle<'tcx>, } #[derive(Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable)] From e2e4d0bdb1c04faa538fea2db097c9a3b0b4df41 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 6 Feb 2025 16:37:02 +1100 Subject: [PATCH 15/23] Remove an unnecessary re-export. It's a bit weird. --- compiler/rustc_infer/src/infer/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index c2513a1af1988..bc701871d46d8 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -28,7 +28,6 @@ use rustc_middle::bug; use rustc_middle::infer::canonical::{CanonicalQueryInput, CanonicalVarValues}; use rustc_middle::mir::ConstraintCategory; use rustc_middle::traits::select; -pub use rustc_middle::ty::IntVarValue; use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::fold::{ BoundVarReplacerDelegate, TypeFoldable, TypeFolder, TypeSuperFoldable, fold_regions, From c301ba57a652ffdf8f39d9ddd92cf168e594f21c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 12 Feb 2025 17:00:57 +1100 Subject: [PATCH 16/23] Fix a typo in a comment. --- compiler/rustc_middle/src/mir/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 528da4ca05776..214fa3f38db37 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -116,7 +116,7 @@ impl MirPhase { } } - /// Parses an `MirPhase` from a pair of strings. Panics if this isn't possible for any reason. + /// Parses a `MirPhase` from a pair of strings. Panics if this isn't possible for any reason. pub fn parse(dialect: String, phase: Option) -> Self { match &*dialect.to_ascii_lowercase() { "built" => { From 2f695dc64ec7912ebc93c0322566a0ac2a6a21b4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 13 Feb 2025 11:09:46 +1100 Subject: [PATCH 17/23] Remove unused `Body::span_for_ty_context` method. --- compiler/rustc_middle/src/mir/mod.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 214fa3f38db37..2e8d3f213947f 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -30,7 +30,6 @@ use rustc_span::{DUMMY_SP, Span, Symbol}; use tracing::{debug, trace}; pub use self::query::*; -use self::visit::TyContext; use crate::mir::interpret::{AllocRange, Scalar}; use crate::mir::visit::MirVisitable; use crate::ty::codec::{TyDecoder, TyEncoder}; @@ -540,17 +539,6 @@ impl<'tcx> Body<'tcx> { } } - pub fn span_for_ty_context(&self, ty_context: TyContext) -> Span { - match ty_context { - TyContext::UserTy(span) => span, - TyContext::ReturnTy(source_info) - | TyContext::LocalDecl { source_info, .. } - | TyContext::YieldTy(source_info) - | TyContext::ResumeTy(source_info) => source_info.span, - TyContext::Location(loc) => self.source_info(loc).span, - } - } - /// Returns the return type; it always return first element from `local_decls` array. #[inline] pub fn return_ty(&self) -> Ty<'tcx> { From 5d2d11fd5da7216595154b7a67ca4477281f65a9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 13 Feb 2025 14:00:39 +1100 Subject: [PATCH 18/23] Rename `ClearCrossCrate::assert_crate_local`. As `unwrap_crate_local`, because it follows exactly the standard form of an `unwrap` function. --- compiler/rustc_middle/src/mir/mod.rs | 4 ++-- compiler/rustc_mir_build/src/builder/expr/as_operand.rs | 2 +- compiler/rustc_mir_build/src/builder/expr/as_temp.rs | 2 +- compiler/rustc_mir_build/src/builder/matches/mod.rs | 2 +- compiler/rustc_mir_build/src/builder/mod.rs | 4 ++-- compiler/rustc_mir_build/src/builder/scope.rs | 4 ++-- compiler/rustc_mir_transform/src/check_const_item_mutation.rs | 2 +- compiler/rustc_mir_transform/src/coroutine.rs | 2 +- compiler/rustc_mir_transform/src/ffi_unwind_calls.rs | 2 +- compiler/rustc_mir_transform/src/function_item_references.rs | 2 +- compiler/rustc_mir_transform/src/patch.rs | 2 +- src/tools/clippy/clippy_lints/src/redundant_clone.rs | 2 +- 12 files changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 2e8d3f213947f..20cf358fd2dc1 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -780,7 +780,7 @@ impl ClearCrossCrate { } } - pub fn assert_crate_local(self) -> T { + pub fn unwrap_crate_local(self) -> T { match self { ClearCrossCrate::Clear => bug!("unwrapping cross-crate data"), ClearCrossCrate::Set(v) => v, @@ -1094,7 +1094,7 @@ pub enum LocalInfo<'tcx> { impl<'tcx> LocalDecl<'tcx> { pub fn local_info(&self) -> &LocalInfo<'tcx> { - self.local_info.as_ref().assert_crate_local() + self.local_info.as_ref().unwrap_crate_local() } /// Returns `true` only if local is a binding that can itself be diff --git a/compiler/rustc_mir_build/src/builder/expr/as_operand.rs b/compiler/rustc_mir_build/src/builder/expr/as_operand.rs index 63e9b1dc6cd5a..2059610ee47d7 100644 --- a/compiler/rustc_mir_build/src/builder/expr/as_operand.rs +++ b/compiler/rustc_mir_build/src/builder/expr/as_operand.rs @@ -142,7 +142,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Overwrite temp local info if we have something more interesting to record. if !matches!(local_info, LocalInfo::Boring) { let decl_info = - this.local_decls[operand].local_info.as_mut().assert_crate_local(); + this.local_decls[operand].local_info.as_mut().unwrap_crate_local(); if let LocalInfo::Boring | LocalInfo::BlockTailTemp(_) = **decl_info { **decl_info = local_info; } diff --git a/compiler/rustc_mir_build/src/builder/expr/as_temp.rs b/compiler/rustc_mir_build/src/builder/expr/as_temp.rs index 2927f5b0c45d5..0bd61168fba05 100644 --- a/compiler/rustc_mir_build/src/builder/expr/as_temp.rs +++ b/compiler/rustc_mir_build/src/builder/expr/as_temp.rs @@ -85,7 +85,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { _ => LocalInfo::Boring, }; - **local_decl.local_info.as_mut().assert_crate_local() = local_info; + **local_decl.local_info.as_mut().unwrap_crate_local() = local_info; this.local_decls.push(local_decl) }; debug!(?temp); diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index ed577f7adeb3d..828dd9f363a62 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -722,7 +722,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if let LocalInfo::User(BindingForm::Var(VarBindingForm { opt_match_place: Some((ref mut match_place, _)), .. - })) = **self.local_decls[local].local_info.as_mut().assert_crate_local() + })) = **self.local_decls[local].local_info.as_mut().unwrap_crate_local() { *match_place = Some(place); } else { diff --git a/compiler/rustc_mir_build/src/builder/mod.rs b/compiler/rustc_mir_build/src/builder/mod.rs index fb0aa354913f4..2fe643e478e97 100644 --- a/compiler/rustc_mir_build/src/builder/mod.rs +++ b/compiler/rustc_mir_build/src/builder/mod.rs @@ -967,7 +967,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } => { self.local_decls[local].mutability = mutability; self.local_decls[local].source_info.scope = self.source_scope; - **self.local_decls[local].local_info.as_mut().assert_crate_local() = + **self.local_decls[local].local_info.as_mut().unwrap_crate_local() = if let Some(kind) = param.self_kind { LocalInfo::User(BindingForm::ImplicitSelf(kind)) } else { @@ -1032,7 +1032,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let parent_id = self.source_scopes[original_source_scope] .local_data .as_ref() - .assert_crate_local() + .unwrap_crate_local() .lint_root; self.maybe_new_source_scope(pattern_span, arg_hir_id, parent_id); } diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs index d3551ea3a97b4..8156123949121 100644 --- a/compiler/rustc_mir_build/src/builder/scope.rs +++ b/compiler/rustc_mir_build/src/builder/scope.rs @@ -604,7 +604,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let source_scope = self.source_scope; if let LintLevel::Explicit(current_hir_id) = lint_level { let parent_id = - self.source_scopes[source_scope].local_data.as_ref().assert_crate_local().lint_root; + self.source_scopes[source_scope].local_data.as_ref().unwrap_crate_local().lint_root; self.maybe_new_source_scope(region_scope.1.span, current_hir_id, parent_id); } self.push_scope(region_scope); @@ -992,7 +992,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { lint_root: if let LintLevel::Explicit(lint_root) = lint_level { lint_root } else { - self.source_scopes[parent].local_data.as_ref().assert_crate_local().lint_root + self.source_scopes[parent].local_data.as_ref().unwrap_crate_local().lint_root }, }; self.source_scopes.push(SourceScopeData { diff --git a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs index 3affe4abbfad8..ceea72c6755a0 100644 --- a/compiler/rustc_mir_transform/src/check_const_item_mutation.rs +++ b/compiler/rustc_mir_transform/src/check_const_item_mutation.rs @@ -79,7 +79,7 @@ impl<'tcx> ConstMutationChecker<'_, 'tcx> { let lint_root = self.body.source_scopes[source_info.scope] .local_data .as_ref() - .assert_crate_local() + .unwrap_crate_local() .lint_root; Some((lint_root, source_info.span, self.tcx.def_span(const_item))) diff --git a/compiler/rustc_mir_transform/src/coroutine.rs b/compiler/rustc_mir_transform/src/coroutine.rs index f3f3a65cd805d..04d96f117072f 100644 --- a/compiler/rustc_mir_transform/src/coroutine.rs +++ b/compiler/rustc_mir_transform/src/coroutine.rs @@ -945,7 +945,7 @@ fn compute_layout<'tcx>( let decl = &body.local_decls[local]; debug!(?decl); - // Do not `assert_crate_local` here, as post-borrowck cleanup may have already cleared + // Do not `unwrap_crate_local` here, as post-borrowck cleanup may have already cleared // the information. This is alright, since `ignore_for_traits` is only relevant when // this code runs on pre-cleanup MIR, and `ignore_for_traits = false` is the safer // default. diff --git a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs index 5d21d687a35a8..7b3553e7afd06 100644 --- a/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs +++ b/compiler/rustc_mir_transform/src/ffi_unwind_calls.rs @@ -85,7 +85,7 @@ fn has_ffi_unwind_calls(tcx: TyCtxt<'_>, local_def_id: LocalDefId) -> bool { let lint_root = body.source_scopes[terminator.source_info.scope] .local_data .as_ref() - .assert_crate_local() + .unwrap_crate_local() .lint_root; let span = terminator.source_info.span; diff --git a/compiler/rustc_mir_transform/src/function_item_references.rs b/compiler/rustc_mir_transform/src/function_item_references.rs index 73e47bb79f0bf..38b5ccdb32e77 100644 --- a/compiler/rustc_mir_transform/src/function_item_references.rs +++ b/compiler/rustc_mir_transform/src/function_item_references.rs @@ -154,7 +154,7 @@ impl<'tcx> FunctionItemRefChecker<'_, 'tcx> { let lint_root = self.body.source_scopes[source_info.scope] .local_data .as_ref() - .assert_crate_local() + .unwrap_crate_local() .lint_root; // FIXME: use existing printing routines to print the function signature let fn_sig = self.tcx.fn_sig(fn_id).instantiate(self.tcx, fn_args); diff --git a/compiler/rustc_mir_transform/src/patch.rs b/compiler/rustc_mir_transform/src/patch.rs index d3d181f6cb2b1..6a177faeac81f 100644 --- a/compiler/rustc_mir_transform/src/patch.rs +++ b/compiler/rustc_mir_transform/src/patch.rs @@ -158,7 +158,7 @@ impl<'tcx> MirPatch<'tcx> { let index = self.next_local; self.next_local += 1; let mut new_decl = LocalDecl::new(ty, span); - **new_decl.local_info.as_mut().assert_crate_local() = local_info; + **new_decl.local_info.as_mut().unwrap_crate_local() = local_info; self.new_locals.push(new_decl); Local::new(index) } diff --git a/src/tools/clippy/clippy_lints/src/redundant_clone.rs b/src/tools/clippy/clippy_lints/src/redundant_clone.rs index fb1bc494bd948..cfa622aea582f 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_clone.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_clone.rs @@ -205,7 +205,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone { let node = mir.source_scopes[scope] .local_data .as_ref() - .assert_crate_local() + .unwrap_crate_local() .lint_root; if let Some(snip) = span.get_source_text(cx) From c49e2df668838a38d9919a2457cde4e47e0f89fe Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 13 Feb 2025 14:27:06 +1100 Subject: [PATCH 19/23] Put a `BlockTailInfo` in `BlockFrame::TailExpr`. Because it has the same fields, and avoids the need to deconstruct the latter to construct the former. --- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_mir_build/src/builder/block.rs | 5 +++-- .../rustc_mir_build/src/builder/expr/stmt.rs | 3 +-- compiler/rustc_mir_build/src/builder/mod.rs | 21 +++++-------------- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 20cf358fd2dc1..afde812fbf510 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -937,7 +937,7 @@ mod binding_form_impl { /// involved in borrow_check errors, e.g., explanations of where the /// temporaries come from, when their destructors are run, and/or how /// one might revise the code to satisfy the borrow checker's rules. -#[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, TyEncodable, TyDecodable, HashStable)] pub struct BlockTailInfo { /// If `true`, then the value resulting from evaluating this tail /// expression is ignored by the block's expression context. diff --git a/compiler/rustc_mir_build/src/builder/block.rs b/compiler/rustc_mir_build/src/builder/block.rs index ba63a97de89fb..93ee90011a581 100644 --- a/compiler/rustc_mir_build/src/builder/block.rs +++ b/compiler/rustc_mir_build/src/builder/block.rs @@ -331,8 +331,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let expr = &this.thir[expr_id]; let tail_result_is_ignored = destination_ty.is_unit() || this.block_context.currently_ignores_tail_results(); - this.block_context - .push(BlockFrame::TailExpr { tail_result_is_ignored, span: expr.span }); + this.block_context.push(BlockFrame::TailExpr { + info: BlockTailInfo { tail_result_is_ignored, span: expr.span }, + }); block = this.expr_into_dest(destination, block, expr_id).into_block(); let popped = this.block_context.pop(); diff --git a/compiler/rustc_mir_build/src/builder/expr/stmt.rs b/compiler/rustc_mir_build/src/builder/expr/stmt.rs index 58090d3748b46..7f8a0a34c3123 100644 --- a/compiler/rustc_mir_build/src/builder/expr/stmt.rs +++ b/compiler/rustc_mir_build/src/builder/expr/stmt.rs @@ -164,8 +164,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } this.block_context.push(BlockFrame::TailExpr { - tail_result_is_ignored: true, - span: expr.span, + info: BlockTailInfo { tail_result_is_ignored: true, span: expr.span }, }); Some(expr.span) } else { diff --git a/compiler/rustc_mir_build/src/builder/mod.rs b/compiler/rustc_mir_build/src/builder/mod.rs index 2fe643e478e97..4348b7a4b4cc9 100644 --- a/compiler/rustc_mir_build/src/builder/mod.rs +++ b/compiler/rustc_mir_build/src/builder/mod.rs @@ -112,16 +112,7 @@ enum BlockFrame { /// Evaluation is currently within the tail expression of a block. /// /// Example: `{ STMT_1; STMT_2; EXPR }` - TailExpr { - /// If true, then the surrounding context of the block ignores - /// the result of evaluating the block's tail expression. - /// - /// Example: `let _ = { STMT_1; EXPR };` - tail_result_is_ignored: bool, - - /// `Span` of the tail expression. - span: Span, - }, + TailExpr { info: BlockTailInfo }, /// Generic mark meaning that the block occurred as a subexpression /// where the result might be used. @@ -277,9 +268,7 @@ impl BlockContext { match bf { BlockFrame::SubExpr => continue, BlockFrame::Statement { .. } => break, - &BlockFrame::TailExpr { tail_result_is_ignored, span } => { - return Some(BlockTailInfo { tail_result_is_ignored, span }); - } + &BlockFrame::TailExpr { info } => return Some(info), } } @@ -302,9 +291,9 @@ impl BlockContext { // otherwise: use accumulated is_ignored state. Some( - BlockFrame::TailExpr { tail_result_is_ignored: ignored, .. } - | BlockFrame::Statement { ignores_expr_result: ignored }, - ) => *ignored, + BlockFrame::TailExpr { info: BlockTailInfo { tail_result_is_ignored: ign, .. } } + | BlockFrame::Statement { ignores_expr_result: ign }, + ) => *ign, } } } From e03c809402d1f60b158a86810a51ab4e6e40f114 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 14 Feb 2025 08:56:58 +1100 Subject: [PATCH 20/23] Remove some unnecessary `FIXME` comments. The comments didn't make much sense to me. I asked Matthew Jasper on Zulip about it and they said: > I think that at the time I wanted to replace all (or most of) this > with a reference to the HIR Id of the variable. I'll give this a look > to see if it's still a reasonable idea, but removing the comments is > fine. and then: > I don't think that changing this to an HirId would be better, > recovering the information from the HIR seems like too much effort in > exchange for making the MIR a little smaller. --- compiler/rustc_middle/src/mir/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index afde812fbf510..cd18d94d0ed87 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -961,7 +961,6 @@ pub struct LocalDecl<'tcx> { /// Temporaries and the return place are always mutable. pub mutability: Mutability, - // FIXME(matthewjasper) Don't store in this in `Body` pub local_info: ClearCrossCrate>>, /// The type of this local. @@ -971,7 +970,6 @@ pub struct LocalDecl<'tcx> { /// e.g., via `let x: T`, then we carry that type here. The MIR /// borrow checker needs this information since it can affect /// region inference. - // FIXME(matthewjasper) Don't store in this in `Body` pub user_ty: Option>, /// The *syntactic* (i.e., not visibility) source scope the local is defined @@ -1079,7 +1077,6 @@ pub enum LocalInfo<'tcx> { AggregateTemp, /// A temporary created for evaluation of some subexpression of some block's tail expression /// (with no intervening statement context). - // FIXME(matthewjasper) Don't store in this in `Body` BlockTailTemp(BlockTailInfo), /// A temporary created during evaluating `if` predicate, possibly for pattern matching for `let`s, /// and subject to Edition 2024 temporary lifetime rules From 0519a58f7a461e0d2f7342de421ce1176f7bfd8d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 14 Feb 2025 16:29:23 +1100 Subject: [PATCH 21/23] Make `PassWhere` impl `Copy`. It's a very small and simple type. --- compiler/rustc_borrowck/src/polonius/dump.rs | 2 +- compiler/rustc_middle/src/mir/pretty.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_borrowck/src/polonius/dump.rs b/compiler/rustc_borrowck/src/polonius/dump.rs index 5f9fa3612b8d1..aa64a7c4e2a68 100644 --- a/compiler/rustc_borrowck/src/polonius/dump.rs +++ b/compiler/rustc_borrowck/src/polonius/dump.rs @@ -226,7 +226,7 @@ fn emit_polonius_mir<'tcx>( regioncx, closure_region_requirements, borrow_set, - pass_where.clone(), + pass_where, out, )?; diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 1e3b8d029e1b3..5d58ce9b4094b 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -22,7 +22,7 @@ pub(crate) const ALIGN: usize = 40; /// An indication of where we are in the control flow graph. Used for printing /// extra information in `dump_mir` -#[derive(Clone)] +#[derive(Clone, Copy)] pub enum PassWhere { /// We have not started dumping the control flow graph, but we are about to. BeforeCFG, From 2edaf684da10792f9e0fa97b8cccff3c270f7402 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Feb 2025 10:35:06 +1100 Subject: [PATCH 22/23] Clarify a comment. --- compiler/rustc_middle/src/mir/terminator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/terminator.rs b/compiler/rustc_middle/src/mir/terminator.rs index 9357e19f7c579..49e0f619b1ec6 100644 --- a/compiler/rustc_middle/src/mir/terminator.rs +++ b/compiler/rustc_middle/src/mir/terminator.rs @@ -86,7 +86,7 @@ impl SwitchTargets { self.iter().find_map(|(v, t)| (v == value).then_some(t)).unwrap_or_else(|| self.otherwise()) } - /// Adds a new target to the switch. But You cannot add an already present value. + /// Adds a new target to the switch. Panics if you add an already present value. #[inline] pub fn add_target(&mut self, value: u128, bb: BasicBlock) { let value = Pu128(value); From 69c7e1d02f1d77ac75e40fe9eef18df271dc3a13 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Fri, 17 Jan 2025 14:57:18 +0100 Subject: [PATCH 23/23] add more s390x target features The target feature names are, right now, based on the llvm target feature names. These mostly line up well with the names of [Facility Inidications](https://publibfp.dhe.ibm.com/epubs/pdf/a227832d.pdf#page=301) names. The linux kernel uses shorter, more cryptic names. (e.g. "vector" is `vx`). We can deviate from the llvm names, but the CPU vendor (IBM) does not appear to use e.g. `vx` for what they call `vector`. There are a number of implied target features between the vector facilities (based on the [Facility Inidications](https://publibfp.dhe.ibm.com/epubs/pdf/a227832d.pdf#page=301) table): - 129 The vector facility for z/Architecture is installed in the z/Architecture architectural mode. - 134 The vector packed decimal facility is installed in the z/Architecture architectural mode. When bit 134 is one, bit 129 is also one. - 135 The vector enhancements facility 1 is installed in the z/Architecture architectural mode. When bit 135 is one, bit 129 is also one. - 148 The vector-enhancements facility 2 is installed in the z/Architecture architectural mode. When bit 148 is one, bits 129 and 135 are also one. - 152 The vector-packed-decimal-enhancement facility 1 is installed in the z/Architecture architectural mode. When bit 152 is one, bits 129 and 134 are also one. - 165 The neural-network-processing-assist facility is installed in the z/Architecture architectural mode. When bit 165 is one, bit 129 is also one. - 192 The vector-packed-decimal-enhancement facility 2 is installed in the z/Architecture architectural mode. When bit 192 is one, bits 129, 134, and 152 are also one. And then there are a number of facilities without any implied target features - 45 The distinct-operands, fast-BCR-serialization, high-word, and population-count facilities, the interlocked-access facility 1, and the load/store-oncondition facility 1 are installed in the z/Architecture architectural mode. - 73 The transactional-execution facility is installed in the z/Architecture architectural mode. Bit 49 is one when bit 73 is one. - 133 The guarded-storage facility is installed in the z/Architecture architectural mode. - 150 The enhanced-sort facility is installed in the z/Architecture architectural mode. - 151 The DEFLATE-conversion facility is installed in the z/Architecture architectural mode. The added target features are those that have ISA implications, can be queried at runtime, and have LLVM support. LLVM [defines more target features](https://github.com/llvm/llvm-project/blob/d49a2d2bc9c65c787bfa04ac8ece614da48a8cd5/llvm/lib/Target/SystemZ/SystemZFeatures.td), but I'm not sure those are useful. They can always be added later, and can already be set globally using `-Ctarget-feature`. --- compiler/rustc_target/src/target_features.rs | 21 +++++++++++++++++++- tests/ui/check-cfg/target_feature.stderr | 11 ++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/target_features.rs b/compiler/rustc_target/src/target_features.rs index bb41d03e87f58..b98bca60c9de4 100644 --- a/compiler/rustc_target/src/target_features.rs +++ b/compiler/rustc_target/src/target_features.rs @@ -611,7 +611,26 @@ static LOONGARCH_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ const IBMZ_FEATURES: &[(&str, Stability, ImpliedFeatures)] = &[ // tidy-alphabetical-start ("backchain", Unstable(sym::s390x_target_feature), &[]), + ("deflate-conversion", Unstable(sym::s390x_target_feature), &[]), + ("enhanced-sort", Unstable(sym::s390x_target_feature), &[]), + ("guarded-storage", Unstable(sym::s390x_target_feature), &[]), + ("high-word", Unstable(sym::s390x_target_feature), &[]), + ("nnp-assist", Unstable(sym::s390x_target_feature), &["vector"]), + ("transactional-execution", Unstable(sym::s390x_target_feature), &[]), ("vector", Unstable(sym::s390x_target_feature), &[]), + ("vector-enhancements-1", Unstable(sym::s390x_target_feature), &["vector"]), + ("vector-enhancements-2", Unstable(sym::s390x_target_feature), &["vector-enhancements-1"]), + ("vector-packed-decimal", Unstable(sym::s390x_target_feature), &["vector"]), + ( + "vector-packed-decimal-enhancement", + Unstable(sym::s390x_target_feature), + &["vector-packed-decimal"], + ), + ( + "vector-packed-decimal-enhancement-2", + Unstable(sym::s390x_target_feature), + &["vector-packed-decimal-enhancement"], + ), // tidy-alphabetical-end ]; @@ -768,7 +787,7 @@ impl Target { /// the first list contains target features that must be enabled for ABI reasons, /// and the second list contains target feature that must be disabled for ABI reasons. /// - /// These features are automatically appended to whatever the target spec sats as default + /// These features are automatically appended to whatever the target spec sets as default /// features for the target. /// /// All features enabled/disabled via `-Ctarget-features` and `#[target_features]` are checked diff --git a/tests/ui/check-cfg/target_feature.stderr b/tests/ui/check-cfg/target_feature.stderr index 70852423bdbef..51808c5c7bcdb 100644 --- a/tests/ui/check-cfg/target_feature.stderr +++ b/tests/ui/check-cfg/target_feature.stderr @@ -58,6 +58,7 @@ LL | cfg!(target_feature = "_UNEXPECTED_VALUE"); `cssc` `d` `d32` +`deflate-conversion` `dit` `doloop` `dotprod` @@ -72,6 +73,7 @@ LL | cfg!(target_feature = "_UNEXPECTED_VALUE"); `ecv` `edsp` `elrw` +`enhanced-sort` `ermsb` `exception-handling` `extended-const` @@ -109,11 +111,13 @@ LL | cfg!(target_feature = "_UNEXPECTED_VALUE"); `frintts` `fxsr` `gfni` +`guarded-storage` `hard-float` `hard-float-abi` `hard-tp` `hbc` `high-registers` +`high-word` `hvx` `hvx-length128b` `hwdiv` @@ -151,6 +155,7 @@ LL | cfg!(target_feature = "_UNEXPECTED_VALUE"); `multivalue` `mutable-globals` `neon` +`nnp-assist` `nontrapping-fptoint` `nvic` `paca` @@ -229,6 +234,7 @@ LL | cfg!(target_feature = "_UNEXPECTED_VALUE"); `thumb-mode` `thumb2` `tme` +`transactional-execution` `trust` `trustzone` `ual` @@ -262,6 +268,11 @@ LL | cfg!(target_feature = "_UNEXPECTED_VALUE"); `vdspv1` `vdspv2` `vector` +`vector-enhancements-1` +`vector-enhancements-2` +`vector-packed-decimal` +`vector-packed-decimal-enhancement` +`vector-packed-decimal-enhancement-2` `vfp2` `vfp3` `vfp4`