From 5ee4db4e05ecb845fa99b8863a080014f7ada9cb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 19 Apr 2024 22:54:53 -0400 Subject: [PATCH 01/27] Warn/error on self ctor from outer item in inner item --- compiler/rustc_hir_typeck/messages.ftl | 4 + compiler/rustc_hir_typeck/src/errors.rs | 28 ++++++ .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 40 ++++++++- compiler/rustc_lint_defs/src/builtin.rs | 42 +++++++++ .../do-not-ice-on-note_and_explain.rs | 17 ++-- .../do-not-ice-on-note_and_explain.stderr | 85 ++----------------- tests/ui/self/self-ctor-nongeneric.rs | 4 + tests/ui/self/self-ctor-nongeneric.stderr | 27 ++++++ tests/ui/self/self-ctor.rs | 14 +++ tests/ui/self/self-ctor.stderr | 21 +++++ 10 files changed, 199 insertions(+), 83 deletions(-) create mode 100644 tests/ui/self/self-ctor-nongeneric.stderr create mode 100644 tests/ui/self/self-ctor.rs create mode 100644 tests/ui/self/self-ctor.stderr diff --git a/compiler/rustc_hir_typeck/messages.ftl b/compiler/rustc_hir_typeck/messages.ftl index 72b95a9603dd0..6f499947d5cd4 100644 --- a/compiler/rustc_hir_typeck/messages.ftl +++ b/compiler/rustc_hir_typeck/messages.ftl @@ -137,6 +137,10 @@ hir_typeck_rpit_change_return_type = you could change the return type to be a bo hir_typeck_rustcall_incorrect_args = functions with the "rust-call" ABI must take a single non-self tuple argument +hir_typeck_self_ctor_from_outer_item = can't reference `Self` constructor from outer item + .label = the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference + .suggestion = replace `Self` with the actual type + hir_typeck_struct_expr_non_exhaustive = cannot create non-exhaustive {$what} using struct expression diff --git a/compiler/rustc_hir_typeck/src/errors.rs b/compiler/rustc_hir_typeck/src/errors.rs index f250b909596ea..3f12f25265454 100644 --- a/compiler/rustc_hir_typeck/src/errors.rs +++ b/compiler/rustc_hir_typeck/src/errors.rs @@ -651,3 +651,31 @@ pub enum SuggestBoxingForReturnImplTrait { ends: Vec, }, } + +#[derive(Diagnostic)] +#[diag(hir_typeck_self_ctor_from_outer_item, code = E0401)] +pub struct SelfCtorFromOuterItem { + #[primary_span] + pub span: Span, + #[label] + pub impl_span: Span, + #[subdiagnostic] + pub sugg: Option, +} + +#[derive(LintDiagnostic)] +#[diag(hir_typeck_self_ctor_from_outer_item)] +pub struct SelfCtorFromOuterItemLint { + #[label] + pub impl_span: Span, + #[subdiagnostic] + pub sugg: Option, +} + +#[derive(Subdiagnostic)] +#[suggestion(hir_typeck_suggestion, code = "{name}", applicability = "machine-applicable")] +pub struct ReplaceWithName { + #[primary_span] + pub span: Span, + pub name: String, +} diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 6e8ef04445215..dc927e6676597 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -1,5 +1,5 @@ use crate::callee::{self, DeferredCallResolution}; -use crate::errors::CtorIsPrivate; +use crate::errors::{self, CtorIsPrivate}; use crate::method::{self, MethodCallee, SelfSource}; use crate::rvalue_scopes; use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy}; @@ -21,6 +21,7 @@ use rustc_hir_analysis::hir_ty_lowering::{ use rustc_infer::infer::canonical::{Canonical, OriginalQueryValues, QueryResponse}; use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282; use rustc_infer::infer::{DefineOpaqueTypes, InferResult}; +use rustc_lint::builtin::SELF_CONSTRUCTOR_FROM_OUTER_ITEM; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; use rustc_middle::ty::error::TypeError; use rustc_middle::ty::fold::TypeFoldable; @@ -1162,6 +1163,43 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { span, tcx.at(span).type_of(impl_def_id).instantiate_identity(), ); + + // Firstly, check that this SelfCtor even comes from the item we're currently + // typechecking. This can happen because we never validated the resolution of + // SelfCtors, and when we started doing so, we noticed regressions. After + // sufficiently long time, we can remove this check and turn it into a hard + // error in `validate_res_from_ribs` -- it's just difficult to tell whether the + // self type has any generic types during rustc_resolve, which is what we use + // to determine if this is a hard error or warning. + if std::iter::successors(Some(self.body_id.to_def_id()), |def_id| { + self.tcx.generics_of(def_id).parent + }) + .all(|def_id| def_id != impl_def_id) + { + let sugg = ty.normalized.ty_adt_def().map(|def| errors::ReplaceWithName { + span: path_span, + name: self.tcx.item_name(def.did()).to_ident_string(), + }); + if ty.raw.has_param() { + let guar = self.tcx.dcx().emit_err(errors::SelfCtorFromOuterItem { + span: path_span, + impl_span: tcx.def_span(impl_def_id), + sugg, + }); + return (Ty::new_error(self.tcx, guar), res); + } else { + self.tcx.emit_node_span_lint( + SELF_CONSTRUCTOR_FROM_OUTER_ITEM, + hir_id, + path_span, + errors::SelfCtorFromOuterItemLint { + impl_span: tcx.def_span(impl_def_id), + sugg, + }, + ); + } + } + match ty.normalized.ty_adt_def() { Some(adt_def) if adt_def.has_ctor() => { let (ctor_kind, ctor_def_id) = adt_def.non_enum_variant().ctor.unwrap(); diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 5369454577249..3ddf7f9e6c022 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -90,6 +90,7 @@ declare_lint_pass! { RUST_2021_PREFIXES_INCOMPATIBLE_SYNTAX, RUST_2021_PRELUDE_COLLISIONS, RUST_2024_INCOMPATIBLE_PAT, + SELF_CONSTRUCTOR_FROM_OUTER_ITEM, SEMICOLON_IN_EXPRESSIONS_FROM_MACROS, SINGLE_USE_LIFETIMES, SOFT_UNSTABLE, @@ -3149,6 +3150,47 @@ declare_lint! { "detects `#[unstable]` on stable trait implementations for stable types" } +declare_lint! { + /// The `self_constructor_from_outer_item` lint detects cases where the `Self` constructor + /// was silently allowed due to a bug in the resolver, and which may produce surprising + /// and unintended behavior. + /// + /// Using a `Self` type alias from an outer item was never intended, but was silently allowed. + /// This is deprecated -- and is a hard error when the `Self` type alias references generics + /// that are not in scope. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![deny(self_constructor_from_outer_item)] + /// + /// struct S0(usize); + /// + /// impl S0 { + /// fn foo() { + /// const C: S0 = Self(0); + /// fn bar() -> S0 { + /// Self(0) + /// } + /// } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// The `Self` type alias should not be reachable because nested items are not associated with + /// the scope of the parameters from the parent item. + pub SELF_CONSTRUCTOR_FROM_OUTER_ITEM, + Warn, + "detect unsupported use of `Self` from outer item", + @future_incompatible = FutureIncompatibleInfo { + reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps, + reference: "issue #124186 ", + }; +} + declare_lint! { /// The `semicolon_in_expressions_from_macros` lint detects trailing semicolons /// in macro bodies when the macro is invoked in expression position. diff --git a/tests/ui/malformed/do-not-ice-on-note_and_explain.rs b/tests/ui/malformed/do-not-ice-on-note_and_explain.rs index e65276fb738dd..be0b18a00d28f 100644 --- a/tests/ui/malformed/do-not-ice-on-note_and_explain.rs +++ b/tests/ui/malformed/do-not-ice-on-note_and_explain.rs @@ -1,7 +1,12 @@ struct A(B); -implA{fn d(){fn d(){Self(1)}}} -//~^ ERROR the size for values of type `B` cannot be known at compilation time -//~| ERROR the size for values of type `B` cannot be known at compilation time -//~| ERROR mismatched types -//~| ERROR mismatched types -//~| ERROR `main` function not found in crate + +impl A { + fn d() { + fn d() { + Self(1) + //~^ ERROR can't reference `Self` constructor from outer item + } + } +} + +fn main() {} diff --git a/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr b/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr index 41d0f17366b1e..11a8c01e49094 100644 --- a/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr +++ b/tests/ui/malformed/do-not-ice-on-note_and_explain.stderr @@ -1,79 +1,12 @@ -error[E0601]: `main` function not found in crate `do_not_ice_on_note_and_explain` - --> $DIR/do-not-ice-on-note_and_explain.rs:2:37 +error[E0401]: can't reference `Self` constructor from outer item + --> $DIR/do-not-ice-on-note_and_explain.rs:6:13 | -LL | implA{fn d(){fn d(){Self(1)}}} - | ^ consider adding a `main` function to `$DIR/do-not-ice-on-note_and_explain.rs` +LL | impl A { + | ------------ the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +... +LL | Self(1) + | ^^^^ help: replace `Self` with the actual type: `A` -error[E0277]: the size for values of type `B` cannot be known at compilation time - --> $DIR/do-not-ice-on-note_and_explain.rs:2:32 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | - ---- ^ doesn't have a size known at compile-time - | | | - | | required by a bound introduced by this call - | this type parameter needs to be `Sized` - | -note: required by a bound in `A` - --> $DIR/do-not-ice-on-note_and_explain.rs:1:10 - | -LL | struct A(B); - | ^ required by this bound in `A` - -error[E0308]: mismatched types - --> $DIR/do-not-ice-on-note_and_explain.rs:2:32 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | ---- ^ expected type parameter `B`, found integer - | | - | arguments to this function are incorrect - | - = note: expected type parameter `B` - found type `{integer}` -note: tuple struct defined here - --> $DIR/do-not-ice-on-note_and_explain.rs:1:8 - | -LL | struct A(B); - | ^ - -error[E0308]: mismatched types - --> $DIR/do-not-ice-on-note_and_explain.rs:2:27 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | ^^^^^^^ expected `()`, found `A` - | - = note: expected unit type `()` - found struct `A` -help: consider using a semicolon here - | -LL | implA{fn d(){fn d(){Self(1);}}} - | + -help: try adding a return type - | -LL | implA{fn d(){fn d() -> A{Self(1)}}} - | +++++++ - -error[E0277]: the size for values of type `B` cannot be known at compilation time - --> $DIR/do-not-ice-on-note_and_explain.rs:2:27 - | -LL | implA{fn d(){fn d(){Self(1)}}} - | - ^^^^^^^ doesn't have a size known at compile-time - | | - | this type parameter needs to be `Sized` - | -note: required by an implicit `Sized` bound in `A` - --> $DIR/do-not-ice-on-note_and_explain.rs:1:10 - | -LL | struct A(B); - | ^ required by the implicit `Sized` requirement on this type parameter in `A` -help: you could relax the implicit `Sized` bound on `B` if it were used through indirection like `&B` or `Box` - --> $DIR/do-not-ice-on-note_and_explain.rs:1:10 - | -LL | struct A(B); - | ^ - ...if indirection were used here: `Box` - | | - | this could be changed to `B: ?Sized`... - -error: aborting due to 5 previous errors +error: aborting due to 1 previous error -Some errors have detailed explanations: E0277, E0308, E0601. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0401`. diff --git a/tests/ui/self/self-ctor-nongeneric.rs b/tests/ui/self/self-ctor-nongeneric.rs index 0594e87a0a464..c32cf9df6946e 100644 --- a/tests/ui/self/self-ctor-nongeneric.rs +++ b/tests/ui/self/self-ctor-nongeneric.rs @@ -6,8 +6,12 @@ struct S0(usize); impl S0 { fn foo() { const C: S0 = Self(0); + //~^ WARN can't reference `Self` constructor from outer item + //~| WARN this was previously accepted by the compiler but is being phased out fn bar() -> S0 { Self(0) + //~^ WARN can't reference `Self` constructor from outer item + //~| WARN this was previously accepted by the compiler but is being phased out } } } diff --git a/tests/ui/self/self-ctor-nongeneric.stderr b/tests/ui/self/self-ctor-nongeneric.stderr new file mode 100644 index 0000000000000..6c03c6f3e38a4 --- /dev/null +++ b/tests/ui/self/self-ctor-nongeneric.stderr @@ -0,0 +1,27 @@ +warning: can't reference `Self` constructor from outer item + --> $DIR/self-ctor-nongeneric.rs:8:23 + | +LL | impl S0 { + | ------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +LL | fn foo() { +LL | const C: S0 = Self(0); + | ^^^^ help: replace `Self` with the actual type: `S0` + | + = 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 #124186 + = note: `#[warn(self_constructor_from_outer_item)]` on by default + +warning: can't reference `Self` constructor from outer item + --> $DIR/self-ctor-nongeneric.rs:12:13 + | +LL | impl S0 { + | ------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +... +LL | Self(0) + | ^^^^ help: replace `Self` with the actual type: `S0` + | + = 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 #124186 + +warning: 2 warnings emitted + diff --git a/tests/ui/self/self-ctor.rs b/tests/ui/self/self-ctor.rs new file mode 100644 index 0000000000000..d166499f88412 --- /dev/null +++ b/tests/ui/self/self-ctor.rs @@ -0,0 +1,14 @@ +struct S0(T); + +impl S0 { + fn foo() { + const C: S0 = Self(0); + //~^ ERROR can't reference `Self` constructor from outer item + fn bar() -> S0 { + Self(0) + //~^ ERROR can't reference `Self` constructor from outer item + } + } +} + +fn main() {} diff --git a/tests/ui/self/self-ctor.stderr b/tests/ui/self/self-ctor.stderr new file mode 100644 index 0000000000000..0cb22baaa1a05 --- /dev/null +++ b/tests/ui/self/self-ctor.stderr @@ -0,0 +1,21 @@ +error[E0401]: can't reference `Self` constructor from outer item + --> $DIR/self-ctor.rs:5:28 + | +LL | impl S0 { + | ------------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +LL | fn foo() { +LL | const C: S0 = Self(0); + | ^^^^ help: replace `Self` with the actual type: `S0` + +error[E0401]: can't reference `Self` constructor from outer item + --> $DIR/self-ctor.rs:8:13 + | +LL | impl S0 { + | ------------- the inner item doesn't inherit generics from this impl, so `Self` is invalid to reference +... +LL | Self(0) + | ^^^^ help: replace `Self` with the actual type: `S0` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0401`. From a25bb5f4acee081295ab83a31e57b98d8d559df0 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 19 Apr 2024 22:59:34 -0400 Subject: [PATCH 02/27] (Mostly) revert "Account for type param from other item in `note_and_explain`" This mostly reverts commit 7449478c2f6fd2d72c12a51d8562f1e6108facab. It also removes an `opt_param_at` that really is unnecessary given our ICE policy for malformed intrinsics. --- .../rustc_hir_analysis/src/check/intrinsic.rs | 5 +- .../infer/error_reporting/note_and_explain.rs | 105 +++++++----------- compiler/rustc_middle/src/ty/generics.rs | 28 ----- 3 files changed, 45 insertions(+), 93 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 00ff470a0a7d7..7b9a5b84b4324 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -164,9 +164,8 @@ pub fn check_intrinsic_type( ) { let generics = tcx.generics_of(intrinsic_id); let param = |n| { - if let Some(&ty::GenericParamDef { - name, kind: ty::GenericParamDefKind::Type { .. }, .. - }) = generics.opt_param_at(n as usize, tcx) + if let &ty::GenericParamDef { name, kind: ty::GenericParamDefKind::Type { .. }, .. } = + generics.param_at(n as usize, tcx) { Ty::new_param(tcx, n, name) } else { diff --git a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs index c24ad1fa1e73a..7ccb78a7d0b49 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs @@ -54,17 +54,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } (ty::Param(expected), ty::Param(found)) => { let generics = tcx.generics_of(body_owner_def_id); - if let Some(param) = generics.opt_type_param(expected, tcx) { - let e_span = tcx.def_span(param.def_id); - if !sp.contains(e_span) { - diag.span_label(e_span, "expected type parameter"); - } + let e_span = tcx.def_span(generics.type_param(expected, tcx).def_id); + if !sp.contains(e_span) { + diag.span_label(e_span, "expected type parameter"); } - if let Some(param) = generics.opt_type_param(found, tcx) { - let f_span = tcx.def_span(param.def_id); - if !sp.contains(f_span) { - diag.span_label(f_span, "found type parameter"); - } + let f_span = tcx.def_span(generics.type_param(found, tcx).def_id); + if !sp.contains(f_span) { + diag.span_label(f_span, "found type parameter"); } diag.note( "a type parameter was expected, but a different one was found; \ @@ -87,29 +83,22 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { | (ty::Alias(ty::Projection, proj), ty::Param(p)) if !tcx.is_impl_trait_in_trait(proj.def_id) => { - let parent = tcx - .generics_of(body_owner_def_id) - .opt_type_param(p, tcx) - .and_then(|param| { - let p_def_id = param.def_id; - let p_span = tcx.def_span(p_def_id); - let expected = match (values.expected.kind(), values.found.kind()) { - (ty::Param(_), _) => "expected ", - (_, ty::Param(_)) => "found ", - _ => "", - }; - if !sp.contains(p_span) { - diag.span_label( - p_span, - format!("{expected}this type parameter"), - ); - } - p_def_id.as_local().and_then(|id| { - let local_id = tcx.local_def_id_to_hir_id(id); - let generics = tcx.parent_hir_node(local_id).generics()?; - Some((id, generics)) - }) - }); + let param = tcx.generics_of(body_owner_def_id).type_param(p, tcx); + let p_def_id = param.def_id; + let p_span = tcx.def_span(p_def_id); + let expected = match (values.expected.kind(), values.found.kind()) { + (ty::Param(_), _) => "expected ", + (_, ty::Param(_)) => "found ", + _ => "", + }; + if !sp.contains(p_span) { + diag.span_label(p_span, format!("{expected}this type parameter")); + } + let parent = p_def_id.as_local().and_then(|id| { + let local_id = tcx.local_def_id_to_hir_id(id); + let generics = tcx.parent_hir_node(local_id).generics()?; + Some((id, generics)) + }); let mut note = true; if let Some((local_id, generics)) = parent { // Synthesize the associated type restriction `Add`. @@ -183,16 +172,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { (ty::Param(p), ty::Dynamic(..) | ty::Alias(ty::Opaque, ..)) | (ty::Dynamic(..) | ty::Alias(ty::Opaque, ..), ty::Param(p)) => { let generics = tcx.generics_of(body_owner_def_id); - if let Some(param) = generics.opt_type_param(p, tcx) { - let p_span = tcx.def_span(param.def_id); - let expected = match (values.expected.kind(), values.found.kind()) { - (ty::Param(_), _) => "expected ", - (_, ty::Param(_)) => "found ", - _ => "", - }; - if !sp.contains(p_span) { - diag.span_label(p_span, format!("{expected}this type parameter")); - } + let p_span = tcx.def_span(generics.type_param(p, tcx).def_id); + let expected = match (values.expected.kind(), values.found.kind()) { + (ty::Param(_), _) => "expected ", + (_, ty::Param(_)) => "found ", + _ => "", + }; + if !sp.contains(p_span) { + diag.span_label(p_span, format!("{expected}this type parameter")); } diag.help("type parameters must be constrained to match other types"); if tcx.sess.teach(diag.code.unwrap()) { @@ -233,11 +220,9 @@ impl Trait for X { ty::Closure(..) | ty::CoroutineClosure(..) | ty::Coroutine(..), ) => { let generics = tcx.generics_of(body_owner_def_id); - if let Some(param) = generics.opt_type_param(p, tcx) { - let p_span = tcx.def_span(param.def_id); - if !sp.contains(p_span) { - diag.span_label(p_span, "expected this type parameter"); - } + let p_span = tcx.def_span(generics.type_param(p, tcx).def_id); + if !sp.contains(p_span) { + diag.span_label(p_span, "expected this type parameter"); } diag.help(format!( "every closure has a distinct type and so could not always match the \ @@ -246,16 +231,14 @@ impl Trait for X { } (ty::Param(p), _) | (_, ty::Param(p)) => { let generics = tcx.generics_of(body_owner_def_id); - if let Some(param) = generics.opt_type_param(p, tcx) { - let p_span = tcx.def_span(param.def_id); - let expected = match (values.expected.kind(), values.found.kind()) { - (ty::Param(_), _) => "expected ", - (_, ty::Param(_)) => "found ", - _ => "", - }; - if !sp.contains(p_span) { - diag.span_label(p_span, format!("{expected}this type parameter")); - } + let p_span = tcx.def_span(generics.type_param(p, tcx).def_id); + let expected = match (values.expected.kind(), values.found.kind()) { + (ty::Param(_), _) => "expected ", + (_, ty::Param(_)) => "found ", + _ => "", + }; + if !sp.contains(p_span) { + diag.span_label(p_span, format!("{expected}this type parameter")); } } (ty::Alias(ty::Projection | ty::Inherent, proj_ty), _) @@ -545,10 +528,8 @@ impl Trait for X { return false; }; let generics = tcx.generics_of(body_owner_def_id); - let Some(param) = generics.opt_type_param(param_ty, tcx) else { - return false; - }; - let Some(def_id) = param.def_id.as_local() else { + let def_id = generics.type_param(param_ty, tcx).def_id; + let Some(def_id) = def_id.as_local() else { return false; }; diff --git a/compiler/rustc_middle/src/ty/generics.rs b/compiler/rustc_middle/src/ty/generics.rs index 04655c5d20bad..0b447b28cd4c5 100644 --- a/compiler/rustc_middle/src/ty/generics.rs +++ b/compiler/rustc_middle/src/ty/generics.rs @@ -244,20 +244,6 @@ impl<'tcx> Generics { } } - /// Returns the `GenericParamDef` with the given index if available. - pub fn opt_param_at( - &'tcx self, - param_index: usize, - tcx: TyCtxt<'tcx>, - ) -> Option<&'tcx GenericParamDef> { - if let Some(index) = param_index.checked_sub(self.parent_count) { - self.own_params.get(index) - } else { - tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?")) - .opt_param_at(param_index, tcx) - } - } - pub fn params_to(&'tcx self, param_index: usize, tcx: TyCtxt<'tcx>) -> &'tcx [GenericParamDef] { if let Some(index) = param_index.checked_sub(self.parent_count) { &self.own_params[..index] @@ -289,20 +275,6 @@ impl<'tcx> Generics { } } - /// Returns the `GenericParamDef` associated with this `ParamTy` if it belongs to this - /// `Generics`. - pub fn opt_type_param( - &'tcx self, - param: ParamTy, - tcx: TyCtxt<'tcx>, - ) -> Option<&'tcx GenericParamDef> { - let param = self.opt_param_at(param.index as usize, tcx)?; - match param.kind { - GenericParamDefKind::Type { .. } => Some(param), - _ => None, - } - } - /// Returns the `GenericParamDef` associated with this `ParamConst`. pub fn const_param(&'tcx self, param: ParamConst, tcx: TyCtxt<'tcx>) -> &GenericParamDef { let param = self.param_at(param.index as usize, tcx); From 0334c45bb525f7eb9078f3c5b5074320a9ac7a3d Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 1 May 2024 10:32:26 +0200 Subject: [PATCH 03/27] Write `char::DebugEscape` sequences using `write_str` Instead of writing each `char` of an escape sequence one by one, this delegates to `Display`, which uses `write_str` internally in order to write the whole escape sequence at once. --- library/core/benches/str/debug.rs | 4 ++-- library/core/src/fmt/mod.rs | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/library/core/benches/str/debug.rs b/library/core/benches/str/debug.rs index 7c72228f0fb5b..cb91169eed8eb 100644 --- a/library/core/benches/str/debug.rs +++ b/library/core/benches/str/debug.rs @@ -44,7 +44,7 @@ fn ascii_escapes(b: &mut Bencher) { assert_fmt( s, r#""some\tmore\tascii\ttext\nthis time with some \"escapes\", also 64 byte""#, - 21, + 15, ); b.iter(|| { black_box(format!("{:?}", black_box(s))); @@ -72,7 +72,7 @@ fn mostly_unicode(b: &mut Bencher) { #[bench] fn mixed(b: &mut Bencher) { let s = "\"❤️\"\n\"hűha ez betű\"\n\"кириллических букв\"."; - assert_fmt(s, r#""\"❤\u{fe0f}\"\n\"hűha ez betű\"\n\"кириллических букв\".""#, 36); + assert_fmt(s, r#""\"❤\u{fe0f}\"\n\"hűha ez betű\"\n\"кириллических букв\".""#, 21); b.iter(|| { black_box(format!("{:?}", black_box(s))); }); diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 9b372eac52455..10e1d27c88a92 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -2409,9 +2409,7 @@ impl Debug for str { // If char needs escaping, flush backlog so far and write, else skip if esc.len() != 1 { f.write_str(&self[from..i])?; - for c in esc { - f.write_char(c)?; - } + Display::fmt(&esc, f)?; from = i + c.len_utf8(); } } @@ -2431,13 +2429,12 @@ impl Display for str { impl Debug for char { fn fmt(&self, f: &mut Formatter<'_>) -> Result { f.write_char('\'')?; - for c in self.escape_debug_ext(EscapeDebugExtArgs { + let esc = self.escape_debug_ext(EscapeDebugExtArgs { escape_grapheme_extended: true, escape_single_quote: true, escape_double_quote: false, - }) { - f.write_char(c)? - } + }); + Display::fmt(&esc, f)?; f.write_char('\'') } } From 3fda931afe98ccfbc2da7307731b268a38b153e9 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Thu, 15 Feb 2024 17:36:21 +0100 Subject: [PATCH 04/27] Add a fast-path to `Debug` ASCII `&str` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of going through the `EscapeDebug` machinery, we can just skip over ASCII chars that don’t need any escaping. --- library/core/src/fmt/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 10e1d27c88a92..4cc2a9cf96d82 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -2401,6 +2401,11 @@ impl Debug for str { f.write_char('"')?; let mut from = 0; for (i, c) in self.char_indices() { + // a fast path for ASCII chars that do not need escapes: + if matches!(c, ' '..='~') && !matches!(c, '\\' | '\"') { + continue; + } + let esc = c.escape_debug_ext(EscapeDebugExtArgs { escape_grapheme_extended: true, escape_single_quote: false, From 42d870ec8815b860b31f03f938ad911a49a5cff3 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Fri, 3 May 2024 13:16:26 +0200 Subject: [PATCH 05/27] Introduce printable-ASCII fast-path for `impl Debug for str` Instead of having a single loop that works on utf-8 `char`s, this splits the implementation into a loop that quickly skips over printable ASCII, falling back to per-char iteration for other chunks. --- library/core/src/fmt/mod.rs | 59 ++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 4cc2a9cf96d82..7fbbbb67f82e6 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -2399,26 +2399,51 @@ impl Display for bool { impl Debug for str { fn fmt(&self, f: &mut Formatter<'_>) -> Result { f.write_char('"')?; - let mut from = 0; - for (i, c) in self.char_indices() { - // a fast path for ASCII chars that do not need escapes: - if matches!(c, ' '..='~') && !matches!(c, '\\' | '\"') { - continue; - } - let esc = c.escape_debug_ext(EscapeDebugExtArgs { - escape_grapheme_extended: true, - escape_single_quote: false, - escape_double_quote: true, - }); - // If char needs escaping, flush backlog so far and write, else skip - if esc.len() != 1 { - f.write_str(&self[from..i])?; - Display::fmt(&esc, f)?; - from = i + c.len_utf8(); + // substring we know is printable + let mut printable_range = 0..0; + + fn needs_escape(b: u8) -> bool { + b > 0x7E || b < 0x20 || b == b'\\' || b == b'"' + } + + // the outer loop here splits the string into chunks of printable ASCII, which is just skipped over, + // and chunks of other chars (unicode, or ASCII that needs escaping), which is handler per-`char`. + let mut rest = self.as_bytes(); + while rest.len() > 0 { + let Some(non_printable_start) = rest.iter().position(|&b| needs_escape(b)) else { + printable_range.end += rest.len(); + break; + }; + + printable_range.end += non_printable_start; + // SAFETY: the position was derived from an iterator, so is known to be within bounds, and at a char boundary + rest = unsafe { rest.get_unchecked(non_printable_start..) }; + + let printable_start = rest.iter().position(|&b| !needs_escape(b)).unwrap_or(rest.len()); + let prefix; + // SAFETY: the position was derived from an iterator, so is known to be within bounds, and at a char boundary + (prefix, rest) = unsafe { rest.split_at_unchecked(printable_start) }; + // SAFETY: prefix is a valid utf8 sequence, and at a char boundary + let prefix = unsafe { crate::str::from_utf8_unchecked(prefix) }; + + for c in prefix.chars() { + let esc = c.escape_debug_ext(EscapeDebugExtArgs { + escape_grapheme_extended: true, + escape_single_quote: false, + escape_double_quote: true, + }); + if esc.len() != 1 { + f.write_str(&self[printable_range.clone()])?; + Display::fmt(&esc, f)?; + printable_range.start = printable_range.end + c.len_utf8(); + } + printable_range.end += c.len_utf8(); } } - f.write_str(&self[from..])?; + + f.write_str(&self[printable_range])?; + f.write_char('"') } } From aaba972e06a35ca6988f41f31ca56d747eac4dc9 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 20 May 2024 11:31:02 +0200 Subject: [PATCH 06/27] Switch to primarily using `&str` Surprisingly, benchmarks have shown that using `&str` instead of `&[u8]` with some `unsafe` code is actually faster. --- library/core/src/fmt/mod.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index 7fbbbb67f82e6..b9f6b2d35c9f2 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -2409,9 +2409,10 @@ impl Debug for str { // the outer loop here splits the string into chunks of printable ASCII, which is just skipped over, // and chunks of other chars (unicode, or ASCII that needs escaping), which is handler per-`char`. - let mut rest = self.as_bytes(); + let mut rest = self; while rest.len() > 0 { - let Some(non_printable_start) = rest.iter().position(|&b| needs_escape(b)) else { + let Some(non_printable_start) = rest.as_bytes().iter().position(|&b| needs_escape(b)) + else { printable_range.end += rest.len(); break; }; @@ -2420,12 +2421,10 @@ impl Debug for str { // SAFETY: the position was derived from an iterator, so is known to be within bounds, and at a char boundary rest = unsafe { rest.get_unchecked(non_printable_start..) }; - let printable_start = rest.iter().position(|&b| !needs_escape(b)).unwrap_or(rest.len()); + let printable_start = + rest.as_bytes().iter().position(|&b| !needs_escape(b)).unwrap_or(rest.len()); let prefix; - // SAFETY: the position was derived from an iterator, so is known to be within bounds, and at a char boundary - (prefix, rest) = unsafe { rest.split_at_unchecked(printable_start) }; - // SAFETY: prefix is a valid utf8 sequence, and at a char boundary - let prefix = unsafe { crate::str::from_utf8_unchecked(prefix) }; + (prefix, rest) = rest.split_at(printable_start); for c in prefix.chars() { let esc = c.escape_debug_ext(EscapeDebugExtArgs { From d6e69188578d95d6bb75cd00cec62e375e62ba6b Mon Sep 17 00:00:00 2001 From: blyxyas Date: Thu, 23 May 2024 18:45:03 +0200 Subject: [PATCH 07/27] Make clamp inline --- library/core/src/cmp.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index fc6022ab75357..f3f757ce69df7 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -898,6 +898,7 @@ pub trait Ord: Eq + PartialOrd { /// assert_eq!(2.clamp(-2, 1), 1); /// ``` #[must_use] + #[inline] #[stable(feature = "clamp", since = "1.50.0")] fn clamp(self, min: Self, max: Self) -> Self where From 004100c222638c980b6509aba0ed4990181fa5dc Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 20 May 2024 12:04:21 +0200 Subject: [PATCH 08/27] Process a single not-ASCII-printable `char` per iteration This avoids having to collect a non-ASCII-printable run before processing it. --- library/core/src/fmt/mod.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/library/core/src/fmt/mod.rs b/library/core/src/fmt/mod.rs index b9f6b2d35c9f2..7f115d5cb20ba 100644 --- a/library/core/src/fmt/mod.rs +++ b/library/core/src/fmt/mod.rs @@ -2407,8 +2407,8 @@ impl Debug for str { b > 0x7E || b < 0x20 || b == b'\\' || b == b'"' } - // the outer loop here splits the string into chunks of printable ASCII, which is just skipped over, - // and chunks of other chars (unicode, or ASCII that needs escaping), which is handler per-`char`. + // the loop here first skips over runs of printable ASCII as a fast path. + // other chars (unicode, or ASCII that needs escaping) are then handled per-`char`. let mut rest = self; while rest.len() > 0 { let Some(non_printable_start) = rest.as_bytes().iter().position(|&b| needs_escape(b)) @@ -2421,12 +2421,8 @@ impl Debug for str { // SAFETY: the position was derived from an iterator, so is known to be within bounds, and at a char boundary rest = unsafe { rest.get_unchecked(non_printable_start..) }; - let printable_start = - rest.as_bytes().iter().position(|&b| !needs_escape(b)).unwrap_or(rest.len()); - let prefix; - (prefix, rest) = rest.split_at(printable_start); - - for c in prefix.chars() { + let mut chars = rest.chars(); + if let Some(c) = chars.next() { let esc = c.escape_debug_ext(EscapeDebugExtArgs { escape_grapheme_extended: true, escape_single_quote: false, @@ -2439,6 +2435,7 @@ impl Debug for str { } printable_range.end += c.len_utf8(); } + rest = chars.as_str(); } f.write_str(&self[printable_range])?; From 24afa42a90eb035e24e5dcffbc538fb96277e3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Thu, 23 May 2024 19:18:07 +0200 Subject: [PATCH 09/27] Properly deal with missing/placeholder types inside GACs --- compiler/rustc_hir_typeck/src/lib.rs | 13 +++++--- .../assoc-const-missing-type.rs | 17 ++++++++++ .../assoc-const-missing-type.stderr | 32 +++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 tests/ui/generic-const-items/assoc-const-missing-type.rs create mode 100644 tests/ui/generic-const-items/assoc-const-missing-type.stderr diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index b9a99fbec3cd8..4f35acaa44755 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -235,11 +235,16 @@ fn infer_type_if_missing<'tcx>(fcx: &FnCtxt<'_, 'tcx>, node: Node<'tcx>) -> Opti if let Some(item) = tcx.opt_associated_item(def_id.into()) && let ty::AssocKind::Const = item.kind && let ty::ImplContainer = item.container - && let Some(trait_item) = item.trait_item_def_id + && let Some(trait_item_def_id) = item.trait_item_def_id { - let args = - tcx.impl_trait_ref(item.container_id(tcx)).unwrap().instantiate_identity().args; - Some(tcx.type_of(trait_item).instantiate(tcx, args)) + let impl_def_id = item.container_id(tcx); + let impl_trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap().instantiate_identity(); + let args = ty::GenericArgs::identity_for_item(tcx, def_id).rebase_onto( + tcx, + impl_def_id, + impl_trait_ref.args, + ); + Some(tcx.type_of(trait_item_def_id).instantiate(tcx, args)) } else { Some(fcx.next_ty_var(span)) } diff --git a/tests/ui/generic-const-items/assoc-const-missing-type.rs b/tests/ui/generic-const-items/assoc-const-missing-type.rs new file mode 100644 index 0000000000000..80f282ea134dc --- /dev/null +++ b/tests/ui/generic-const-items/assoc-const-missing-type.rs @@ -0,0 +1,17 @@ +// Ensure that we properly deal with missing/placeholder types inside GACs. +// issue: rust-lang/rust#124833 +#![feature(generic_const_items)] +#![allow(incomplete_features)] + +trait Trait { + const K: T; +} + +impl Trait for () { + const K = (); + //~^ ERROR missing type for `const` item + //~| ERROR mismatched types + //~| ERROR mismatched types +} + +fn main() {} diff --git a/tests/ui/generic-const-items/assoc-const-missing-type.stderr b/tests/ui/generic-const-items/assoc-const-missing-type.stderr new file mode 100644 index 0000000000000..158e9a068f657 --- /dev/null +++ b/tests/ui/generic-const-items/assoc-const-missing-type.stderr @@ -0,0 +1,32 @@ +error[E0308]: mismatched types + --> $DIR/assoc-const-missing-type.rs:11:18 + | +LL | const K = (); + | - ^^ expected type parameter `T`, found `()` + | | + | expected this type parameter + | + = note: expected type parameter `T` + found unit type `()` + +error: missing type for `const` item + --> $DIR/assoc-const-missing-type.rs:11:15 + | +LL | const K = (); + | ^ help: provide a type for the associated constant: `()` + +error[E0308]: mismatched types + --> $DIR/assoc-const-missing-type.rs:11:18 + | +LL | const K = (); + | - ^^ expected type parameter `T`, found `()` + | | + | expected this type parameter + | + = note: expected type parameter `T` + found unit type `()` + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. From 39d9b840bbd2ed663241f4800f4d07b7950df46f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Thu, 23 May 2024 20:05:28 +0200 Subject: [PATCH 10/27] Handle trait/impl GAC mismatches when inferring missing/placeholder types --- compiler/rustc_hir_typeck/src/lib.rs | 3 ++- tests/crashes/124833.rs | 10 ------- .../assoc-const-missing-type.rs | 4 +++ .../assoc-const-missing-type.stderr | 26 +++++++++++++++---- 4 files changed, 27 insertions(+), 16 deletions(-) delete mode 100644 tests/crashes/124833.rs diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index 4f35acaa44755..86fe2756b59a0 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -244,7 +244,8 @@ fn infer_type_if_missing<'tcx>(fcx: &FnCtxt<'_, 'tcx>, node: Node<'tcx>) -> Opti impl_def_id, impl_trait_ref.args, ); - Some(tcx.type_of(trait_item_def_id).instantiate(tcx, args)) + tcx.check_args_compatible(trait_item_def_id, args) + .then(|| tcx.type_of(trait_item_def_id).instantiate(tcx, args)) } else { Some(fcx.next_ty_var(span)) } diff --git a/tests/crashes/124833.rs b/tests/crashes/124833.rs deleted file mode 100644 index f1c4847b544a5..0000000000000 --- a/tests/crashes/124833.rs +++ /dev/null @@ -1,10 +0,0 @@ -//@ known-bug: rust-lang/rust#124833 -#![feature(generic_const_items)] - -trait Trait { - const C<'a>: &'a str; -} - -impl Trait for () { - const C<'a>: = "C"; -} diff --git a/tests/ui/generic-const-items/assoc-const-missing-type.rs b/tests/ui/generic-const-items/assoc-const-missing-type.rs index 80f282ea134dc..93160f0b5758d 100644 --- a/tests/ui/generic-const-items/assoc-const-missing-type.rs +++ b/tests/ui/generic-const-items/assoc-const-missing-type.rs @@ -5,6 +5,7 @@ trait Trait { const K: T; + const Q<'a>: &'a str; } impl Trait for () { @@ -12,6 +13,9 @@ impl Trait for () { //~^ ERROR missing type for `const` item //~| ERROR mismatched types //~| ERROR mismatched types + const Q = ""; + //~^ ERROR missing type for `const` item + //~| ERROR lifetime parameters or bounds on const `Q` do not match the trait declaration } fn main() {} diff --git a/tests/ui/generic-const-items/assoc-const-missing-type.stderr b/tests/ui/generic-const-items/assoc-const-missing-type.stderr index 158e9a068f657..6f35c0958d45c 100644 --- a/tests/ui/generic-const-items/assoc-const-missing-type.stderr +++ b/tests/ui/generic-const-items/assoc-const-missing-type.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/assoc-const-missing-type.rs:11:18 + --> $DIR/assoc-const-missing-type.rs:12:18 | LL | const K = (); | - ^^ expected type parameter `T`, found `()` @@ -10,13 +10,28 @@ LL | const K = (); found unit type `()` error: missing type for `const` item - --> $DIR/assoc-const-missing-type.rs:11:15 + --> $DIR/assoc-const-missing-type.rs:12:15 | LL | const K = (); | ^ help: provide a type for the associated constant: `()` +error[E0195]: lifetime parameters or bounds on const `Q` do not match the trait declaration + --> $DIR/assoc-const-missing-type.rs:16:12 + | +LL | const Q<'a>: &'a str; + | ---- lifetimes in impl do not match this const in trait +... +LL | const Q = ""; + | ^ lifetimes do not match const in trait + +error: missing type for `const` item + --> $DIR/assoc-const-missing-type.rs:16:12 + | +LL | const Q = ""; + | ^ help: provide a type for the associated constant: `: &str` + error[E0308]: mismatched types - --> $DIR/assoc-const-missing-type.rs:11:18 + --> $DIR/assoc-const-missing-type.rs:12:18 | LL | const K = (); | - ^^ expected type parameter `T`, found `()` @@ -27,6 +42,7 @@ LL | const K = (); found unit type `()` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error: aborting due to 3 previous errors +error: aborting due to 5 previous errors -For more information about this error, try `rustc --explain E0308`. +Some errors have detailed explanations: E0195, E0308. +For more information about an error, try `rustc --explain E0195`. From a02aba7c542bf65f53c3a631f56110d8d95afc1c Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 23 May 2024 19:22:55 -0400 Subject: [PATCH 11/27] Only suppress binop error in favor of semicolon suggestion if we're in an assignment statement --- compiler/rustc_hir_typeck/src/op.rs | 5 +++- .../binop/nested-assignment-may-be-deref.rs | 14 +++++++++ .../nested-assignment-may-be-deref.stderr | 29 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/ui/binop/nested-assignment-may-be-deref.rs create mode 100644 tests/ui/binop/nested-assignment-may-be-deref.stderr diff --git a/compiler/rustc_hir_typeck/src/op.rs b/compiler/rustc_hir_typeck/src/op.rs index 87b76b978b937..25b74dca12fc5 100644 --- a/compiler/rustc_hir_typeck/src/op.rs +++ b/compiler/rustc_hir_typeck/src/op.rs @@ -381,10 +381,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let maybe_missing_semi = self.check_for_missing_semi(expr, &mut err); // We defer to the later error produced by `check_lhs_assignable`. - // We only downgrade this if it's the LHS, though. + // We only downgrade this if it's the LHS, though, and if this is a + // valid assignment statement. if maybe_missing_semi && let hir::Node::Expr(parent) = self.tcx.parent_hir_node(expr.hir_id) && let hir::ExprKind::Assign(lhs, _, _) = parent.kind + && let hir::Node::Stmt(stmt) = self.tcx.parent_hir_node(parent.hir_id) + && let hir::StmtKind::Expr(_) | hir::StmtKind::Semi(_) = stmt.kind && lhs.hir_id == expr.hir_id { err.downgrade_to_delayed_bug(); diff --git a/tests/ui/binop/nested-assignment-may-be-deref.rs b/tests/ui/binop/nested-assignment-may-be-deref.rs new file mode 100644 index 0000000000000..f675ab2e9183c --- /dev/null +++ b/tests/ui/binop/nested-assignment-may-be-deref.rs @@ -0,0 +1,14 @@ +pub fn bad(x: &mut bool) { + if true + *x = true {} + //~^ ERROR cannot multiply `bool` by `&mut bool` +} + +pub fn bad2(x: &mut bool) { + let y: bool; + y = true + *x = true; + //~^ ERROR cannot multiply `bool` by `&mut bool` +} + +fn main() {} diff --git a/tests/ui/binop/nested-assignment-may-be-deref.stderr b/tests/ui/binop/nested-assignment-may-be-deref.stderr new file mode 100644 index 0000000000000..95b2db2b26c8f --- /dev/null +++ b/tests/ui/binop/nested-assignment-may-be-deref.stderr @@ -0,0 +1,29 @@ +error[E0369]: cannot multiply `bool` by `&mut bool` + --> $DIR/nested-assignment-may-be-deref.rs:3:5 + | +LL | if true + | ---- bool +LL | *x = true {} + | ^- &mut bool + | +help: you might have meant to write a semicolon here + | +LL | if true; + | + + +error[E0369]: cannot multiply `bool` by `&mut bool` + --> $DIR/nested-assignment-may-be-deref.rs:10:5 + | +LL | y = true + | ---- bool +LL | *x = true; + | ^- &mut bool + | +help: you might have meant to write a semicolon here + | +LL | y = true; + | + + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0369`. From c1ac4a2f2840e312af84d453751861f400fd6798 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 24 May 2024 15:08:18 +1000 Subject: [PATCH 12/27] Run rustfmt on files that need it. Somehow these files aren't properly formatted. By default `x fmt` and `x tidy` only check files that have changed against master, so if an ill-formatted file somehow slips in it can stay that way as long as it doesn't get modified(?) I found these when I ran `x fmt` explicitly on every `.rs` file in the repo, while working on https://github.com/rust-lang/compiler-team/issues/750. --- compiler/rustc_codegen_ssa/src/base.rs | 2 +- .../rustc_const_eval/src/const_eval/error.rs | 7 ++--- .../rustc_incremental/src/persist/load.rs | 3 +- .../src/for_loops_over_fallibles.rs | 4 ++- library/test/src/bench.rs | 2 +- src/bootstrap/src/core/builder/tests.rs | 29 +++++++++++++++---- src/bootstrap/src/core/sanity.rs | 10 +++++-- src/tools/build_helper/src/lib.rs | 2 +- 8 files changed, 41 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/base.rs b/compiler/rustc_codegen_ssa/src/base.rs index b4556f8fcb8fc..5d7eb0525615f 100644 --- a/compiler/rustc_codegen_ssa/src/base.rs +++ b/compiler/rustc_codegen_ssa/src/base.rs @@ -29,8 +29,8 @@ use rustc_middle::middle::debugger_visualizer::{DebuggerVisualizerFile, Debugger use rustc_middle::middle::exported_symbols; use rustc_middle::middle::exported_symbols::SymbolExportKind; use rustc_middle::middle::lang_items; -use rustc_middle::mir::BinOp; use rustc_middle::mir::mono::{CodegenUnit, CodegenUnitNameBuilder, MonoItem}; +use rustc_middle::mir::BinOp; use rustc_middle::query::Providers; use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, TyAndLayout}; use rustc_middle::ty::{self, Instance, Ty, TyCtxt}; diff --git a/compiler/rustc_const_eval/src/const_eval/error.rs b/compiler/rustc_const_eval/src/const_eval/error.rs index 7a1c2a7b6fa51..650669ac690b3 100644 --- a/compiler/rustc_const_eval/src/const_eval/error.rs +++ b/compiler/rustc_const_eval/src/const_eval/error.rs @@ -139,10 +139,9 @@ where ErrorHandled::TooGeneric(span) } err_inval!(AlreadyReported(guar)) => ErrorHandled::Reported(guar, span), - err_inval!(Layout(LayoutError::ReferencesError(guar))) => ErrorHandled::Reported( - ReportedErrorInfo::tainted_by_errors(guar), - span, - ), + err_inval!(Layout(LayoutError::ReferencesError(guar))) => { + ErrorHandled::Reported(ReportedErrorInfo::tainted_by_errors(guar), span) + } // Report remaining errors. _ => { let (our_span, frames) = get_span_and_frames(); diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index 6c3f73cf46594..af667a57ce123 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -116,8 +116,7 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkPr if let LoadResult::Ok { data: (work_products_data, start_pos) } = load_result { // Decode the list of work_products - let Ok(mut work_product_decoder) = - MemDecoder::new(&work_products_data[..], start_pos) + let Ok(mut work_product_decoder) = MemDecoder::new(&work_products_data[..], start_pos) else { sess.dcx().emit_warn(errors::CorruptFile { path: &work_products_path }); return LoadResult::DataOutOfDate; diff --git a/compiler/rustc_lint/src/for_loops_over_fallibles.rs b/compiler/rustc_lint/src/for_loops_over_fallibles.rs index b05f5e7638b4e..aa00fb4573d80 100644 --- a/compiler/rustc_lint/src/for_loops_over_fallibles.rs +++ b/compiler/rustc_lint/src/for_loops_over_fallibles.rs @@ -62,7 +62,9 @@ impl<'tcx> LateLintPass<'tcx> for ForLoopsOverFallibles { }; let (article, ty, var) = match adt.did() { - did if cx.tcx.is_diagnostic_item(sym::Option, did) && ref_mutability.is_some() => ("a", "Option", "Some"), + did if cx.tcx.is_diagnostic_item(sym::Option, did) && ref_mutability.is_some() => { + ("a", "Option", "Some") + } did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"), did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"), _ => return, diff --git a/library/test/src/bench.rs b/library/test/src/bench.rs index 9a5dc351f6d07..64ca13c0d4ed3 100644 --- a/library/test/src/bench.rs +++ b/library/test/src/bench.rs @@ -98,7 +98,7 @@ fn fmt_thousands_sep(mut n: f64, sep: char) -> String { (0, true) => write!(output, "{:06.2}", n / base as f64).unwrap(), (0, false) => write!(output, "{:.2}", n / base as f64).unwrap(), (_, true) => write!(output, "{:03}", n as usize / base).unwrap(), - _ => write!(output, "{}", n as usize / base).unwrap() + _ => write!(output, "{}", n as usize / base).unwrap(), } if pow != 0 { output.push(sep); diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 9710365ef114d..276fd0b11d64c 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -60,7 +60,14 @@ fn check_cli(paths: [&str; N]) { macro_rules! std { ($host:ident => $target:ident, stage = $stage:literal) => { compile::Std::new( - Compiler { host: TargetSelection::from_user(concat!(stringify!($host), "-", stringify!($host))), stage: $stage }, + Compiler { + host: TargetSelection::from_user(concat!( + stringify!($host), + "-", + stringify!($host) + )), + stage: $stage, + }, TargetSelection::from_user(concat!(stringify!($target), "-", stringify!($target))), ) }; @@ -83,7 +90,14 @@ macro_rules! doc_std { macro_rules! rustc { ($host:ident => $target:ident, stage = $stage:literal) => { compile::Rustc::new( - Compiler { host: TargetSelection::from_user(concat!(stringify!($host), "-", stringify!($host))), stage: $stage }, + Compiler { + host: TargetSelection::from_user(concat!( + stringify!($host), + "-", + stringify!($host) + )), + stage: $stage, + }, TargetSelection::from_user(concat!(stringify!($target), "-", stringify!($target))), ) }; @@ -141,10 +155,14 @@ fn check_missing_paths_for_x_test_tests() { // Skip if not a test directory. if path.ends_with("tests/auxiliary") || !path.is_dir() { - continue + continue; } - assert!(tests_remap_paths.iter().any(|item| path.ends_with(*item)), "{} is missing in PATH_REMAP tests list.", path.display()); + assert!( + tests_remap_paths.iter().any(|item| path.ends_with(*item)), + "{} is missing in PATH_REMAP tests list.", + path.display() + ); } } @@ -185,7 +203,8 @@ fn alias_and_path_for_library() { &[std!(A => A, stage = 0), std!(A => A, stage = 1)] ); - let mut cache = run_build(&["library".into(), "core".into()], configure("doc", &["A-A"], &["A-A"])); + let mut cache = + run_build(&["library".into(), "core".into()], configure("doc", &["A-A"], &["A-A"])); assert_eq!(first(cache.all::()), &[doc_std!(A => A, stage = 0)]); } diff --git a/src/bootstrap/src/core/sanity.rs b/src/bootstrap/src/core/sanity.rs index 9c3df6fa9e39b..8ffa97ab78b55 100644 --- a/src/bootstrap/src/core/sanity.rs +++ b/src/bootstrap/src/core/sanity.rs @@ -199,11 +199,15 @@ than building it. if !["A-A", "B-B", "C-C"].contains(&target_str.as_str()) { let mut has_target = false; - let missing_targets_hashset: HashSet<_> = STAGE0_MISSING_TARGETS.iter().map(|t| t.to_string()).collect(); - let duplicated_targets: Vec<_> = stage0_supported_target_list.intersection(&missing_targets_hashset).collect(); + let missing_targets_hashset: HashSet<_> = + STAGE0_MISSING_TARGETS.iter().map(|t| t.to_string()).collect(); + let duplicated_targets: Vec<_> = + stage0_supported_target_list.intersection(&missing_targets_hashset).collect(); if !duplicated_targets.is_empty() { - println!("Following targets supported from the stage0 compiler, please remove them from STAGE0_MISSING_TARGETS list."); + println!( + "Following targets supported from the stage0 compiler, please remove them from STAGE0_MISSING_TARGETS list." + ); for duplicated_target in duplicated_targets { println!(" {duplicated_target}"); } diff --git a/src/tools/build_helper/src/lib.rs b/src/tools/build_helper/src/lib.rs index 6a4e86eb1dfe5..2abda5d3ebf27 100644 --- a/src/tools/build_helper/src/lib.rs +++ b/src/tools/build_helper/src/lib.rs @@ -1,5 +1,5 @@ pub mod ci; pub mod git; pub mod metrics; -pub mod util; pub mod stage0_parser; +pub mod util; From d83f3ca8ca2d20eadf92a135a1a4b65ca91a24f6 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Thu, 23 May 2024 23:17:58 -0700 Subject: [PATCH 13/27] Validate the special layout restriction on DynMetadata --- .../src/transform/validate.rs | 9 +++++++ library/core/src/ptr/metadata.rs | 27 +++++++++++++------ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 3a2b2c5f3002d..66cc65de64709 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -685,6 +685,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { check_equal(self, location, *f_ty); } ty::Adt(adt_def, args) => { + // see + if Some(adt_def.did()) == self.tcx.lang_items().dyn_metadata() { + self.fail( + location, + format!("You can't project to field {f:?} of `DynMetadata` because \ + layout is weird and thinks it doesn't have fields."), + ); + } + let var = parent_ty.variant_index.unwrap_or(FIRST_VARIANT); let Some(field) = adt_def.variant(var).fields.get(f) else { fail_out_of_bounds(self, location); diff --git a/library/core/src/ptr/metadata.rs b/library/core/src/ptr/metadata.rs index eb815b6d822c5..e501970b580de 100644 --- a/library/core/src/ptr/metadata.rs +++ b/library/core/src/ptr/metadata.rs @@ -178,8 +178,8 @@ impl Clone for PtrComponents { /// compare equal (since identical vtables can be deduplicated within a codegen unit). #[lang = "dyn_metadata"] pub struct DynMetadata { - vtable_ptr: &'static VTable, - phantom: crate::marker::PhantomData, + _vtable_ptr: &'static VTable, + _phantom: crate::marker::PhantomData, } extern "C" { @@ -191,6 +191,17 @@ extern "C" { } impl DynMetadata { + /// One of the things that rustc_middle does with this being a lang item is + /// give it `FieldsShape::Primitive`, which means that as far as codegen can + /// tell, it *is* a reference, and thus doesn't have any fields. + /// That means we can't use field access, and have to transmute it instead. + #[inline] + fn vtable_ptr(self) -> *const VTable { + // SAFETY: this layout assumption is hard-coded into the compiler. + // If it's somehow not a size match, the transmute will error. + unsafe { crate::mem::transmute::(self) } + } + /// Returns the size of the type associated with this vtable. #[inline] pub fn size_of(self) -> usize { @@ -199,7 +210,7 @@ impl DynMetadata { // `Send` part! // SAFETY: DynMetadata always contains a valid vtable pointer return unsafe { - crate::intrinsics::vtable_size(self.vtable_ptr as *const VTable as *const ()) + crate::intrinsics::vtable_size(self.vtable_ptr() as *const ()) }; } @@ -208,7 +219,7 @@ impl DynMetadata { pub fn align_of(self) -> usize { // SAFETY: DynMetadata always contains a valid vtable pointer return unsafe { - crate::intrinsics::vtable_align(self.vtable_ptr as *const VTable as *const ()) + crate::intrinsics::vtable_align(self.vtable_ptr() as *const ()) }; } @@ -226,7 +237,7 @@ unsafe impl Sync for DynMetadata {} impl fmt::Debug for DynMetadata { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_tuple("DynMetadata").field(&(self.vtable_ptr as *const VTable)).finish() + f.debug_tuple("DynMetadata").field(&self.vtable_ptr()).finish() } } @@ -248,7 +259,7 @@ impl Eq for DynMetadata {} impl PartialEq for DynMetadata { #[inline] fn eq(&self, other: &Self) -> bool { - crate::ptr::eq::(self.vtable_ptr, other.vtable_ptr) + crate::ptr::eq::(self.vtable_ptr(), other.vtable_ptr()) } } @@ -256,7 +267,7 @@ impl Ord for DynMetadata { #[inline] #[allow(ambiguous_wide_pointer_comparisons)] fn cmp(&self, other: &Self) -> crate::cmp::Ordering { - (self.vtable_ptr as *const VTable).cmp(&(other.vtable_ptr as *const VTable)) + <*const VTable>::cmp(&self.vtable_ptr(), &other.vtable_ptr()) } } @@ -270,6 +281,6 @@ impl PartialOrd for DynMetadata { impl Hash for DynMetadata { #[inline] fn hash(&self, hasher: &mut H) { - crate::ptr::hash::(self.vtable_ptr, hasher) + crate::ptr::hash::(self.vtable_ptr(), hasher) } } From e78671e61fa14cd541a1f6ae357a0d1f070e2cd1 Mon Sep 17 00:00:00 2001 From: Xinzhao Xu Date: Fri, 24 May 2024 15:43:40 +0800 Subject: [PATCH 14/27] Fix the dead link in the bootstrap README --- src/bootstrap/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/README.md b/src/bootstrap/README.md index 077db44ae54ee..fb3c862704306 100644 --- a/src/bootstrap/README.md +++ b/src/bootstrap/README.md @@ -6,7 +6,7 @@ and some of the technical details of the build system. Note that this README only covers internal information, not how to use the tool. Please check [bootstrapping dev guide][bootstrapping-dev-guide] for further information. -[bootstrapping-dev-guide]: https://rustc-dev-guide.rust-lang.org/building/bootstrapping.html +[bootstrapping-dev-guide]: https://rustc-dev-guide.rust-lang.org/building/bootstrapping/intro.html ## Introduction From daff84372d67fe39e13b35fdcd3f6f4267a0063a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 24 May 2024 11:19:30 +0200 Subject: [PATCH 15/27] Migrate `run-make/rustdoc-with-output-dir-option` to `rmake.rs` --- .../tidy/src/allowed_run_make_makefiles.txt | 1 - .../run-make/rustdoc-with-output-option/Makefile | 8 -------- .../run-make/rustdoc-with-output-option/rmake.rs | 16 ++++++++++++++++ 3 files changed, 16 insertions(+), 9 deletions(-) delete mode 100644 tests/run-make/rustdoc-with-output-option/Makefile create mode 100644 tests/run-make/rustdoc-with-output-option/rmake.rs diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index ad900ce038618..9a6ae18abeade 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -228,7 +228,6 @@ run-make/rmeta-preferred/Makefile run-make/rustc-macro-dep-files/Makefile run-make/rustdoc-io-error/Makefile run-make/rustdoc-verify-output-files/Makefile -run-make/rustdoc-with-output-option/Makefile run-make/sanitizer-cdylib-link/Makefile run-make/sanitizer-dylib-link/Makefile run-make/sanitizer-staticlib-link/Makefile diff --git a/tests/run-make/rustdoc-with-output-option/Makefile b/tests/run-make/rustdoc-with-output-option/Makefile deleted file mode 100644 index d0a8205a8ee55..0000000000000 --- a/tests/run-make/rustdoc-with-output-option/Makefile +++ /dev/null @@ -1,8 +0,0 @@ -include ../tools.mk - -OUTPUT_DIR := "$(TMPDIR)/rustdoc" - -all: - $(RUSTDOC) src/lib.rs --crate-name foobar --crate-type lib --output $(OUTPUT_DIR) - - $(HTMLDOCCK) $(OUTPUT_DIR) src/lib.rs diff --git a/tests/run-make/rustdoc-with-output-option/rmake.rs b/tests/run-make/rustdoc-with-output-option/rmake.rs new file mode 100644 index 0000000000000..1a009419273e1 --- /dev/null +++ b/tests/run-make/rustdoc-with-output-option/rmake.rs @@ -0,0 +1,16 @@ +use run_make_support::{htmldocck, rustdoc, tmp_dir}; + +fn main() { + let out_dir = tmp_dir().join("rustdoc"); + + rustdoc() + .input("src/lib.rs") + .crate_name("foobar") + .crate_type("lib") + // This is intentionally using `--output` option flag and not the `output()` method. + .arg("--output") + .arg(&out_dir) + .run(); + + assert!(htmldocck().arg(out_dir).arg("src/lib.rs").status().unwrap().success()); +} From 5f0531da05114b473cb04edce0d4374d516b9125 Mon Sep 17 00:00:00 2001 From: joboet Date: Thu, 14 Mar 2024 14:25:09 +0100 Subject: [PATCH 16/27] std: simplify key-based thread locals --- library/std/src/sys/thread_local/mod.rs | 83 ---------- library/std/src/sys/thread_local/os_local.rs | 162 +++++++------------ 2 files changed, 60 insertions(+), 185 deletions(-) diff --git a/library/std/src/sys/thread_local/mod.rs b/library/std/src/sys/thread_local/mod.rs index 36f6f907e7213..1acf1f8d8c16f 100644 --- a/library/std/src/sys/thread_local/mod.rs +++ b/library/std/src/sys/thread_local/mod.rs @@ -24,89 +24,6 @@ cfg_if::cfg_if! { } } -// Not used by the fast-local TLS anymore. -// FIXME(#110897): remove this. -#[allow(unused)] -mod lazy { - use crate::cell::UnsafeCell; - use crate::hint; - use crate::mem; - - pub struct LazyKeyInner { - inner: UnsafeCell>, - } - - impl LazyKeyInner { - pub const fn new() -> LazyKeyInner { - LazyKeyInner { inner: UnsafeCell::new(None) } - } - - pub unsafe fn get(&self) -> Option<&'static T> { - // SAFETY: The caller must ensure no reference is ever handed out to - // the inner cell nor mutable reference to the Option inside said - // cell. This make it safe to hand a reference, though the lifetime - // of 'static is itself unsafe, making the get method unsafe. - unsafe { (*self.inner.get()).as_ref() } - } - - /// The caller must ensure that no reference is active: this method - /// needs unique access. - pub unsafe fn initialize T>(&self, init: F) -> &'static T { - // Execute the initialization up front, *then* move it into our slot, - // just in case initialization fails. - let value = init(); - let ptr = self.inner.get(); - - // SAFETY: - // - // note that this can in theory just be `*ptr = Some(value)`, but due to - // the compiler will currently codegen that pattern with something like: - // - // ptr::drop_in_place(ptr) - // ptr::write(ptr, Some(value)) - // - // Due to this pattern it's possible for the destructor of the value in - // `ptr` (e.g., if this is being recursively initialized) to re-access - // TLS, in which case there will be a `&` and `&mut` pointer to the same - // value (an aliasing violation). To avoid setting the "I'm running a - // destructor" flag we just use `mem::replace` which should sequence the - // operations a little differently and make this safe to call. - // - // The precondition also ensures that we are the only one accessing - // `self` at the moment so replacing is fine. - unsafe { - let _ = mem::replace(&mut *ptr, Some(value)); - } - - // SAFETY: With the call to `mem::replace` it is guaranteed there is - // a `Some` behind `ptr`, not a `None` so `unreachable_unchecked` - // will never be reached. - unsafe { - // After storing `Some` we want to get a reference to the contents of - // what we just stored. While we could use `unwrap` here and it should - // always work it empirically doesn't seem to always get optimized away, - // which means that using something like `try_with` can pull in - // panicking code and cause a large size bloat. - match *ptr { - Some(ref x) => x, - None => hint::unreachable_unchecked(), - } - } - } - - /// Watch out: unsynchronized internal mutability! - /// - /// # Safety - /// Causes UB if any reference to the value is used after this. - #[allow(unused)] - pub(crate) unsafe fn take(&self) -> Option { - let mutable: *mut _ = UnsafeCell::get(&self.inner); - // SAFETY: That's the caller's problem. - unsafe { mutable.replace(None) } - } - } -} - /// Run a callback in a scenario which must not unwind (such as a `extern "C" /// fn` declared in a user crate). If the callback unwinds anyway, then /// `rtabort` with a message about thread local panicking on drop. diff --git a/library/std/src/sys/thread_local/os_local.rs b/library/std/src/sys/thread_local/os_local.rs index 3edffd7e4437c..6b24fe254a15e 100644 --- a/library/std/src/sys/thread_local/os_local.rs +++ b/library/std/src/sys/thread_local/os_local.rs @@ -1,7 +1,8 @@ -use super::lazy::LazyKeyInner; +use super::abort_on_dtor_unwind; use crate::cell::Cell; -use crate::sys_common::thread_local_key::StaticKey as OsStaticKey; -use crate::{fmt, marker, panic, ptr}; +use crate::marker::PhantomData; +use crate::sys_common::thread_local_key::StaticKey as OsKey; +use crate::{panic, ptr}; #[doc(hidden)] #[allow_internal_unstable(thread_local_internals)] @@ -10,38 +11,9 @@ use crate::{fmt, marker, panic, ptr}; #[rustc_macro_transparency = "semitransparent"] pub macro thread_local_inner { // used to generate the `LocalKey` value for const-initialized thread locals - (@key $t:ty, const $init:expr) => {{ - #[inline] - #[deny(unsafe_op_in_unsafe_fn)] - unsafe fn __getit( - _init: $crate::option::Option<&mut $crate::option::Option<$t>>, - ) -> $crate::option::Option<&'static $t> { - const INIT_EXPR: $t = $init; - - // On platforms without `#[thread_local]` we fall back to the - // same implementation as below for os thread locals. - #[inline] - const fn __init() -> $t { INIT_EXPR } - static __KEY: $crate::thread::local_impl::Key<$t> = - $crate::thread::local_impl::Key::new(); - unsafe { - __KEY.get(move || { - if let $crate::option::Option::Some(init) = _init { - if let $crate::option::Option::Some(value) = init.take() { - return value; - } else if $crate::cfg!(debug_assertions) { - $crate::unreachable!("missing initial value"); - } - } - __init() - }) - } - } - - unsafe { - $crate::thread::LocalKey::new(__getit) - } - }}, + (@key $t:ty, const $init:expr) => { + $crate::thread::local_impl::thread_local_inner!(@key $t, { const INIT_EXPR: $t = $init; INIT_EXPR }) + }, // used to generate the `LocalKey` value for `thread_local!` (@key $t:ty, $init:expr) => { @@ -55,20 +27,11 @@ pub macro thread_local_inner { unsafe fn __getit( init: $crate::option::Option<&mut $crate::option::Option<$t>>, ) -> $crate::option::Option<&'static $t> { - static __KEY: $crate::thread::local_impl::Key<$t> = - $crate::thread::local_impl::Key::new(); + use $crate::thread::local_impl::Key; + static __KEY: Key<$t> = Key::new(); unsafe { - __KEY.get(move || { - if let $crate::option::Option::Some(init) = init { - if let $crate::option::Option::Some(value) = init.take() { - return value; - } else if $crate::cfg!(debug_assertions) { - $crate::unreachable!("missing default value"); - } - } - __init() - }) + __KEY.get(init, __init) } } @@ -85,78 +48,78 @@ pub macro thread_local_inner { /// Use a regular global static to store this key; the state provided will then be /// thread-local. +#[allow(missing_debug_implementations)] pub struct Key { - // OS-TLS key that we'll use to key off. - os: OsStaticKey, - marker: marker::PhantomData>, -} - -impl fmt::Debug for Key { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Key").finish_non_exhaustive() - } + os: OsKey, + marker: PhantomData>, } unsafe impl Sync for Key {} struct Value { - inner: LazyKeyInner, + value: T, key: &'static Key, } impl Key { #[rustc_const_unstable(feature = "thread_local_internals", issue = "none")] pub const fn new() -> Key { - Key { os: OsStaticKey::new(Some(destroy_value::)), marker: marker::PhantomData } + Key { os: OsKey::new(Some(destroy_value::)), marker: PhantomData } } - /// It is a requirement for the caller to ensure that no mutable - /// reference is active when this method is called. - pub unsafe fn get(&'static self, init: impl FnOnce() -> T) -> Option<&'static T> { - // SAFETY: See the documentation for this method. + /// Get the value associated with this key, initializating it if necessary. + /// + /// # Safety + /// * the returned reference must not be used after recursive initialization + /// or thread destruction occurs. + pub unsafe fn get( + &'static self, + i: Option<&mut Option>, + f: impl FnOnce() -> T, + ) -> Option<&'static T> { + // SAFETY: (FIXME: get should actually be safe) let ptr = unsafe { self.os.get() as *mut Value }; if ptr.addr() > 1 { // SAFETY: the check ensured the pointer is safe (its destructor // is not running) + it is coming from a trusted source (self). - if let Some(ref value) = unsafe { (*ptr).inner.get() } { - return Some(value); - } + unsafe { Some(&(*ptr).value) } + } else { + // SAFETY: At this point we are sure we have no value and so + // initializing (or trying to) is safe. + unsafe { self.try_initialize(ptr, i, f) } } - // SAFETY: At this point we are sure we have no value and so - // initializing (or trying to) is safe. - unsafe { self.try_initialize(init) } } - // `try_initialize` is only called once per os thread local variable, - // except in corner cases where thread_local dtors reference other - // thread_local's, or it is being recursively initialized. - unsafe fn try_initialize(&'static self, init: impl FnOnce() -> T) -> Option<&'static T> { - // SAFETY: No mutable references are ever handed out meaning getting - // the value is ok. - let ptr = unsafe { self.os.get() as *mut Value }; + unsafe fn try_initialize( + &'static self, + ptr: *mut Value, + i: Option<&mut Option>, + f: impl FnOnce() -> T, + ) -> Option<&'static T> { if ptr.addr() == 1 { // destructor is running return None; } - let ptr = if ptr.is_null() { - // If the lookup returned null, we haven't initialized our own - // local copy, so do that now. - let ptr = Box::into_raw(Box::new(Value { inner: LazyKeyInner::new(), key: self })); - // SAFETY: At this point we are sure there is no value inside - // ptr so setting it will not affect anyone else. - unsafe { - self.os.set(ptr as *mut u8); - } - ptr - } else { - // recursive initialization - ptr - }; + let value = i.and_then(Option::take).unwrap_or_else(f); + let ptr = Box::into_raw(Box::new(Value { value, key: self })); + // SAFETY: (FIXME: get should actually be safe) + let old = unsafe { self.os.get() as *mut Value }; + // SAFETY: `ptr` is a correct pointer that can be destroyed by the key destructor. + unsafe { + self.os.set(ptr as *mut u8); + } + if !old.is_null() { + // If the variable was recursively initialized, drop the old value. + // SAFETY: We cannot be inside a `LocalKey::with` scope, as the + // initializer has already returned and the next scope only starts + // after we return the pointer. Therefore, there can be no references + // to the old value. + drop(unsafe { Box::from_raw(old) }); + } - // SAFETY: ptr has been ensured as non-NUL just above an so can be - // dereferenced safely. - unsafe { Some((*ptr).inner.initialize(init)) } + // SAFETY: We just created this value above. + unsafe { Some(&(*ptr).value) } } } @@ -170,16 +133,11 @@ unsafe extern "C" fn destroy_value(ptr: *mut u8) { // // Note that to prevent an infinite loop we reset it back to null right // before we return from the destructor ourselves. - // - // Wrap the call in a catch to ensure unwinding is caught in the event - // a panic takes place in a destructor. - if let Err(_) = panic::catch_unwind(|| unsafe { - let ptr = Box::from_raw(ptr as *mut Value); + abort_on_dtor_unwind(|| { + let ptr = unsafe { Box::from_raw(ptr as *mut Value) }; let key = ptr.key; - key.os.set(ptr::without_provenance_mut(1)); + unsafe { key.os.set(ptr::without_provenance_mut(1)) }; drop(ptr); - key.os.set(ptr::null_mut()); - }) { - rtabort!("thread local panicked on drop"); - } + unsafe { key.os.set(ptr::null_mut()) }; + }); } From 0e7e75ebca2ee9cc9a6c019110797250094908f5 Mon Sep 17 00:00:00 2001 From: joboet Date: Fri, 24 May 2024 11:46:09 +0200 Subject: [PATCH 17/27] std: clean up the TLS implementation --- library/std/src/sys/mod.rs | 2 -- library/std/src/sys/thread_local/fast_local/lazy.rs | 1 - library/std/src/sys/thread_local/mod.rs | 4 +++- library/std/src/sys/thread_local/os_local.rs | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/mod.rs b/library/std/src/sys/mod.rs index bbd1d840e92dc..8f70cefc60121 100644 --- a/library/std/src/sys/mod.rs +++ b/library/std/src/sys/mod.rs @@ -9,8 +9,6 @@ pub mod cmath; pub mod os_str; pub mod path; pub mod sync; -#[allow(dead_code)] -#[allow(unused_imports)] pub mod thread_local; // FIXME(117276): remove this, move feature implementations into individual diff --git a/library/std/src/sys/thread_local/fast_local/lazy.rs b/library/std/src/sys/thread_local/fast_local/lazy.rs index 14371768d30ce..c2e9a17145468 100644 --- a/library/std/src/sys/thread_local/fast_local/lazy.rs +++ b/library/std/src/sys/thread_local/fast_local/lazy.rs @@ -1,6 +1,5 @@ use crate::cell::UnsafeCell; use crate::hint::unreachable_unchecked; -use crate::mem::forget; use crate::ptr; use crate::sys::thread_local::abort_on_dtor_unwind; use crate::sys::thread_local_dtor::register_dtor; diff --git a/library/std/src/sys/thread_local/mod.rs b/library/std/src/sys/thread_local/mod.rs index 1acf1f8d8c16f..0a78a1a1cf02d 100644 --- a/library/std/src/sys/thread_local/mod.rs +++ b/library/std/src/sys/thread_local/mod.rs @@ -1,4 +1,5 @@ #![unstable(feature = "thread_local_internals", reason = "should not be necessary", issue = "none")] +#![cfg_attr(test, allow(unused))] // There are three thread-local implementations: "static", "fast", "OS". // The "OS" thread local key type is accessed via platform-specific API calls and is slow, while the @@ -28,7 +29,8 @@ cfg_if::cfg_if! { /// fn` declared in a user crate). If the callback unwinds anyway, then /// `rtabort` with a message about thread local panicking on drop. #[inline] -pub fn abort_on_dtor_unwind(f: impl FnOnce()) { +#[allow(dead_code)] +fn abort_on_dtor_unwind(f: impl FnOnce()) { // Using a guard like this is lower cost. let guard = DtorUnwindGuard; f(); diff --git a/library/std/src/sys/thread_local/os_local.rs b/library/std/src/sys/thread_local/os_local.rs index 6b24fe254a15e..d6ddbb78a9c86 100644 --- a/library/std/src/sys/thread_local/os_local.rs +++ b/library/std/src/sys/thread_local/os_local.rs @@ -1,8 +1,8 @@ use super::abort_on_dtor_unwind; use crate::cell::Cell; use crate::marker::PhantomData; +use crate::ptr; use crate::sys_common::thread_local_key::StaticKey as OsKey; -use crate::{panic, ptr}; #[doc(hidden)] #[allow_internal_unstable(thread_local_internals)] From 56c135c9253563f92755b0a9cd54ec67f7c17fc7 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 24 May 2024 13:21:59 +0000 Subject: [PATCH 18/27] Revert "Rollup merge of #123979 - oli-obk:define_opaque_types7, r=compiler-errors" This reverts commit f939d1ff4820844595d0f50f94038869f1445d08, reversing changes made to 183c706305d8c4e0ccb0967932381baf7e0c3611. --- compiler/rustc_infer/src/infer/mod.rs | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 8d4011421bd33..6f603d9b612eb 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -957,27 +957,14 @@ impl<'tcx> InferCtxt<'tcx> { (&ty::Infer(ty::TyVar(a_vid)), &ty::Infer(ty::TyVar(b_vid))) => { return Err((a_vid, b_vid)); } - // We don't silently want to constrain hidden types here, so we assert that either one side is - // an infer var, so it'll get constrained to whatever the other side is, or there are no opaque - // types involved. - // We don't expect this to actually get hit, but if it does, we now at least know how to write - // a test for it. - (_, ty::Infer(ty::TyVar(_))) => {} - (ty::Infer(ty::TyVar(_)), _) => {} - _ if r_a != r_b && (r_a, r_b).has_opaque_types() => { - span_bug!( - cause.span(), - "opaque types got hidden types registered from within subtype predicate: {r_a:?} vs {r_b:?}" - ) - } _ => {} } self.enter_forall(predicate, |ty::SubtypePredicate { a_is_expected, a, b }| { if a_is_expected { - Ok(self.at(cause, param_env).sub(DefineOpaqueTypes::Yes, a, b)) + Ok(self.at(cause, param_env).sub(DefineOpaqueTypes::No, a, b)) } else { - Ok(self.at(cause, param_env).sup(DefineOpaqueTypes::Yes, b, a)) + Ok(self.at(cause, param_env).sup(DefineOpaqueTypes::No, b, a)) } }) } From 526090b901cbedcef7e1eec132e217917606a54d Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 23 May 2024 10:03:55 +0000 Subject: [PATCH 19/27] Add regression tests --- tests/crashes/124891.rs | 22 ------- .../impl-trait/lazy_subtyping_of_opaques.rs | 59 +++++++++++++++++++ .../lazy_subtyping_of_opaques.stderr | 21 +++++++ .../lazy_subtyping_of_opaques.rs | 30 ++++++++++ .../lazy_subtyping_of_opaques.stderr | 26 ++++++++ 5 files changed, 136 insertions(+), 22 deletions(-) delete mode 100644 tests/crashes/124891.rs create mode 100644 tests/ui/impl-trait/lazy_subtyping_of_opaques.rs create mode 100644 tests/ui/impl-trait/lazy_subtyping_of_opaques.stderr create mode 100644 tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.rs create mode 100644 tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.stderr diff --git a/tests/crashes/124891.rs b/tests/crashes/124891.rs deleted file mode 100644 index 9b5892418c897..0000000000000 --- a/tests/crashes/124891.rs +++ /dev/null @@ -1,22 +0,0 @@ -//@ known-bug: rust-lang/rust#124891 - -type Tait = impl FnOnce() -> (); - -fn reify_as_tait() -> Thunk { - Thunk::new(|cont| cont) -} - -struct Thunk(F); - -impl Thunk { - fn new(f: F) - where - F: ContFn, - { - todo!(); - } -} - -trait ContFn {} - -impl ()> ContFn for F {} diff --git a/tests/ui/impl-trait/lazy_subtyping_of_opaques.rs b/tests/ui/impl-trait/lazy_subtyping_of_opaques.rs new file mode 100644 index 0000000000000..65331894725a8 --- /dev/null +++ b/tests/ui/impl-trait/lazy_subtyping_of_opaques.rs @@ -0,0 +1,59 @@ +//! This test checks that we allow subtyping predicates that contain opaque types. +//! No hidden types are being constrained in the subtyping predicate, but type and +//! lifetime variables get subtyped in the generic parameter list of the opaque. + +use std::iter; + +mod either { + pub enum Either { + Left(L), + Right(R), + } + + impl> Iterator for Either { + type Item = L::Item; + fn next(&mut self) -> Option { + todo!() + } + } + pub use self::Either::{Left, Right}; +} + +pub enum BabeConsensusLogRef<'a> { + NextEpochData(BabeNextEpochRef<'a>), + NextConfigData, +} + +impl<'a> BabeConsensusLogRef<'a> { + pub fn scale_encoding( + &self, + ) -> impl Iterator + Clone + 'a> + Clone + 'a { + //~^ ERROR is not satisfied + //~| ERROR is not satisfied + //~| ERROR is not satisfied + match self { + BabeConsensusLogRef::NextEpochData(digest) => either::Left(either::Left( + digest.scale_encoding().map(either::Left).map(either::Left), + )), + BabeConsensusLogRef::NextConfigData => either::Right( + // The Opaque type from ``scale_encoding` gets used opaquely here, while the `R` + // generic parameter of `Either` contains type variables that get subtyped and the + // opaque type contains lifetime variables that get subtyped. + iter::once(either::Right(either::Left([1]))) + .chain(std::iter::once([1]).map(either::Right).map(either::Right)), + ), + } + } +} + +pub struct BabeNextEpochRef<'a>(&'a ()); + +impl<'a> BabeNextEpochRef<'a> { + pub fn scale_encoding( + &self, + ) -> impl Iterator + Clone + 'a> + Clone + 'a { + std::iter::once([1]) + } +} + +fn main() {} diff --git a/tests/ui/impl-trait/lazy_subtyping_of_opaques.stderr b/tests/ui/impl-trait/lazy_subtyping_of_opaques.stderr new file mode 100644 index 0000000000000..2f8c957c2c7dc --- /dev/null +++ b/tests/ui/impl-trait/lazy_subtyping_of_opaques.stderr @@ -0,0 +1,21 @@ +error[E0277]: the trait bound `Either + Clone + '_> + Clone + '_, fn(impl AsRef<[u8]> + Clone + '_) -> Either + Clone + '_, _> {Either:: + Clone + '_, _>::Left}>, fn(Either + Clone + '_, _>) -> Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>> {Either:: + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>::Left}>, _>, std::iter::Chain + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>>, Map, fn([{integer}; 1]) -> Either<[{integer}; 1], [{integer}; 1]> {Either::<[{integer}; 1], [{integer}; 1]>::Right}>, fn(Either<[{integer}; 1], [{integer}; 1]>) -> Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>> {Either:: + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>::Right}>>>: Clone` is not satisfied + --> $DIR/lazy_subtyping_of_opaques.rs:30:10 + | +LL | ) -> impl Iterator + Clone + 'a> + Clone + 'a { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `Either + Clone + '_> + Clone + '_, fn(impl AsRef<[u8]> + Clone + '_) -> Either + Clone + '_, _> {Either:: + Clone + '_, _>::Left}>, fn(Either + Clone + '_, _>) -> Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>> {Either:: + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>::Left}>, _>, std::iter::Chain + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>>, Map, fn([{integer}; 1]) -> Either<[{integer}; 1], [{integer}; 1]> {Either::<[{integer}; 1], [{integer}; 1]>::Right}>, fn(Either<[{integer}; 1], [{integer}; 1]>) -> Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>> {Either:: + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>::Right}>>>` + +error[E0277]: the trait bound `Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>: AsRef<[u8]>` is not satisfied + --> $DIR/lazy_subtyping_of_opaques.rs:30:31 + | +LL | ) -> impl Iterator + Clone + 'a> + Clone + 'a { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `AsRef<[u8]>` is not implemented for `Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>` + +error[E0277]: the trait bound `Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>: Clone` is not satisfied + --> $DIR/lazy_subtyping_of_opaques.rs:30:31 + | +LL | ) -> impl Iterator + Clone + 'a> + Clone + 'a { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `Either + Clone + '_, _>, Either<[{integer}; 1], [{integer}; 1]>>` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.rs b/tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.rs new file mode 100644 index 0000000000000..72a90287e374f --- /dev/null +++ b/tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.rs @@ -0,0 +1,30 @@ +#![feature(type_alias_impl_trait)] + +//! This test used to ICE rust-lang/rust#124891 +//! because we added an assertion for catching cases where opaque types get +//! registered during the processing of subtyping predicates. + +type Tait = impl FnOnce() -> (); + +fn reify_as_tait() -> Thunk { + Thunk::new(|cont| cont) + //~^ ERROR: mismatched types + //~| ERROR: mismatched types +} + +struct Thunk(F); + +impl Thunk { + fn new(f: F) + where + F: ContFn, + { + todo!(); + } +} + +trait ContFn {} + +impl ()> ContFn for F {} + +fn main() {} diff --git a/tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.stderr b/tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.stderr new file mode 100644 index 0000000000000..5a35dc27446cb --- /dev/null +++ b/tests/ui/type-alias-impl-trait/lazy_subtyping_of_opaques.stderr @@ -0,0 +1,26 @@ +error[E0308]: mismatched types + --> $DIR/lazy_subtyping_of_opaques.rs:10:23 + | +LL | type Tait = impl FnOnce() -> (); + | ------------------- the found opaque type +... +LL | Thunk::new(|cont| cont) + | ^^^^ expected `()`, found opaque type + | + = note: expected unit type `()` + found opaque type `Tait` + +error[E0308]: mismatched types + --> $DIR/lazy_subtyping_of_opaques.rs:10:5 + | +LL | fn reify_as_tait() -> Thunk { + | ----------- expected `Thunk<_>` because of return type +LL | Thunk::new(|cont| cont) + | ^^^^^^^^^^^^^^^^^^^^^^^ expected `Thunk<_>`, found `()` + | + = note: expected struct `Thunk<_>` + found unit type `()` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From a85f6a66400c119dda97ad6dde23fff2e5516eb0 Mon Sep 17 00:00:00 2001 From: Mees Frensel <33722705+meesfrensel@users.noreply.github.com> Date: Fri, 24 May 2024 17:34:12 +0200 Subject: [PATCH 20/27] Fix some SIMD intrinsics documentation --- library/core/src/intrinsics/simd.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/intrinsics/simd.rs b/library/core/src/intrinsics/simd.rs index d1be534eaf083..820f6b2cddc34 100644 --- a/library/core/src/intrinsics/simd.rs +++ b/library/core/src/intrinsics/simd.rs @@ -152,7 +152,7 @@ extern "rust-intrinsic" { #[rustc_nounwind] pub fn simd_fabs(x: T) -> T; - /// Elementwise minimum of a vector. + /// Elementwise minimum of two vectors. /// /// `T` must be a vector of floating-point primitive types. /// @@ -160,7 +160,7 @@ extern "rust-intrinsic" { #[rustc_nounwind] pub fn simd_fmin(x: T, y: T) -> T; - /// Elementwise maximum of a vector. + /// Elementwise maximum of two vectors. /// /// `T` must be a vector of floating-point primitive types. /// @@ -387,7 +387,7 @@ extern "rust-intrinsic" { #[rustc_nounwind] pub fn simd_reduce_mul_ordered(x: T, y: U) -> U; - /// Add elements within a vector in arbitrary order. May also be re-associated with + /// Multiply elements within a vector in arbitrary order. May also be re-associated with /// unordered additions on the inputs/outputs. /// /// `T` must be a vector of integer or floating-point primitive types. @@ -405,7 +405,7 @@ extern "rust-intrinsic" { #[rustc_nounwind] pub fn simd_reduce_all(x: T) -> bool; - /// Check if all mask values are true. + /// Check if any mask value is true. /// /// `T` must be a vector of integer primitive types. /// From 3ee84983f1a0cf1fe0be0d0fb024be9e2eb6fd13 Mon Sep 17 00:00:00 2001 From: Alona Enraght-Moony Date: Fri, 24 May 2024 16:40:20 +0000 Subject: [PATCH 21/27] rustdoc-json: Add test for keywords with `--document-private-items` --- tests/rustdoc-json/keyword_private.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 tests/rustdoc-json/keyword_private.rs diff --git a/tests/rustdoc-json/keyword_private.rs b/tests/rustdoc-json/keyword_private.rs new file mode 100644 index 0000000000000..1c2b7d0215505 --- /dev/null +++ b/tests/rustdoc-json/keyword_private.rs @@ -0,0 +1,20 @@ +// Ensure keyword docs are present with --document-private-items + +//@ compile-flags: --document-private-items +#![feature(rustdoc_internals)] + +// @!has "$.index[*][?(@.name=='match')]" +// @has "$.index[*][?(@.name=='foo')]" +// @is "$.index[*][?(@.name=='foo')].attrs" '["#[doc(keyword = \"match\")]"]' +// @is "$.index[*][?(@.name=='foo')].docs" '"this is a test!"' +#[doc(keyword = "match")] +/// this is a test! +pub mod foo {} + +// @!has "$.index[*][?(@.name=='hello')]" +// @has "$.index[*][?(@.name=='bar')]" +// @is "$.index[*][?(@.name=='bar')].attrs" '["#[doc(keyword = \"hello\")]"]' +// @is "$.index[*][?(@.name=='bar')].docs" '"hello"' +#[doc(keyword = "hello")] +/// hello +mod bar {} From db6ec2618a2abb82efe9e119f301f4ae8ec07682 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 24 May 2024 01:14:31 -0700 Subject: [PATCH 22/27] compiler: const_eval/transform/validate.rs -> mir_transform/validate.rs --- Cargo.lock | 1 + compiler/rustc_const_eval/src/transform/mod.rs | 1 - compiler/rustc_mir_transform/Cargo.toml | 1 + compiler/rustc_mir_transform/src/inline.rs | 2 +- compiler/rustc_mir_transform/src/lib.rs | 2 +- .../src/transform => rustc_mir_transform/src}/validate.rs | 0 6 files changed, 4 insertions(+), 3 deletions(-) rename compiler/{rustc_const_eval/src/transform => rustc_mir_transform/src}/validate.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 3a4f028e695f4..92e6a22b4b916 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4252,6 +4252,7 @@ dependencies = [ "rustc_fluent_macro", "rustc_hir", "rustc_index", + "rustc_infer", "rustc_macros", "rustc_middle", "rustc_mir_build", diff --git a/compiler/rustc_const_eval/src/transform/mod.rs b/compiler/rustc_const_eval/src/transform/mod.rs index e3582c7d31746..b4516a412ef01 100644 --- a/compiler/rustc_const_eval/src/transform/mod.rs +++ b/compiler/rustc_const_eval/src/transform/mod.rs @@ -1,2 +1 @@ pub mod check_consts; -pub mod validate; diff --git a/compiler/rustc_mir_transform/Cargo.toml b/compiler/rustc_mir_transform/Cargo.toml index bd0a54ef3638a..f864a13a31bbc 100644 --- a/compiler/rustc_mir_transform/Cargo.toml +++ b/compiler/rustc_mir_transform/Cargo.toml @@ -16,6 +16,7 @@ rustc_errors = { path = "../rustc_errors" } rustc_fluent_macro = { path = "../rustc_fluent_macro" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } +rustc_infer = { path = "../rustc_infer" } rustc_macros = { path = "../rustc_macros" } rustc_middle = { path = "../rustc_middle" } rustc_mir_build = { path = "../rustc_mir_build" } diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 401056cd49603..fe2237dd2e97a 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -1,7 +1,6 @@ //! Inlining pass for MIR functions use crate::deref_separator::deref_finder; use rustc_attr::InlineAttr; -use rustc_const_eval::transform::validate::validate_types; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_index::bit_set::BitSet; @@ -21,6 +20,7 @@ use rustc_target::spec::abi::Abi; use crate::cost_checker::CostChecker; use crate::simplify::simplify_cfg; use crate::util; +use crate::validate::validate_types; use std::iter; use std::ops::{Range, RangeFrom}; diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 9af48f0bad25c..30d4b480f5890 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -109,9 +109,9 @@ mod simplify_comparison_integral; mod sroa; mod unreachable_enum_branching; mod unreachable_prop; +mod validate; use rustc_const_eval::transform::check_consts::{self, ConstCx}; -use rustc_const_eval::transform::validate; use rustc_mir_dataflow::rustc_peek; rustc_fluent_macro::fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_mir_transform/src/validate.rs similarity index 100% rename from compiler/rustc_const_eval/src/transform/validate.rs rename to compiler/rustc_mir_transform/src/validate.rs From 87048a46fc75dc343637e3c7d3d654fc10d5e589 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 24 May 2024 01:33:13 -0700 Subject: [PATCH 23/27] compiler: unnest rustc_const_eval::check_consts --- .../rustc_const_eval/src/{transform => }/check_consts/check.rs | 0 .../rustc_const_eval/src/{transform => }/check_consts/mod.rs | 0 .../rustc_const_eval/src/{transform => }/check_consts/ops.rs | 0 .../src/{transform => }/check_consts/post_drop_elaboration.rs | 0 .../src/{transform => }/check_consts/qualifs.rs | 0 .../src/{transform => }/check_consts/resolver.rs | 0 compiler/rustc_const_eval/src/lib.rs | 2 +- compiler/rustc_const_eval/src/transform/mod.rs | 1 - compiler/rustc_mir_transform/src/lib.rs | 2 +- compiler/rustc_mir_transform/src/promote_consts.rs | 2 +- 10 files changed, 3 insertions(+), 4 deletions(-) rename compiler/rustc_const_eval/src/{transform => }/check_consts/check.rs (100%) rename compiler/rustc_const_eval/src/{transform => }/check_consts/mod.rs (100%) rename compiler/rustc_const_eval/src/{transform => }/check_consts/ops.rs (100%) rename compiler/rustc_const_eval/src/{transform => }/check_consts/post_drop_elaboration.rs (100%) rename compiler/rustc_const_eval/src/{transform => }/check_consts/qualifs.rs (100%) rename compiler/rustc_const_eval/src/{transform => }/check_consts/resolver.rs (100%) delete mode 100644 compiler/rustc_const_eval/src/transform/mod.rs diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/check_consts/check.rs similarity index 100% rename from compiler/rustc_const_eval/src/transform/check_consts/check.rs rename to compiler/rustc_const_eval/src/check_consts/check.rs diff --git a/compiler/rustc_const_eval/src/transform/check_consts/mod.rs b/compiler/rustc_const_eval/src/check_consts/mod.rs similarity index 100% rename from compiler/rustc_const_eval/src/transform/check_consts/mod.rs rename to compiler/rustc_const_eval/src/check_consts/mod.rs diff --git a/compiler/rustc_const_eval/src/transform/check_consts/ops.rs b/compiler/rustc_const_eval/src/check_consts/ops.rs similarity index 100% rename from compiler/rustc_const_eval/src/transform/check_consts/ops.rs rename to compiler/rustc_const_eval/src/check_consts/ops.rs diff --git a/compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs b/compiler/rustc_const_eval/src/check_consts/post_drop_elaboration.rs similarity index 100% rename from compiler/rustc_const_eval/src/transform/check_consts/post_drop_elaboration.rs rename to compiler/rustc_const_eval/src/check_consts/post_drop_elaboration.rs diff --git a/compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs b/compiler/rustc_const_eval/src/check_consts/qualifs.rs similarity index 100% rename from compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs rename to compiler/rustc_const_eval/src/check_consts/qualifs.rs diff --git a/compiler/rustc_const_eval/src/transform/check_consts/resolver.rs b/compiler/rustc_const_eval/src/check_consts/resolver.rs similarity index 100% rename from compiler/rustc_const_eval/src/transform/check_consts/resolver.rs rename to compiler/rustc_const_eval/src/check_consts/resolver.rs diff --git a/compiler/rustc_const_eval/src/lib.rs b/compiler/rustc_const_eval/src/lib.rs index b14780c0d98c2..3a7c87c1cad94 100644 --- a/compiler/rustc_const_eval/src/lib.rs +++ b/compiler/rustc_const_eval/src/lib.rs @@ -14,10 +14,10 @@ #![feature(yeet_expr)] #![feature(if_let_guard)] +pub mod check_consts; pub mod const_eval; mod errors; pub mod interpret; -pub mod transform; pub mod util; use std::sync::atomic::AtomicBool; diff --git a/compiler/rustc_const_eval/src/transform/mod.rs b/compiler/rustc_const_eval/src/transform/mod.rs deleted file mode 100644 index b4516a412ef01..0000000000000 --- a/compiler/rustc_const_eval/src/transform/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod check_consts; diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 30d4b480f5890..e4670633914e4 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -111,7 +111,7 @@ mod unreachable_enum_branching; mod unreachable_prop; mod validate; -use rustc_const_eval::transform::check_consts::{self, ConstCx}; +use rustc_const_eval::check_consts::{self, ConstCx}; use rustc_mir_dataflow::rustc_peek; rustc_fluent_macro::fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_mir_transform/src/promote_consts.rs b/compiler/rustc_mir_transform/src/promote_consts.rs index 34aa31baab79d..e37f90ae7f400 100644 --- a/compiler/rustc_mir_transform/src/promote_consts.rs +++ b/compiler/rustc_mir_transform/src/promote_consts.rs @@ -30,7 +30,7 @@ use std::assert_matches::assert_matches; use std::cell::Cell; use std::{cmp, iter, mem}; -use rustc_const_eval::transform::check_consts::{qualifs, ConstCx}; +use rustc_const_eval::check_consts::{qualifs, ConstCx}; /// A `MirPass` for promotion. /// From 584975d60693d177228229e0500a1abd3faf623d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 24 May 2024 01:33:55 -0700 Subject: [PATCH 24/27] clippy: unnest check_consts --- src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 8ee7d87acb3e0..81e94725a70cb 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -6,7 +6,7 @@ use clippy_config::msrvs::{self, Msrv}; use hir::LangItem; use rustc_attr::StableSince; -use rustc_const_eval::transform::check_consts::ConstCx; +use rustc_const_eval::check_consts::ConstCx; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_infer::infer::TyCtxtInferExt; From 14fc3fdb2c10ec2c9d860618b7681b36078668f1 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 24 May 2024 02:24:22 -0700 Subject: [PATCH 25/27] miri: receive the blessings of validate.rs --- src/tools/miri/tests/panic/mir-validation.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/tests/panic/mir-validation.stderr b/src/tools/miri/tests/panic/mir-validation.stderr index d158c996dc3de..d5dd53d7b4e99 100644 --- a/src/tools/miri/tests/panic/mir-validation.stderr +++ b/src/tools/miri/tests/panic/mir-validation.stderr @@ -1,4 +1,4 @@ -thread 'rustc' panicked at compiler/rustc_const_eval/src/transform/validate.rs:LL:CC: +thread 'rustc' panicked at compiler/rustc_mir_transform/src/validate.rs:LL:CC: broken MIR in Item(DefId) (after phase change to runtime-optimized) at bb0[1]: (*(_2.0: *mut i32)), has deref at the wrong place stack backtrace: From de517b79bc417caa507d598824ae869fe4f7c74e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 24 May 2024 12:19:33 -0400 Subject: [PATCH 26/27] Actually just remove the special case altogether --- .../src/collect/predicates_of.rs | 122 +++++++----------- tests/crashes/118403.rs | 8 -- tests/crashes/121574-2.rs | 8 -- tests/crashes/121574.rs | 6 - .../double-opaque-parent-predicates.rs | 13 ++ .../double-opaque-parent-predicates.stderr | 11 ++ 6 files changed, 71 insertions(+), 97 deletions(-) delete mode 100644 tests/crashes/118403.rs delete mode 100644 tests/crashes/121574-2.rs delete mode 100644 tests/crashes/121574.rs create mode 100644 tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.rs create mode 100644 tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.stderr diff --git a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs index db36aba7edf44..606f16537678a 100644 --- a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs @@ -461,83 +461,55 @@ pub(super) fn explicit_predicates_of<'tcx>( } } } else { - if matches!(def_kind, DefKind::AnonConst) && tcx.features().generic_const_exprs { - let hir_id = tcx.local_def_id_to_hir_id(def_id); - let parent_def_id = tcx.hir().get_parent_item(hir_id); - - if let Some(defaulted_param_def_id) = - tcx.hir().opt_const_param_default_param_def_id(hir_id) - { - // In `generics_of` we set the generics' parent to be our parent's parent which means that - // we lose out on the predicates of our actual parent if we dont return those predicates here. - // (See comment in `generics_of` for more information on why the parent shenanigans is necessary) - // - // struct Foo::ASSOC }>(T) where T: Trait; - // ^^^ ^^^^^^^^^^^^^^^^^^^^^^^ the def id we are calling - // ^^^ explicit_predicates_of on - // parent item we dont have set as the - // parent of generics returned by `generics_of` - // - // In the above code we want the anon const to have predicates in its param env for `T: Trait` - // and we would be calling `explicit_predicates_of(Foo)` here - let parent_preds = tcx.explicit_predicates_of(parent_def_id); - - // If we dont filter out `ConstArgHasType` predicates then every single defaulted const parameter - // will ICE because of #106994. FIXME(generic_const_exprs): remove this when a more general solution - // to #106994 is implemented. - let filtered_predicates = parent_preds - .predicates - .into_iter() - .filter(|(pred, _)| { - if let ty::ClauseKind::ConstArgHasType(ct, _) = pred.kind().skip_binder() { - match ct.kind() { - ty::ConstKind::Param(param_const) => { - let defaulted_param_idx = tcx - .generics_of(parent_def_id) - .param_def_id_to_index[&defaulted_param_def_id.to_def_id()]; - param_const.index < defaulted_param_idx - } - _ => bug!( - "`ConstArgHasType` in `predicates_of`\ - that isn't a `Param` const" - ), + if matches!(def_kind, DefKind::AnonConst) + && tcx.features().generic_const_exprs + && let Some(defaulted_param_def_id) = + tcx.hir().opt_const_param_default_param_def_id(tcx.local_def_id_to_hir_id(def_id)) + { + // In `generics_of` we set the generics' parent to be our parent's parent which means that + // we lose out on the predicates of our actual parent if we dont return those predicates here. + // (See comment in `generics_of` for more information on why the parent shenanigans is necessary) + // + // struct Foo::ASSOC }>(T) where T: Trait; + // ^^^ ^^^^^^^^^^^^^^^^^^^^^^^ the def id we are calling + // ^^^ explicit_predicates_of on + // parent item we dont have set as the + // parent of generics returned by `generics_of` + // + // In the above code we want the anon const to have predicates in its param env for `T: Trait` + // and we would be calling `explicit_predicates_of(Foo)` here + let parent_def_id = tcx.local_parent(def_id); + let parent_preds = tcx.explicit_predicates_of(parent_def_id); + + // If we dont filter out `ConstArgHasType` predicates then every single defaulted const parameter + // will ICE because of #106994. FIXME(generic_const_exprs): remove this when a more general solution + // to #106994 is implemented. + let filtered_predicates = parent_preds + .predicates + .into_iter() + .filter(|(pred, _)| { + if let ty::ClauseKind::ConstArgHasType(ct, _) = pred.kind().skip_binder() { + match ct.kind() { + ty::ConstKind::Param(param_const) => { + let defaulted_param_idx = tcx + .generics_of(parent_def_id) + .param_def_id_to_index[&defaulted_param_def_id.to_def_id()]; + param_const.index < defaulted_param_idx } - } else { - true + _ => bug!( + "`ConstArgHasType` in `predicates_of`\ + that isn't a `Param` const" + ), } - }) - .cloned(); - return GenericPredicates { - parent: parent_preds.parent, - predicates: { tcx.arena.alloc_from_iter(filtered_predicates) }, - }; - } - - let parent_def_kind = tcx.def_kind(parent_def_id); - if matches!(parent_def_kind, DefKind::OpaqueTy) { - // In `instantiate_identity` we inherit the predicates of our parent. - // However, opaque types do not have a parent (see `gather_explicit_predicates_of`), which means - // that we lose out on the predicates of our actual parent if we dont return those predicates here. - // - // - // fn foo() -> impl Iterator::ASSOC }> > { todo!() } - // ^^^^^^^^^^^^^^^^^^^ the def id we are calling - // explicit_predicates_of on - // - // In the above code we want the anon const to have predicates in its param env for `T: Trait`. - // However, the anon const cannot inherit predicates from its parent since it's opaque. - // - // To fix this, we call `explicit_predicates_of` directly on `foo`, the parent's parent. - - // In the above example this is `foo::{opaque#0}` or `impl Iterator` - let parent_hir_id = tcx.local_def_id_to_hir_id(parent_def_id.def_id); - - // In the above example this is the function `foo` - let item_def_id = tcx.hir().get_parent_item(parent_hir_id); - - // In the above code example we would be calling `explicit_predicates_of(foo)` here - return tcx.explicit_predicates_of(item_def_id); - } + } else { + true + } + }) + .cloned(); + return GenericPredicates { + parent: parent_preds.parent, + predicates: { tcx.arena.alloc_from_iter(filtered_predicates) }, + }; } gather_explicit_predicates_of(tcx, def_id) } diff --git a/tests/crashes/118403.rs b/tests/crashes/118403.rs deleted file mode 100644 index 21ab15f9ffd0a..0000000000000 --- a/tests/crashes/118403.rs +++ /dev/null @@ -1,8 +0,0 @@ -//@ known-bug: #118403 -#![feature(generic_const_exprs)] -pub struct X {} -impl X { - pub fn y<'a, U: 'a>(&'a self) -> impl Iterator + '_> { - (0..1).map(move |_| (0..1).map(move |_| loop {})) - } -} diff --git a/tests/crashes/121574-2.rs b/tests/crashes/121574-2.rs deleted file mode 100644 index a08f3f063974b..0000000000000 --- a/tests/crashes/121574-2.rs +++ /dev/null @@ -1,8 +0,0 @@ -//@ known-bug: #121574 -#![feature(generic_const_exprs)] -pub struct DimName {} -impl X { - pub fn y<'a, U: 'a>(&'a self) -> impl Iterator + '_> { - "0".as_bytes(move |_| (0..1).map(move |_| loop {})) - } -} diff --git a/tests/crashes/121574.rs b/tests/crashes/121574.rs deleted file mode 100644 index 53eec829c5f38..0000000000000 --- a/tests/crashes/121574.rs +++ /dev/null @@ -1,6 +0,0 @@ -//@ known-bug: #121574 -#![feature(generic_const_exprs)] - -impl X { - pub fn y<'a, U: 'a>(&'a self) -> impl Iterator + '_> {} -} diff --git a/tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.rs b/tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.rs new file mode 100644 index 0000000000000..e48d559aa3270 --- /dev/null +++ b/tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.rs @@ -0,0 +1,13 @@ +//@ check-pass + +#![feature(generic_const_exprs)] +//~^ WARN the feature `generic_const_exprs` is incomplete and may not be safe to use + +pub fn y<'a, U: 'a>() -> impl IntoIterator + 'a> { + [[[1, 2, 3]]] +} +// Make sure that the `predicates_of` for `{ 1 + 2 }` don't mention the duplicated lifetimes of +// the *outer* iterator. Whether they should mention the duplicated lifetimes of the *inner* +// iterator are another question, but not really something we need to answer immediately. + +fn main() {} diff --git a/tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.stderr b/tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.stderr new file mode 100644 index 0000000000000..faaede13e6b64 --- /dev/null +++ b/tests/ui/const-generics/generic_const_exprs/double-opaque-parent-predicates.stderr @@ -0,0 +1,11 @@ +warning: the feature `generic_const_exprs` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/double-opaque-parent-predicates.rs:3:12 + | +LL | #![feature(generic_const_exprs)] + | ^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #76560 for more information + = note: `#[warn(incomplete_features)]` on by default + +warning: 1 warning emitted + From c97ed58c381e1d2455b64672e1afef0c1caf34e9 Mon Sep 17 00:00:00 2001 From: lcnr Date: Fri, 24 May 2024 20:27:47 +0000 Subject: [PATCH 27/27] tag more stuff with `WG-trait-system-refactor` --- triagebot.toml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index 2e45b257f8126..8b6d5f7ef7af7 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -389,8 +389,11 @@ exclude_labels = [ [autolabel."WG-trait-system-refactor"] trigger_files = [ + "compiler/rustc_middle/src/traits/solve", + "compiler/rustc_next_trait_solver", "compiler/rustc_trait_selection/src/solve", - "compiler/rustc_middle/src/traits/solve" + "compiler/rustc_type_ir/src/solve", + "tests/ui/traits/next-solver", ] [autolabel."PG-exploit-mitigations"]