From c5aa659832334dca7cc9b27074bdc686b9650c12 Mon Sep 17 00:00:00 2001 From: GnomedDev Date: Tue, 20 Feb 2024 18:59:56 +0000 Subject: [PATCH 01/18] Reduce alignment of TypeId to u64 alignment --- library/core/src/any.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/core/src/any.rs b/library/core/src/any.rs index e8f00e8760e13..7efbabb725748 100644 --- a/library/core/src/any.rs +++ b/library/core/src/any.rs @@ -605,7 +605,8 @@ impl dyn Any + Send + Sync { #[derive(Clone, Copy, Debug, Eq, PartialOrd, Ord)] #[stable(feature = "rust1", since = "1.0.0")] pub struct TypeId { - t: u128, + // See #115620 for this representation. + t: (u64, u64), } #[stable(feature = "rust1", since = "1.0.0")] @@ -637,7 +638,10 @@ impl TypeId { #[rustc_const_unstable(feature = "const_type_id", issue = "77125")] pub const fn of() -> TypeId { let t: u128 = intrinsics::type_id::(); - TypeId { t } + + let t1 = (t >> 64) as u64; + let t2 = t as u64; + TypeId { t: (t1, t2) } } } @@ -657,7 +661,7 @@ impl hash::Hash for TypeId { // - It is correct to do so -- only hashing a subset of `self` is still // with an `Eq` implementation that considers the entire value, as // ours does. - (self.t as u64).hash(state); + self.t.0.hash(state); } } From aa25c481b77cb9e0ec25c6350d7ded110fae7f0e Mon Sep 17 00:00:00 2001 From: Gnome! Date: Tue, 20 Feb 2024 19:12:42 +0000 Subject: [PATCH 02/18] Add extra detail to field comment Co-authored-by: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> --- library/core/src/any.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/core/src/any.rs b/library/core/src/any.rs index 7efbabb725748..adc45038148e8 100644 --- a/library/core/src/any.rs +++ b/library/core/src/any.rs @@ -605,7 +605,8 @@ impl dyn Any + Send + Sync { #[derive(Clone, Copy, Debug, Eq, PartialOrd, Ord)] #[stable(feature = "rust1", since = "1.0.0")] pub struct TypeId { - // See #115620 for this representation. + // We avoid using `u128` because that imposes higher alignment requirements on many platforms. + // See issue #115620 for more information. t: (u64, u64), } From f142476cf3f0c0c2296c82836fd4c7c86e20d0db Mon Sep 17 00:00:00 2001 From: David Thomas Date: Sun, 25 Feb 2024 14:09:30 +0000 Subject: [PATCH 03/18] Fix Hash impl --- library/core/src/any.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/any.rs b/library/core/src/any.rs index adc45038148e8..a4252d0c9e03c 100644 --- a/library/core/src/any.rs +++ b/library/core/src/any.rs @@ -662,7 +662,7 @@ impl hash::Hash for TypeId { // - It is correct to do so -- only hashing a subset of `self` is still // with an `Eq` implementation that considers the entire value, as // ours does. - self.t.0.hash(state); + self.t.1.hash(state); } } From 5c87ca2d1f23214d4890c9db88bd39684d6c8852 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 29 Feb 2024 20:51:24 +0100 Subject: [PATCH 04/18] Add some weird test cases to the non_local_definitions lint tests --- tests/ui/lint/non_local_definitions.rs | 20 +++++++++ tests/ui/lint/non_local_definitions.stderr | 50 ++++++++++++++++------ 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/tests/ui/lint/non_local_definitions.rs b/tests/ui/lint/non_local_definitions.rs index bbcc81898713c..d14312d237a2a 100644 --- a/tests/ui/lint/non_local_definitions.rs +++ b/tests/ui/lint/non_local_definitions.rs @@ -245,6 +245,26 @@ fn bad() { //~^ WARN non-local `impl` definition } +trait Uto9 {} +trait Uto10 {} +const _: u32 = { + let _a = || { + impl Uto9 for Test {} + //~^ WARN non-local `impl` definition + + 1 + }; + + type A = [u32; { + impl Uto10 for Test {} + //~^ WARN non-local `impl` definition + + 1 + }]; + + 1 +}; + struct UwU(T); fn fun() { diff --git a/tests/ui/lint/non_local_definitions.stderr b/tests/ui/lint/non_local_definitions.stderr index b9583ae983f34..ef74e262f9dbc 100644 --- a/tests/ui/lint/non_local_definitions.stderr +++ b/tests/ui/lint/non_local_definitions.stderr @@ -442,7 +442,29 @@ LL | impl Uto8 for T {} = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:253:5 + --> $DIR/non_local_definitions.rs:252:9 + | +LL | impl Uto9 for Test {} + | ^^^^^^^^^^^^^^^^^^^^^ + | + = help: move this `impl` block outside the of the current closure `` and up 2 bodies + = note: an `impl` definition is non-local if it is nested inside an item and neither the type nor the trait are at the same nesting level as the `impl` block + = note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type + = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue + +warning: non-local `impl` definition, they should be avoided as they go against expectation + --> $DIR/non_local_definitions.rs:259:9 + | +LL | impl Uto10 for Test {} + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = help: move this `impl` block outside the of the current constant expression `` and up 2 bodies + = note: an `impl` definition is non-local if it is nested inside an item and neither the type nor the trait are at the same nesting level as the `impl` block + = note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type + = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue + +warning: non-local `impl` definition, they should be avoided as they go against expectation + --> $DIR/non_local_definitions.rs:273:5 | LL | / impl Default for UwU { LL | | @@ -458,7 +480,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:264:5 + --> $DIR/non_local_definitions.rs:284:5 | LL | / impl From for () { LL | | @@ -474,7 +496,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:273:5 + --> $DIR/non_local_definitions.rs:293:5 | LL | / impl AsRef for () { LL | | @@ -488,7 +510,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:284:5 + --> $DIR/non_local_definitions.rs:304:5 | LL | / impl PartialEq for G { LL | | @@ -504,7 +526,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:301:5 + --> $DIR/non_local_definitions.rs:321:5 | LL | / impl PartialEq for &Dog { LL | | @@ -520,7 +542,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:308:5 + --> $DIR/non_local_definitions.rs:328:5 | LL | / impl PartialEq<()> for Dog { LL | | @@ -536,7 +558,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:315:5 + --> $DIR/non_local_definitions.rs:335:5 | LL | / impl PartialEq<()> for &Dog { LL | | @@ -552,7 +574,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:322:5 + --> $DIR/non_local_definitions.rs:342:5 | LL | / impl PartialEq for () { LL | | @@ -568,7 +590,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:344:5 + --> $DIR/non_local_definitions.rs:364:5 | LL | / impl From>> for () { LL | | @@ -584,7 +606,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:351:5 + --> $DIR/non_local_definitions.rs:371:5 | LL | / impl From<()> for Wrap { LL | | @@ -600,7 +622,7 @@ LL | | } = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:364:13 + --> $DIR/non_local_definitions.rs:384:13 | LL | impl MacroTrait for OutsideStruct {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -615,7 +637,7 @@ LL | m!(); = note: this warning originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info) warning: non-local `impl` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:374:1 + --> $DIR/non_local_definitions.rs:394:1 | LL | non_local_macro::non_local_impl!(CargoUpdate); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -628,7 +650,7 @@ LL | non_local_macro::non_local_impl!(CargoUpdate); = note: this warning originates in the macro `non_local_macro::non_local_impl` (in Nightly builds, run with -Z macro-backtrace for more info) warning: non-local `macro_rules!` definition, they should be avoided as they go against expectation - --> $DIR/non_local_definitions.rs:377:1 + --> $DIR/non_local_definitions.rs:397:1 | LL | non_local_macro::non_local_macro_rules!(my_macro); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -640,5 +662,5 @@ LL | non_local_macro::non_local_macro_rules!(my_macro); = note: the macro `non_local_macro::non_local_macro_rules` may come from an old version of the `non_local_macro` crate, try updating your dependency with `cargo update -p non_local_macro` = note: this warning originates in the macro `non_local_macro::non_local_macro_rules` (in Nightly builds, run with -Z macro-backtrace for more info) -warning: 50 warnings emitted +warning: 52 warnings emitted From 20200f65ca63c4b3cd75aa1ebb9829188c78b388 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 29 Feb 2024 20:59:09 +0100 Subject: [PATCH 05/18] Remove useless smallvec dependency in rustc_lint::non_local_def --- Cargo.lock | 1 - compiler/rustc_lint/Cargo.toml | 1 - compiler/rustc_lint/src/non_local_def.rs | 30 ++++++++++++++---------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 635146492b042..81c606849e7e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4133,7 +4133,6 @@ dependencies = [ "rustc_target", "rustc_trait_selection", "rustc_type_ir", - "smallvec", "tracing", "unicode-security", ] diff --git a/compiler/rustc_lint/Cargo.toml b/compiler/rustc_lint/Cargo.toml index 2271321b8bf22..fa1133e7780ff 100644 --- a/compiler/rustc_lint/Cargo.toml +++ b/compiler/rustc_lint/Cargo.toml @@ -23,7 +23,6 @@ rustc_span = { path = "../rustc_span" } rustc_target = { path = "../rustc_target" } rustc_trait_selection = { path = "../rustc_trait_selection" } rustc_type_ir = { path = "../rustc_type_ir" } -smallvec = { version = "1.8.1", features = ["union", "may_dangle"] } tracing = "0.1" unicode-security = "0.1.0" # tidy-alphabetical-end diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index a4fd5a7c45f97..597010b8925e0 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -2,8 +2,6 @@ use rustc_hir::{def::DefKind, Body, Item, ItemKind, Node, Path, QPath, TyKind}; use rustc_span::def_id::{DefId, LOCAL_CRATE}; use rustc_span::{sym, symbol::kw, ExpnKind, MacroKind}; -use smallvec::{smallvec, SmallVec}; - use crate::lints::{NonLocalDefinitionsCargoUpdateNote, NonLocalDefinitionsDiag}; use crate::{LateContext, LateLintPass, LintContext}; @@ -114,25 +112,25 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { // is using local items and so we don't lint on it. // We also ignore anon-const in item by including the anon-const - // parent as well; and since it's quite uncommon, we use smallvec - // to avoid unnecessary heap allocations. - let local_parents: SmallVec<[DefId; 1]> = if parent_def_kind == DefKind::Const + // parent as well. + let parent_parent = if parent_def_kind == DefKind::Const && parent_opt_item_name == Some(kw::Underscore) { - smallvec![parent, cx.tcx.parent(parent)] + Some(cx.tcx.parent(parent)) } else { - smallvec![parent] + None }; let self_ty_has_local_parent = match impl_.self_ty.kind { TyKind::Path(QPath::Resolved(_, ty_path)) => { - path_has_local_parent(ty_path, cx, &*local_parents) + path_has_local_parent(ty_path, cx, parent, parent_parent) } TyKind::TraitObject([principle_poly_trait_ref, ..], _, _) => { path_has_local_parent( principle_poly_trait_ref.trait_ref.path, cx, - &*local_parents, + parent, + parent_parent, ) } TyKind::TraitObject([], _, _) @@ -154,7 +152,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { let of_trait_has_local_parent = impl_ .of_trait - .map(|of_trait| path_has_local_parent(of_trait.path, cx, &*local_parents)) + .map(|of_trait| path_has_local_parent(of_trait.path, cx, parent, parent_parent)) .unwrap_or(false); // If none of them have a local parent (LOGICAL NOR) this means that @@ -218,6 +216,14 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { /// std::convert::PartialEq> /// ^^^^^^^^^^^^^^^^^^^^^^^ /// ``` -fn path_has_local_parent(path: &Path<'_>, cx: &LateContext<'_>, local_parents: &[DefId]) -> bool { - path.res.opt_def_id().is_some_and(|did| local_parents.contains(&cx.tcx.parent(did))) +fn path_has_local_parent( + path: &Path<'_>, + cx: &LateContext<'_>, + impl_parent: DefId, + impl_parent_parent: Option, +) -> bool { + path.res.opt_def_id().is_some_and(|did| { + let res_parent = cx.tcx.parent(did); + res_parent == impl_parent || Some(res_parent) == impl_parent_parent + }) } From 6c4eadd74746eaa21c6da1756c440b1fdabf0729 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 29 Feb 2024 21:00:35 +0100 Subject: [PATCH 06/18] Add early-return when checking if a path is local --- compiler/rustc_lint/src/non_local_def.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index 597010b8925e0..1ae1c72b6e8f4 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -223,7 +223,9 @@ fn path_has_local_parent( impl_parent_parent: Option, ) -> bool { path.res.opt_def_id().is_some_and(|did| { - let res_parent = cx.tcx.parent(did); - res_parent == impl_parent || Some(res_parent) == impl_parent_parent + did.is_local() && { + let res_parent = cx.tcx.parent(did); + res_parent == impl_parent || Some(res_parent) == impl_parent_parent + } }) } From 98dbe9abac8e02f9a60393ae2eb2ca448c69e7d4 Mon Sep 17 00:00:00 2001 From: Urgau Date: Thu, 29 Feb 2024 21:02:47 +0100 Subject: [PATCH 07/18] Use was_invoked_from_cargo method instead of hand-written one --- compiler/rustc_lint/src/non_local_def.rs | 2 +- tests/ui/lint/non_local_definitions.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/non_local_def.rs b/compiler/rustc_lint/src/non_local_def.rs index 1ae1c72b6e8f4..7c4d92d3ce038 100644 --- a/compiler/rustc_lint/src/non_local_def.rs +++ b/compiler/rustc_lint/src/non_local_def.rs @@ -83,7 +83,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions { if let Some(def_id) = oexpn.macro_def_id && let ExpnKind::Macro(macro_kind, macro_name) = oexpn.kind && def_id.krate != LOCAL_CRATE - && std::env::var_os("CARGO").is_some() + && rustc_session::utils::was_invoked_from_cargo() { Some(NonLocalDefinitionsCargoUpdateNote { macro_kind: macro_kind.descr(), diff --git a/tests/ui/lint/non_local_definitions.rs b/tests/ui/lint/non_local_definitions.rs index d14312d237a2a..eee582a6f11ba 100644 --- a/tests/ui/lint/non_local_definitions.rs +++ b/tests/ui/lint/non_local_definitions.rs @@ -1,7 +1,7 @@ //@ check-pass //@ edition:2021 //@ aux-build:non_local_macro.rs -//@ rustc-env:CARGO=/usr/bin/cargo +//@ rustc-env:CARGO_CRATE_NAME=non_local_def #![feature(inline_const)] #![warn(non_local_definitions)] From 4663fbb2cb6922c28c4bd4e4947d8213f00382b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Thu, 7 Mar 2024 23:03:42 +0000 Subject: [PATCH 08/18] Eagerly translate HelpUseLatestEdition in parser diagnostics --- .../rustc_parse/src/parser/diagnostics.rs | 2 +- .../ui/parser/help-set-edition-ice-122130.rs | 5 +++++ .../parser/help-set-edition-ice-122130.stderr | 21 +++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 tests/ui/parser/help-set-edition-ice-122130.rs create mode 100644 tests/ui/parser/help-set-edition-ice-122130.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 2f7ac7d3a12e5..de088b9364b26 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -667,7 +667,7 @@ impl<'a> Parser<'a> { { err.note("you may be trying to write a c-string literal"); err.note("c-string literals require Rust 2021 or later"); - HelpUseLatestEdition::new().add_to_diagnostic(&mut err); + err.subdiagnostic(self.dcx(), HelpUseLatestEdition::new()); } // `pub` may be used for an item or `pub(crate)` diff --git a/tests/ui/parser/help-set-edition-ice-122130.rs b/tests/ui/parser/help-set-edition-ice-122130.rs new file mode 100644 index 0000000000000..bc5af04ecbc0c --- /dev/null +++ b/tests/ui/parser/help-set-edition-ice-122130.rs @@ -0,0 +1,5 @@ +enum will { + s#[c"owned_box"] + //~^ERROR expected one of `(`, `,`, `=`, `{`, or `}`, found `#` + //~|ERROR expected item, found `"owned_box"` +} diff --git a/tests/ui/parser/help-set-edition-ice-122130.stderr b/tests/ui/parser/help-set-edition-ice-122130.stderr new file mode 100644 index 0000000000000..fe4d212f2db65 --- /dev/null +++ b/tests/ui/parser/help-set-edition-ice-122130.stderr @@ -0,0 +1,21 @@ +error: expected one of `(`, `,`, `=`, `{`, or `}`, found `#` + --> $DIR/help-set-edition-ice-122130.rs:2:6 + | +LL | s#[c"owned_box"] + | ^ expected one of `(`, `,`, `=`, `{`, or `}` + | + = note: you may be trying to write a c-string literal + = note: c-string literals require Rust 2021 or later + = help: pass `--edition 2021` to `rustc` + = note: for more on editions, read https://doc.rust-lang.org/edition-guide + +error: expected item, found `"owned_box"` + --> $DIR/help-set-edition-ice-122130.rs:2:9 + | +LL | s#[c"owned_box"] + | ^^^^^^^^^^^ expected item + | + = note: for a full list of items that can appear in modules, see + +error: aborting due to 2 previous errors + From bef1cd80d8f6264b5ed66b5696a758db5d47491b Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 7 Mar 2024 18:30:04 -0800 Subject: [PATCH 09/18] ci: add a runner for vanilla LLVM 18 --- .github/workflows/ci.yml | 4 ++ .../host-x86_64/x86_64-gnu-llvm-17/Dockerfile | 4 ++ .../host-x86_64/x86_64-gnu-llvm-18/Dockerfile | 55 +++++++++++++++++++ src/ci/github-actions/ci.yml | 5 ++ 4 files changed, 68 insertions(+) create mode 100644 src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d1056de25c10..cd13ce35007b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -315,6 +315,10 @@ jobs: - name: x86_64-gnu-distcheck os: ubuntu-20.04-8core-32gb env: {} + - name: x86_64-gnu-llvm-18 + env: + RUST_BACKTRACE: 1 + os: ubuntu-20.04-8core-32gb - name: x86_64-gnu-llvm-17 env: RUST_BACKTRACE: 1 diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile index fe30a95344104..1eedfe3f09b4d 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-17/Dockerfile @@ -42,6 +42,10 @@ RUN sh /scripts/sccache.sh ENV NO_DOWNLOAD_CI_LLVM 1 ENV EXTERNAL_LLVM 1 +# This is not the latest LLVM version, so some components required by tests may +# be missing. +ENV IS_NOT_LATEST_LLVM 1 + # Using llvm-link-shared due to libffi issues -- see #34486 ENV RUST_CONFIGURE_ARGS \ --build=x86_64-unknown-linux-gnu \ diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile new file mode 100644 index 0000000000000..e8383500dfc94 --- /dev/null +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-18/Dockerfile @@ -0,0 +1,55 @@ +FROM ubuntu:24.04 + +ARG DEBIAN_FRONTEND=noninteractive + +RUN apt-get update && apt-get install -y --no-install-recommends \ + g++ \ + gcc-multilib \ + make \ + ninja-build \ + file \ + curl \ + ca-certificates \ + python3 \ + git \ + cmake \ + sudo \ + gdb \ + llvm-18-tools \ + llvm-18-dev \ + libedit-dev \ + libssl-dev \ + pkg-config \ + zlib1g-dev \ + xz-utils \ + nodejs \ + mingw-w64 \ + libgccjit-13-dev \ + && rm -rf /var/lib/apt/lists/* + +# Note: libgccjit needs to match the default gcc version for the linker to find it. + +# Install powershell (universal package) so we can test x.ps1 on Linux +# FIXME: need a "universal" version that supports libicu74, but for now it still works to ignore that dep. +RUN curl -sL "https://github.com/PowerShell/PowerShell/releases/download/v7.3.1/powershell_7.3.1-1.deb_amd64.deb" > powershell.deb && \ + dpkg --ignore-depends=libicu72 -i powershell.deb && \ + rm -f powershell.deb + +COPY scripts/sccache.sh /scripts/ +RUN sh /scripts/sccache.sh + +# We are disabling CI LLVM since this builder is intentionally using a host +# LLVM, rather than the typical src/llvm-project LLVM. +ENV NO_DOWNLOAD_CI_LLVM 1 +ENV EXTERNAL_LLVM 1 + +# Using llvm-link-shared due to libffi issues -- see #34486 +ENV RUST_CONFIGURE_ARGS \ + --build=x86_64-unknown-linux-gnu \ + --llvm-root=/usr/lib/llvm-18 \ + --enable-llvm-link-shared \ + --set rust.thin-lto-import-instr-limit=10 + +COPY host-x86_64/x86_64-gnu-llvm-16/script.sh /tmp/ + +ENV SCRIPT /tmp/script.sh diff --git a/src/ci/github-actions/ci.yml b/src/ci/github-actions/ci.yml index 2ba5d357a1d00..c392ccec3dc2b 100644 --- a/src/ci/github-actions/ci.yml +++ b/src/ci/github-actions/ci.yml @@ -510,6 +510,11 @@ jobs: - name: x86_64-gnu-distcheck <<: *job-linux-8c + - name: x86_64-gnu-llvm-18 + env: + RUST_BACKTRACE: 1 + <<: *job-linux-8c + - name: x86_64-gnu-llvm-17 env: RUST_BACKTRACE: 1 From fdff4d7682071b981a40dcc3575300a661a1f382 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 8 Mar 2024 14:49:28 +0000 Subject: [PATCH 10/18] Move metadata header and version checks together This will make it easier to report rustc versions for older metadata formats. --- compiler/rustc_driver_impl/src/lib.rs | 1 + compiler/rustc_metadata/src/locator.rs | 99 ++++++++++++-------- compiler/rustc_metadata/src/rmeta/decoder.rs | 24 +++-- 3 files changed, 79 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 89ab5c6bd3b76..4edfd111e5cf1 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -676,6 +676,7 @@ fn list_metadata(early_dcx: &EarlyDiagCtxt, sess: &Session, metadata_loader: &dy metadata_loader, &mut v, &sess.opts.unstable_opts.ls, + sess.cfg_version, ) .unwrap(); safe_println!("{}", String::from_utf8(v).unwrap()); diff --git a/compiler/rustc_metadata/src/locator.rs b/compiler/rustc_metadata/src/locator.rs index dcccace12b00a..10bbebb3871e3 100644 --- a/compiler/rustc_metadata/src/locator.rs +++ b/compiler/rustc_metadata/src/locator.rs @@ -569,31 +569,47 @@ impl<'a> CrateLocator<'a> { debug!("skipping empty file"); continue; } - let (hash, metadata) = - match get_metadata_section(self.target, flavor, &lib, self.metadata_loader) { - Ok(blob) => { - if let Some(h) = self.crate_matches(&blob, &lib) { - (h, blob) - } else { - info!("metadata mismatch"); - continue; - } - } - Err(MetadataError::LoadFailure(err)) => { - info!("no metadata found: {}", err); - // The file was present and created by the same compiler version, but we - // couldn't load it for some reason. Give a hard error instead of silently - // ignoring it, but only if we would have given an error anyway. - self.crate_rejections - .via_invalid - .push(CrateMismatch { path: lib, got: err }); - continue; - } - Err(err @ MetadataError::NotPresent(_)) => { - info!("no metadata found: {}", err); + let (hash, metadata) = match get_metadata_section( + self.target, + flavor, + &lib, + self.metadata_loader, + self.cfg_version, + ) { + Ok(blob) => { + if let Some(h) = self.crate_matches(&blob, &lib) { + (h, blob) + } else { + info!("metadata mismatch"); continue; } - }; + } + Err(MetadataError::VersionMismatch { expected_version, found_version }) => { + // The file was present and created by the same compiler version, but we + // couldn't load it for some reason. Give a hard error instead of silently + // ignoring it, but only if we would have given an error anyway. + info!( + "Rejecting via version: expected {} got {}", + expected_version, found_version + ); + self.crate_rejections + .via_version + .push(CrateMismatch { path: lib, got: found_version }); + continue; + } + Err(MetadataError::LoadFailure(err)) => { + info!("no metadata found: {}", err); + // The file was present and created by the same compiler version, but we + // couldn't load it for some reason. Give a hard error instead of silently + // ignoring it, but only if we would have given an error anyway. + self.crate_rejections.via_invalid.push(CrateMismatch { path: lib, got: err }); + continue; + } + Err(err @ MetadataError::NotPresent(_)) => { + info!("no metadata found: {}", err); + continue; + } + }; // If we see multiple hashes, emit an error about duplicate candidates. if slot.as_ref().is_some_and(|s| s.0 != hash) { if let Some(candidates) = err_data { @@ -648,16 +664,6 @@ impl<'a> CrateLocator<'a> { } fn crate_matches(&mut self, metadata: &MetadataBlob, libpath: &Path) -> Option { - let rustc_version = rustc_version(self.cfg_version); - let found_version = metadata.get_rustc_version(); - if found_version != rustc_version { - info!("Rejecting via version: expected {} got {}", rustc_version, found_version); - self.crate_rejections - .via_version - .push(CrateMismatch { path: libpath.to_path_buf(), got: found_version }); - return None; - } - let header = metadata.get_header(); if header.is_proc_macro_crate != self.is_proc_macro { info!( @@ -770,6 +776,7 @@ fn get_metadata_section<'p>( flavor: CrateFlavor, filename: &'p Path, loader: &dyn MetadataLoader, + cfg_version: &'static str, ) -> Result> { if !filename.exists() { return Err(MetadataError::NotPresent(filename)); @@ -847,13 +854,18 @@ fn get_metadata_section<'p>( } }; let blob = MetadataBlob(raw_bytes); - if blob.is_compatible() { - Ok(blob) - } else { - Err(MetadataError::LoadFailure(format!( + match blob.check_compatibility(cfg_version) { + Ok(()) => Ok(blob), + Err(None) => Err(MetadataError::LoadFailure(format!( "invalid metadata version found: {}", filename.display() - ))) + ))), + Err(Some(found_version)) => { + return Err(MetadataError::VersionMismatch { + expected_version: rustc_version(cfg_version), + found_version, + }); + } } } @@ -864,9 +876,10 @@ pub fn list_file_metadata( metadata_loader: &dyn MetadataLoader, out: &mut dyn Write, ls_kinds: &[String], + cfg_version: &'static str, ) -> IoResult<()> { let flavor = get_flavor_from_path(path); - match get_metadata_section(target, flavor, path, metadata_loader) { + match get_metadata_section(target, flavor, path, metadata_loader, cfg_version) { Ok(metadata) => metadata.list_crate_metadata(out, ls_kinds), Err(msg) => write!(out, "{msg}\n"), } @@ -932,6 +945,8 @@ enum MetadataError<'a> { NotPresent(&'a Path), /// The file was present and invalid. LoadFailure(String), + /// The file was present, but compiled with a different rustc version. + VersionMismatch { expected_version: String, found_version: String }, } impl fmt::Display for MetadataError<'_> { @@ -941,6 +956,12 @@ impl fmt::Display for MetadataError<'_> { f.write_str(&format!("no such file: '{}'", filename.display())) } MetadataError::LoadFailure(msg) => f.write_str(msg), + MetadataError::VersionMismatch { expected_version, found_version } => { + f.write_str(&format!( + "rustc version mismatch. expected {}, found {}", + expected_version, found_version, + )) + } } } } diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index da384007c22c2..0467cf2969fc5 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -684,13 +684,25 @@ impl<'a, 'tcx, I: Idx, T> Decodable> for LazyTable implement_ty_decoder!(DecodeContext<'a, 'tcx>); impl MetadataBlob { - pub(crate) fn is_compatible(&self) -> bool { - self.blob().starts_with(METADATA_HEADER) - } + pub(crate) fn check_compatibility( + &self, + cfg_version: &'static str, + ) -> Result<(), Option> { + if !self.blob().starts_with(METADATA_HEADER) { + if self.blob().starts_with(b"rust") { + return Err(Some("".to_owned())); + } + return Err(None); + } - pub(crate) fn get_rustc_version(&self) -> String { - LazyValue::::from_position(NonZero::new(METADATA_HEADER.len() + 8).unwrap()) - .decode(self) + let found_version = + LazyValue::::from_position(NonZero::new(METADATA_HEADER.len() + 8).unwrap()) + .decode(self); + if rustc_version(cfg_version) != found_version { + return Err(Some(found_version)); + } + + Ok(()) } fn root_pos(&self) -> NonZero { From 87ab9e8c6eac37374475fbc1409b4153dd52e1a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 9 Mar 2024 00:24:14 +0100 Subject: [PATCH 11/18] Some tweaks to the parallel query cycle handler --- compiler/rustc_interface/src/util.rs | 19 ++++++++++++++--- compiler/rustc_query_system/src/query/job.rs | 22 +++++++------------- compiler/rustc_query_system/src/query/mod.rs | 2 +- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 0d50200133cb2..23bd2dac57e3b 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -101,10 +101,11 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( threads: usize, f: F, ) -> R { - use rustc_data_structures::{jobserver, sync::FromDyn}; + use rustc_data_structures::{defer, jobserver, sync::FromDyn}; use rustc_middle::ty::tls; use rustc_query_impl::QueryCtxt; - use rustc_query_system::query::{deadlock, QueryContext}; + use rustc_query_system::query::{break_query_cycles, QueryContext}; + use std::process; let registry = sync::Registry::new(std::num::NonZero::new(threads).unwrap()); @@ -128,7 +129,19 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( let query_map = FromDyn::from(tls::with(|tcx| QueryCtxt::new(tcx).collect_active_jobs())); let registry = rayon_core::Registry::current(); - thread::spawn(move || deadlock(query_map.into_inner(), ®istry)); + thread::Builder::new() + .name("rustc query cycle handler".to_string()) + .spawn(move || { + let on_panic = defer(|| { + eprintln!("query cycle handler thread panicked, aborting process"); + // We need to abort here as we failed to resolve the deadlock, + // otherwise the compiler could just hang, + process::abort(); + }); + break_query_cycles(query_map.into_inner(), ®istry); + on_panic.disable(); + }) + .unwrap(); }); if let Some(size) = get_stack_size() { builder = builder.stack_size(size); diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index 1a54a2293573b..248a741af9079 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -17,10 +17,9 @@ use std::num::NonZero; use { parking_lot::{Condvar, Mutex}, rustc_data_structures::fx::FxHashSet, - rustc_data_structures::{defer, jobserver}, + rustc_data_structures::jobserver, rustc_span::DUMMY_SP, std::iter, - std::process, std::sync::Arc, }; @@ -514,12 +513,7 @@ fn remove_cycle( /// There may be multiple cycles involved in a deadlock, so this searches /// all active queries for cycles before finally resuming all the waiters at once. #[cfg(parallel_compiler)] -pub fn deadlock(query_map: QueryMap, registry: &rayon_core::Registry) { - let on_panic = defer(|| { - eprintln!("deadlock handler panicked, aborting process"); - process::abort(); - }); - +pub fn break_query_cycles(query_map: QueryMap, registry: &rayon_core::Registry) { let mut wakelist = Vec::new(); let mut jobs: Vec = query_map.keys().cloned().collect(); @@ -539,19 +533,17 @@ pub fn deadlock(query_map: QueryMap, registry: &rayon_core::Registry) { // X to Y due to Rayon waiting and a true dependency from Y to X. The algorithm here // only considers the true dependency and won't detect a cycle. if !found_cycle { - if query_map.len() == 0 { - panic!("deadlock detected without any query!") - } else { - panic!("deadlock detected! current query map:\n{:#?}", query_map); - } + panic!( + "deadlock detected as we're unable to find a query cycle to break\n\ + current query map:\n{:#?}", + query_map + ); } // FIXME: Ensure this won't cause a deadlock before we return for waiter in wakelist.into_iter() { waiter.notify(registry); } - - on_panic.disable(); } #[inline(never)] diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index 0aefe553a3435..01b9d458f1e4f 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -3,7 +3,7 @@ pub use self::plumbing::*; mod job; #[cfg(parallel_compiler)] -pub use self::job::deadlock; +pub use self::job::break_query_cycles; pub use self::job::{ print_query_stack, report_cycle, QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryMap, }; From 564837e23d9293edddb3d7a997667a6c6796fb2b Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Fri, 8 Mar 2024 23:20:29 -0500 Subject: [PATCH 12/18] Fix typo in `VisitorResult` --- compiler/rustc_ast_ir/src/visit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_ast_ir/src/visit.rs b/compiler/rustc_ast_ir/src/visit.rs index dec9f7a47d09d..f6d6bf3a3e309 100644 --- a/compiler/rustc_ast_ir/src/visit.rs +++ b/compiler/rustc_ast_ir/src/visit.rs @@ -14,7 +14,7 @@ impl VisitorResult for () { type Residual = !; #[cfg(not(feature = "nightly"))] - type Residual = core::ops::Infallible; + type Residual = core::convert::Infallible; fn output() -> Self {} fn from_residual(_: Self::Residual) -> Self {} From 7c13421dc0498e05624946327b1a022f2fa7b294 Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Sat, 9 Mar 2024 00:06:48 +0300 Subject: [PATCH 13/18] fix incorrect path resolution in tidy Previously, reading the current path from the environment led to failure when invoking x from outside the source root. This change fixes this issue by passing the already resolved root path into `ui_tests::check`. Signed-off-by: onur-ozkan --- src/tools/tidy/src/main.rs | 2 +- src/tools/tidy/src/ui_tests.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 4b98c91319da0..2bce246c30827 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -102,7 +102,7 @@ fn main() { check!(tests_placement, &root_path); check!(tests_revision_unpaired_stdout_stderr, &tests_path); check!(debug_artifacts, &tests_path); - check!(ui_tests, &tests_path, bless); + check!(ui_tests, &root_path, bless); check!(mir_opt_tests, &tests_path, bless); check!(rustdoc_gui_tests, &tests_path); check!(rustdoc_css_themes, &librustdoc_path); diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 39b14adfea413..c946554b98ff6 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -99,7 +99,8 @@ fn check_entries(tests_path: &Path, bad: &mut bool) { } } -pub fn check(path: &Path, bless: bool, bad: &mut bool) { +pub fn check(root_path: &Path, bless: bool, bad: &mut bool) { + let path = &root_path.join("tests"); check_entries(&path, bad); // the list of files in ui tests that are allowed to start with `issue-XXXX` @@ -193,7 +194,7 @@ pub fn check(path: &Path, bless: bool, bad: &mut bool) { */ [ "#; - let tidy_src = std::env::current_dir().unwrap().join("src/tools/tidy/src"); + let tidy_src = root_path.join("src/tools/tidy/src"); // instead of overwriting the file, recreate it and use an "atomic rename" // so we don't bork things on panic or a contributor using Ctrl+C let blessed_issues_path = tidy_src.join("issues_blessed.txt"); From b1f4657fe99b63d2ec9fa647701a7975339ac457 Mon Sep 17 00:00:00 2001 From: Gurinder Singh Date: Sat, 9 Mar 2024 12:01:19 +0530 Subject: [PATCH 14/18] Add missing regression tests for a couple of issues --- .../ice-unsized-struct-arg-issue-121612.rs | 13 +++++ ...ice-unsized-struct-arg-issue-121612.stderr | 57 +++++++++++++++++++ .../ice-unsized-struct-arg-issue2-121424.rs | 9 +++ ...ce-unsized-struct-arg-issue2-121424.stderr | 30 ++++++++++ 4 files changed, 109 insertions(+) create mode 100644 tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.rs create mode 100644 tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.stderr create mode 100644 tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.rs create mode 100644 tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.stderr diff --git a/tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.rs b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.rs new file mode 100644 index 0000000000000..5ca4f49c3bac5 --- /dev/null +++ b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.rs @@ -0,0 +1,13 @@ +// Regression test for issue #121612 + +trait Trait {} +impl Trait for bool {} +struct MySlice Idx>(bool, T); +//~^ ERROR cannot find type `Idx` in this scope +//~| ERROR cannot find type `Idx` in this scope +type MySliceBool = MySlice<[bool]>; +const MYSLICE_GOOD: &MySliceBool = &MySlice(true, [false]); +//~^ ERROR the size for values of type `[bool]` cannot be known at compilation time +//~| ERROR the size for values of type `[bool]` cannot be known at compilation time + +fn main() {} diff --git a/tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.stderr b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.stderr new file mode 100644 index 0000000000000..0be80e9479f0b --- /dev/null +++ b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue-121612.stderr @@ -0,0 +1,57 @@ +error[E0412]: cannot find type `Idx` in this scope + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:5:30 + | +LL | struct MySlice Idx>(bool, T); + | ^^^ not found in this scope + +error[E0412]: cannot find type `Idx` in this scope + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:5:38 + | +LL | struct MySlice Idx>(bool, T); + | ^^^ not found in this scope + +error[E0277]: the size for values of type `[bool]` cannot be known at compilation time + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:9:22 + | +LL | const MYSLICE_GOOD: &MySliceBool = &MySlice(true, [false]); + | ^^^^^^^^^^^ doesn't have a size known at compile-time + | + = help: the trait `Sized` is not implemented for `[bool]` +note: required by an implicit `Sized` bound in `MySlice` + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:5:16 + | +LL | struct MySlice Idx>(bool, T); + | ^ required by the implicit `Sized` requirement on this type parameter in `MySlice` +help: you could relax the implicit `Sized` bound on `T` if it were used through indirection like `&T` or `Box` + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:5:16 + | +LL | struct MySlice Idx>(bool, T); + | ^ - ...if indirection were used here: `Box` + | | + | this could be changed to `T: ?Sized`... + +error[E0277]: the size for values of type `[bool]` cannot be known at compilation time + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:9:22 + | +LL | const MYSLICE_GOOD: &MySliceBool = &MySlice(true, [false]); + | ^^^^^^^^^^^ doesn't have a size known at compile-time + | + = help: the trait `Sized` is not implemented for `[bool]` +note: required by an implicit `Sized` bound in `MySlice` + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:5:16 + | +LL | struct MySlice Idx>(bool, T); + | ^ required by the implicit `Sized` requirement on this type parameter in `MySlice` +help: you could relax the implicit `Sized` bound on `T` if it were used through indirection like `&T` or `Box` + --> $DIR/ice-unsized-struct-arg-issue-121612.rs:5:16 + | +LL | struct MySlice Idx>(bool, T); + | ^ - ...if indirection were used here: `Box` + | | + | this could be changed to `T: ?Sized`... + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0277, E0412. +For more information about an error, try `rustc --explain E0277`. diff --git a/tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.rs b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.rs new file mode 100644 index 0000000000000..1a9266e05def3 --- /dev/null +++ b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.rs @@ -0,0 +1,9 @@ +// Regression test for issue #121424 +#[repr(C)] +struct MySlice(bool, T); +type MySliceBool = MySlice<[bool]>; +const MYSLICE_GOOD: &MySliceBool = &MySlice(true, [false]); +//~^ ERROR the trait bound `[bool]: Copy` is not satisfied +//~| ERROR the trait bound `[bool]: Copy` is not satisfied + +fn main() {} diff --git a/tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.stderr b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.stderr new file mode 100644 index 0000000000000..3738bbfb8de96 --- /dev/null +++ b/tests/ui/trait-bounds/ice-unsized-struct-arg-issue2-121424.stderr @@ -0,0 +1,30 @@ +error[E0277]: the trait bound `[bool]: Copy` is not satisfied + --> $DIR/ice-unsized-struct-arg-issue2-121424.rs:5:22 + | +LL | const MYSLICE_GOOD: &MySliceBool = &MySlice(true, [false]); + | ^^^^^^^^^^^ the trait `Copy` is not implemented for `[bool]` + | + = help: the trait `Copy` is implemented for `[T; N]` +note: required by a bound in `MySlice` + --> $DIR/ice-unsized-struct-arg-issue2-121424.rs:3:19 + | +LL | struct MySlice(bool, T); + | ^^^^ required by this bound in `MySlice` + +error[E0277]: the trait bound `[bool]: Copy` is not satisfied + --> $DIR/ice-unsized-struct-arg-issue2-121424.rs:5:22 + | +LL | const MYSLICE_GOOD: &MySliceBool = &MySlice(true, [false]); + | ^^^^^^^^^^^ the trait `Copy` is not implemented for `[bool]` + | + = help: the trait `Copy` is implemented for `[T; N]` +note: required by a bound in `MySlice` + --> $DIR/ice-unsized-struct-arg-issue2-121424.rs:3:19 + | +LL | struct MySlice(bool, T); + | ^^^^ required by this bound in `MySlice` + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. From 6aff1ca68ca056a483cbd935b6e202298bef0080 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 11:11:10 +0100 Subject: [PATCH 15/18] fix warning when building libcore for Miri --- library/core/src/intrinsics.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index aff1c589e628a..9f4a1ed6870bb 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -3120,6 +3120,7 @@ pub(crate) const fn miri_promise_symbolic_alignment(ptr: *const (), align: usize const fn compiletime(_ptr: *const (), _align: usize) {} + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: the extra behavior at runtime is for UB checks only. unsafe { const_eval_select((ptr, align), compiletime, runtime); From 1082c36a4c734972a13734c7e9565a5f2b9fc79a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 11:54:27 +0100 Subject: [PATCH 16/18] fn is_align_to: move some comments closer to the cast they refer to --- library/core/src/ptr/const_ptr.rs | 6 +++--- library/core/src/ptr/mut_ptr.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 0c69bf2aef9c1..90dbc798fcf18 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -1628,12 +1628,12 @@ impl *const T { #[inline] const fn const_impl(ptr: *const (), align: usize) -> bool { // We can't use the address of `self` in a `const fn`, so we use `align_offset` instead. - // The cast to `()` is used to - // 1. deal with fat pointers; and - // 2. ensure that `align_offset` doesn't actually try to compute an offset. ptr.align_offset(align) == 0 } + // The cast to `()` is used to + // 1. deal with fat pointers; and + // 2. ensure that `align_offset` (in `const_impl`) doesn't actually try to compute an offset. #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. unsafe { diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 5ce3b1f298cba..6a9033a144deb 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -1900,12 +1900,12 @@ impl *mut T { #[inline] const fn const_impl(ptr: *mut (), align: usize) -> bool { // We can't use the address of `self` in a `const fn`, so we use `align_offset` instead. - // The cast to `()` is used to - // 1. deal with fat pointers; and - // 2. ensure that `align_offset` doesn't actually try to compute an offset. ptr.align_offset(align) == 0 } + // The cast to `()` is used to + // 1. deal with fat pointers; and + // 2. ensure that `align_offset` (in `const_impl`) doesn't actually try to compute an offset. #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // on bootstrap bump, remove unsafe block // SAFETY: The two versions are equivalent at runtime. unsafe { From e632e3f9a501daefde13a264e1d0aff54a417510 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 9 Mar 2024 13:05:13 +0100 Subject: [PATCH 17/18] miri: do not apply aliasing restrictions to Box with custom allocator --- .../src/interpret/validity.rs | 8 +- .../rustc_const_eval/src/interpret/visitor.rs | 10 +- .../rustc_hir_analysis/src/check/intrinsic.rs | 4 + compiler/rustc_middle/src/ty/sty.rs | 7 +- compiler/rustc_span/src/symbol.rs | 1 + library/alloc/src/boxed.rs | 24 ++-- library/core/src/intrinsics.rs | 13 ++ src/tools/miri/src/borrow_tracker/mod.rs | 15 ++- .../src/borrow_tracker/stacked_borrows/mod.rs | 34 ++++- .../src/borrow_tracker/tree_borrows/mod.rs | 33 +++-- src/tools/miri/src/shims/intrinsics/mod.rs | 13 ++ .../tests/pass/box-custom-alloc-aliasing.rs | 123 ++++++++++++++++++ 12 files changed, 250 insertions(+), 35 deletions(-) create mode 100644 src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index ff1cb43db864e..c568f9acfd38b 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -17,8 +17,8 @@ use rustc_middle::mir::interpret::{ ExpectedKind, InterpError, InvalidMetaKind, Misalignment, PointerKind, Provenance, ValidationErrorInfo, ValidationErrorKind, ValidationErrorKind::*, }; -use rustc_middle::ty; use rustc_middle::ty::layout::{LayoutOf, TyAndLayout}; +use rustc_middle::ty::{self, Ty}; use rustc_span::symbol::{sym, Symbol}; use rustc_target::abi::{ Abi, FieldIdx, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange, @@ -783,7 +783,11 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> } #[inline] - fn visit_box(&mut self, op: &OpTy<'tcx, M::Provenance>) -> InterpResult<'tcx> { + fn visit_box( + &mut self, + _box_ty: Ty<'tcx>, + op: &OpTy<'tcx, M::Provenance>, + ) -> InterpResult<'tcx> { self.check_safe_pointer(op, PointerKind::Box)?; Ok(()) } diff --git a/compiler/rustc_const_eval/src/interpret/visitor.rs b/compiler/rustc_const_eval/src/interpret/visitor.rs index b200ecbf73af5..0e824f3f592d4 100644 --- a/compiler/rustc_const_eval/src/interpret/visitor.rs +++ b/compiler/rustc_const_eval/src/interpret/visitor.rs @@ -3,7 +3,7 @@ use rustc_index::IndexVec; use rustc_middle::mir::interpret::InterpResult; -use rustc_middle::ty; +use rustc_middle::ty::{self, Ty}; use rustc_target::abi::FieldIdx; use rustc_target::abi::{FieldsShape, VariantIdx, Variants}; @@ -47,10 +47,10 @@ pub trait ValueVisitor<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized { Ok(()) } /// Visits the given value as the pointer of a `Box`. There is nothing to recurse into. - /// The type of `v` will be a raw pointer, but this is a field of `Box` and the - /// pointee type is the actual `T`. + /// The type of `v` will be a raw pointer to `T`, but this is a field of `Box` and the + /// pointee type is the actual `T`. `box_ty` provides the full type of the `Box` itself. #[inline(always)] - fn visit_box(&mut self, _v: &Self::V) -> InterpResult<'tcx> { + fn visit_box(&mut self, _box_ty: Ty<'tcx>, _v: &Self::V) -> InterpResult<'tcx> { Ok(()) } @@ -144,7 +144,7 @@ pub trait ValueVisitor<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized { assert_eq!(nonnull_ptr.layout().fields.count(), 1); let raw_ptr = self.ecx().project_field(&nonnull_ptr, 0)?; // the actual raw ptr // ... whose only field finally is a raw ptr we can dereference. - self.visit_box(&raw_ptr)?; + self.visit_box(ty, &raw_ptr)?; // The second `Box` field is the allocator, which we recursively check for validity // like in regular structs. diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 95c51cc0486fc..5a74bdc8a84cb 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -657,6 +657,10 @@ pub fn check_intrinsic_type( sym::simd_shuffle => (3, 0, vec![param(0), param(0), param(1)], param(2)), sym::simd_shuffle_generic => (2, 1, vec![param(0), param(0)], param(1)), + sym::retag_box_to_raw => { + (2, 0, vec![Ty::new_mut_ptr(tcx, param(0))], Ty::new_mut_ptr(tcx, param(0))) + } + other => { tcx.dcx().emit_err(UnrecognizedIntrinsicFunction { span, name: other }); return; diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 427c0f04bd11e..39b5d3b6ea763 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -2008,13 +2008,10 @@ impl<'tcx> Ty<'tcx> { // Single-argument Box is always global. (for "minicore" tests) return true; }; - if let Some(alloc_adt) = alloc.expect_ty().ty_adt_def() { + alloc.expect_ty().ty_adt_def().is_some_and(|alloc_adt| { let global_alloc = tcx.require_lang_item(LangItem::GlobalAlloc, None); alloc_adt.did() == global_alloc - } else { - // Allocator is not an ADT... - false - } + }) } _ => false, } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 9dadd47124771..283c685eaa305 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1450,6 +1450,7 @@ symbols! { residual, result, resume, + retag_box_to_raw, return_position_impl_trait_in_trait, return_type_notation, rhs, diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 2736e5ee6c588..304f607000b01 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -155,6 +155,7 @@ use core::error::Error; use core::fmt; use core::future::Future; use core::hash::{Hash, Hasher}; +use core::intrinsics::retag_box_to_raw; use core::iter::FusedIterator; use core::marker::Tuple; use core::marker::Unsize; @@ -1110,8 +1111,16 @@ impl Box { #[unstable(feature = "allocator_api", issue = "32838")] #[inline] pub fn into_raw_with_allocator(b: Self) -> (*mut T, A) { - let (leaked, alloc) = Box::into_unique(b); - (leaked.as_ptr(), alloc) + // This is the transition point from `Box` to raw pointers. For Stacked Borrows, these casts + // are relevant -- if this is a global allocator Box and we just get the pointer from `b.0`, + // it will have `Unique` permission, which is not what we want from a raw pointer. We could + // fix that by going through `&mut`, but then if this is *not* a global allocator Box, we'd + // be adding uniqueness assertions that we do not want. So for Miri's sake we pass this + // pointer through an intrinsic for box-to-raw casts, which can do the right thing wrt the + // aliasing model. + let b = mem::ManuallyDrop::new(b); + let alloc = unsafe { ptr::read(&b.1) }; + (unsafe { retag_box_to_raw::(b.0.as_ptr()) }, alloc) } #[unstable( @@ -1122,13 +1131,8 @@ impl Box { #[inline] #[doc(hidden)] pub fn into_unique(b: Self) -> (Unique, A) { - // Box is recognized as a "unique pointer" by Stacked Borrows, but internally it is a - // raw pointer for the type system. Turning it directly into a raw pointer would not be - // recognized as "releasing" the unique pointer to permit aliased raw accesses, - // so all raw pointer methods have to go through `Box::leak`. Turning *that* to a raw pointer - // behaves correctly. - let alloc = unsafe { ptr::read(&b.1) }; - (Unique::from(Box::leak(b)), alloc) + let (ptr, alloc) = Box::into_raw_with_allocator(b); + unsafe { (Unique::from(&mut *ptr), alloc) } } /// Returns a reference to the underlying allocator. @@ -1184,7 +1188,7 @@ impl Box { where A: 'a, { - unsafe { &mut *mem::ManuallyDrop::new(b).0.as_ptr() } + unsafe { &mut *Box::into_raw(b) } } /// Converts a `Box` into a `Pin>`. If `T` does not implement [`Unpin`], then diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index aff1c589e628a..f19f5f322aa50 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2695,6 +2695,19 @@ pub unsafe fn vtable_size(_ptr: *const ()) -> usize { unreachable!() } +/// Retag a box pointer as part of casting it to a raw pointer. This is the `Box` equivalent of +/// `(x: &mut T) as *mut T`. The input pointer must be the pointer of a `Box` (passed as raw pointer +/// to avoid all questions around move semantics and custom allocators), and `A` must be the `Box`'s +/// allocator. +#[unstable(feature = "core_intrinsics", issue = "none")] +#[rustc_nounwind] +#[cfg_attr(not(bootstrap), rustc_intrinsic)] +#[cfg_attr(bootstrap, inline)] +pub unsafe fn retag_box_to_raw(ptr: *mut T) -> *mut T { + // Miri needs to adjust the provenance, but for regular codegen this is not needed + ptr +} + // Some functions are defined here because they accidentally got made // available in this module on stable. See . // (`transmute` also falls into this category, but it cannot be wrapped due to the diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 262f7c449d275..884b8a3b9bc68 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -5,7 +5,7 @@ use std::num::NonZero; use smallvec::SmallVec; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_middle::mir::RetagKind; +use rustc_middle::{mir::RetagKind, ty::Ty}; use rustc_target::abi::Size; use crate::*; @@ -291,6 +291,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } + fn retag_box_to_raw( + &mut self, + val: &ImmTy<'tcx, Provenance>, + alloc_ty: Ty<'tcx>, + ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { + let this = self.eval_context_mut(); + let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; + match method { + BorrowTrackerMethod::StackedBorrows => this.sb_retag_box_to_raw(val, alloc_ty), + BorrowTrackerMethod::TreeBorrows => this.tb_retag_box_to_raw(val, alloc_ty), + } + } + fn retag_place_contents( &mut self, kind: RetagKind, diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 86d22229714d5..6eed62d7edc6a 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -865,6 +865,24 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.sb_retag_reference(val, new_perm, RetagInfo { cause, in_field: false }) } + fn sb_retag_box_to_raw( + &mut self, + val: &ImmTy<'tcx, Provenance>, + alloc_ty: Ty<'tcx>, + ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { + let this = self.eval_context_mut(); + let is_global_alloc = alloc_ty.ty_adt_def().is_some_and(|adt| { + let global_alloc = this.tcx.require_lang_item(rustc_hir::LangItem::GlobalAlloc, None); + adt.did() == global_alloc + }); + if is_global_alloc { + // Retag this as-if it was a mutable reference. + this.sb_retag_ptr_value(RetagKind::Raw, val) + } else { + Ok(val.clone()) + } + } + fn sb_retag_place_contents( &mut self, kind: RetagKind, @@ -916,10 +934,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { self.ecx } - fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { - // Boxes get a weak protectors, since they may be deallocated. - let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx); - self.retag_ptr_inplace(place, new_perm) + fn visit_box( + &mut self, + box_ty: Ty<'tcx>, + place: &PlaceTy<'tcx, Provenance>, + ) -> InterpResult<'tcx> { + // Only boxes for the global allocator get any special treatment. + if box_ty.is_box_global(*self.ecx.tcx) { + // Boxes get a weak protectors, since they may be deallocated. + let new_perm = NewPermission::from_box_ty(place.layout.ty, self.kind, self.ecx); + self.retag_ptr_inplace(place, new_perm)?; + } + Ok(()) } fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 4b944ea88f503..ae38ce6e75346 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -392,6 +392,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } + fn tb_retag_box_to_raw( + &mut self, + val: &ImmTy<'tcx, Provenance>, + _alloc_ty: Ty<'tcx>, + ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { + // Casts to raw pointers are NOPs in Tree Borrows. + Ok(val.clone()) + } + /// Retag all pointers that are stored in this place. fn tb_retag_place_contents( &mut self, @@ -441,14 +450,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Regardless of how `Unique` is handled, Boxes are always reborrowed. /// When `Unique` is also reborrowed, then it behaves exactly like `Box` /// except for the fact that `Box` has a non-zero-sized reborrow. - fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { - let new_perm = NewPermission::from_unique_ty( - place.layout.ty, - self.kind, - self.ecx, - /* zero_size */ false, - ); - self.retag_ptr_inplace(place, new_perm) + fn visit_box( + &mut self, + box_ty: Ty<'tcx>, + place: &PlaceTy<'tcx, Provenance>, + ) -> InterpResult<'tcx> { + // Only boxes for the global allocator get any special treatment. + if box_ty.is_box_global(*self.ecx.tcx) { + let new_perm = NewPermission::from_unique_ty( + place.layout.ty, + self.kind, + self.ecx, + /* zero_size */ false, + ); + self.retag_ptr_inplace(place, new_perm)?; + } + Ok(()) } fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { diff --git a/src/tools/miri/src/shims/intrinsics/mod.rs b/src/tools/miri/src/shims/intrinsics/mod.rs index 46f0c771cb51c..075ce44fbceed 100644 --- a/src/tools/miri/src/shims/intrinsics/mod.rs +++ b/src/tools/miri/src/shims/intrinsics/mod.rs @@ -129,6 +129,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_bytes_ptr(ptr, iter::repeat(val_byte).take(byte_count.bytes_usize()))?; } + // Memory model / provenance manipulation "ptr_mask" => { let [ptr, mask] = check_arg_count(args)?; @@ -139,6 +140,18 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_pointer(Pointer::new(ptr.provenance, masked_addr), dest)?; } + "retag_box_to_raw" => { + let [ptr] = check_arg_count(args)?; + let alloc_ty = generic_args[1].expect_ty(); + + let val = this.read_immediate(ptr)?; + let new_val = if this.machine.borrow_tracker.is_some() { + this.retag_box_to_raw(&val, alloc_ty)? + } else { + val + }; + this.write_immediate(*new_val, dest)?; + } // We want to return either `true` or `false` at random, or else something like // ``` diff --git a/src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs b/src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs new file mode 100644 index 0000000000000..541e5f244db08 --- /dev/null +++ b/src/tools/miri/tests/pass/box-custom-alloc-aliasing.rs @@ -0,0 +1,123 @@ +//! Regression test for : +//! If `Box` has a local allocator, then it can't be `noalias` as the allocator +//! may want to access allocator state based on the data pointer. + +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(allocator_api)] +#![feature(strict_provenance)] + +use std::{ + alloc::{AllocError, Allocator, Layout}, + cell::{Cell, UnsafeCell}, + ptr::{self, addr_of, NonNull}, + thread::{self, ThreadId}, + mem, +}; + +const BIN_SIZE: usize = 8; + +// A bin represents a collection of blocks of a specific layout. +#[repr(align(128))] +struct MyBin { + top: Cell, + thread_id: ThreadId, + memory: UnsafeCell<[usize; BIN_SIZE]>, +} + +impl MyBin { + fn pop(&self) -> Option> { + let top = self.top.get(); + if top == BIN_SIZE { + return None; + } + // Cast the *entire* thing to a raw pointer to not restrict its provenance. + let bin = self as *const MyBin; + let base_ptr = UnsafeCell::raw_get(unsafe{ addr_of!((*bin).memory )}).cast::(); + let ptr = unsafe { NonNull::new_unchecked(base_ptr.add(top)) }; + self.top.set(top + 1); + Some(ptr.cast()) + } + + // Pretends to not be a throwaway allocation method like this. A more realistic + // substitute is using intrusive linked lists, which requires access to the + // metadata of this bin as well. + unsafe fn push(&self, ptr: NonNull) { + // For now just check that this really is in this bin. + let start = self.memory.get().addr(); + let end = start + BIN_SIZE * mem::size_of::(); + let addr = ptr.addr().get(); + assert!((start..end).contains(&addr)); + } +} + +// A collection of bins. +struct MyAllocator { + thread_id: ThreadId, + // Pretends to be some complex collection of bins, such as an array of linked lists. + bins: Box<[MyBin; 1]>, +} + +impl MyAllocator { + fn new() -> Self { + let thread_id = thread::current().id(); + MyAllocator { + thread_id, + bins: Box::new( + [MyBin { + top: Cell::new(0), + thread_id, + memory: UnsafeCell::default(), + }; 1], + ), + } + } + + // Pretends to be expensive finding a suitable bin for the layout. + fn find_bin(&self, layout: Layout) -> Option<&MyBin> { + if layout == Layout::new::() { + Some(&self.bins[0]) + } else { + None + } + } +} + +unsafe impl Allocator for MyAllocator { + fn allocate(&self, layout: Layout) -> Result, AllocError> { + // Expensive bin search. + let bin = self.find_bin(layout).ok_or(AllocError)?; + let ptr = bin.pop().ok_or(AllocError)?; + Ok(NonNull::slice_from_raw_parts(ptr, layout.size())) + } + + unsafe fn deallocate(&self, ptr: NonNull, _layout: Layout) { + // Since manually finding the corresponding bin of `ptr` is very expensive, + // doing pointer arithmetics is preferred. + // But this means we access `top` via `ptr` rather than `self`! + // That is fundamentally the source of the aliasing trouble in this example. + let their_bin = ptr.as_ptr().map_addr(|addr| addr & !127).cast::(); + let thread_id = ptr::read(ptr::addr_of!((*their_bin).thread_id)); + if self.thread_id == thread_id { + unsafe { (*their_bin).push(ptr) }; + } else { + todo!("Deallocating from another thread") + } + } +} + +// Make sure to involve `Box` in allocating these, +// as that's where `noalias` may come from. +fn v(t: T, a: A) -> Vec { + (Box::new_in([t], a) as Box<[T], A>).into_vec() +} + +fn main() { + assert!(mem::size_of::() <= 128); // if it grows bigger, the trick to access the "header" no longer works + let my_alloc = MyAllocator::new(); + let a = v(1usize, &my_alloc); + let b = v(2usize, &my_alloc); + assert_eq!(a[0] + 1, b[0]); + assert_eq!(addr_of!(a[0]).wrapping_add(1), addr_of!(b[0])); + drop((a, b)); +} From 7e1969ac13b94694bb6c9315dcecdc6b0ee344ad Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sat, 9 Mar 2024 21:16:18 +0800 Subject: [PATCH 18/18] Remove `Ord` from `ClosureKind` Using `Ord` to accomplish a meaning of subset relationship can be hard to read. The existing uses for that are easily replaced with a `match`, and in my opinion, more readable without needing to resorting to comments to explain the intention. --- compiler/rustc_hir_typeck/src/closure.rs | 17 +++++++---- compiler/rustc_middle/src/ty/instance.rs | 2 +- compiler/rustc_ty_utils/src/instance.rs | 37 +++++++++++++----------- compiler/rustc_type_ir/src/lib.rs | 14 +++++---- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/closure.rs b/compiler/rustc_hir_typeck/src/closure.rs index 1ff961a90890c..c17af666eb9fe 100644 --- a/compiler/rustc_hir_typeck/src/closure.rs +++ b/compiler/rustc_hir_typeck/src/closure.rs @@ -19,7 +19,7 @@ use rustc_target::spec::abi::Abi; use rustc_trait_selection::traits; use rustc_trait_selection::traits::error_reporting::ArgKind; use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _; -use std::cmp; +use rustc_type_ir::ClosureKind; use std::iter; use std::ops::ControlFlow; @@ -437,10 +437,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; if let Some(found_kind) = found_kind { - expected_kind = Some( - expected_kind - .map_or_else(|| found_kind, |current| cmp::min(current, found_kind)), - ); + // always use the closure kind that is more permissive. + match (expected_kind, found_kind) { + (None, _) => expected_kind = Some(found_kind), + (Some(ClosureKind::FnMut), ClosureKind::Fn) => { + expected_kind = Some(ClosureKind::Fn) + } + (Some(ClosureKind::FnOnce), ClosureKind::Fn | ClosureKind::FnMut) => { + expected_kind = Some(found_kind) + } + _ => {} + } } } } diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 260d0885089ef..e381bffa39542 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -30,7 +30,7 @@ pub struct Instance<'tcx> { pub args: GenericArgsRef<'tcx>, } -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] #[derive(TyEncodable, TyDecodable, HashStable, TypeFoldable, TypeVisitable, Lift)] pub enum InstanceDef<'tcx> { /// A user-defined callable item. diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 381681fb1f416..2816bcc888b0c 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -7,6 +7,7 @@ use rustc_middle::ty::GenericArgsRef; use rustc_middle::ty::{self, Instance, TyCtxt, TypeVisitableExt}; use rustc_span::sym; use rustc_trait_selection::traits; +use rustc_type_ir::ClosureKind; use traits::{translate_args, Reveal}; use crate::errors::UnexpectedFnPtrAssociatedItem; @@ -296,23 +297,25 @@ fn resolve_associated_item<'tcx>( { match *rcvr_args.type_at(0).kind() { ty::CoroutineClosure(coroutine_closure_def_id, args) => { - // If we're computing `AsyncFnOnce`/`AsyncFnMut` for a by-ref closure, - // or `AsyncFnOnce` for a by-mut closure, then construct a new body that - // has the right return types. - // - // Specifically, `AsyncFnMut` for a by-ref coroutine-closure just needs - // to have its input and output types fixed (`&mut self` and returning - // `i16` coroutine kind). - if target_kind > args.as_coroutine_closure().kind() { - Some(Instance { - def: ty::InstanceDef::ConstructCoroutineInClosureShim { - coroutine_closure_def_id, - target_kind, - }, - args, - }) - } else { - Some(Instance::new(coroutine_closure_def_id, args)) + match (target_kind, args.as_coroutine_closure().kind()) { + (ClosureKind::FnOnce | ClosureKind::FnMut, ClosureKind::Fn) + | (ClosureKind::FnOnce, ClosureKind::FnMut) => { + // If we're computing `AsyncFnOnce`/`AsyncFnMut` for a by-ref closure, + // or `AsyncFnOnce` for a by-mut closure, then construct a new body that + // has the right return types. + // + // Specifically, `AsyncFnMut` for a by-ref coroutine-closure just needs + // to have its input and output types fixed (`&mut self` and returning + // `i16` coroutine kind). + Some(Instance { + def: ty::InstanceDef::ConstructCoroutineInClosureShim { + coroutine_closure_def_id, + target_kind, + }, + args, + }) + } + _ => Some(Instance::new(coroutine_closure_def_id, args)), } } ty::Closure(closure_def_id, args) => { diff --git a/compiler/rustc_type_ir/src/lib.rs b/compiler/rustc_type_ir/src/lib.rs index 2ded1b956e519..c01baa58ae784 100644 --- a/compiler/rustc_type_ir/src/lib.rs +++ b/compiler/rustc_type_ir/src/lib.rs @@ -369,12 +369,9 @@ rustc_index::newtype_index! { /// /// You can get the environment type of a closure using /// `tcx.closure_env_ty()`. -#[derive(Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)] #[cfg_attr(feature = "nightly", derive(Encodable, Decodable, HashStable_NoContext))] pub enum ClosureKind { - // Warning: Ordering is significant here! The ordering is chosen - // because the trait Fn is a subtrait of FnMut and so in turn, and - // hence we order it so that Fn < FnMut < FnOnce. Fn, FnMut, FnOnce, @@ -394,8 +391,15 @@ impl ClosureKind { /// Returns `true` if a type that impls this closure kind /// must also implement `other`. + #[rustfmt::skip] pub fn extends(self, other: ClosureKind) -> bool { - self <= other + use ClosureKind::*; + match (self, other) { + (Fn, Fn | FnMut | FnOnce) + | (FnMut, FnMut | FnOnce) + | (FnOnce, FnOnce) => true, + _ => false, + } } }