From b81adab25a6da17368035bb5670b76317d56c66f Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sat, 25 May 2024 22:32:16 +0200 Subject: [PATCH 1/3] Also recover type for errors in static items in type_of The const interner uses the body to intern static allocations. This means that an allocation will be mutable if the resulting type contains interior mutability. Const eval contains an assertion that types that are `Freeze` do not have mutable allocations, which was failing for cases where the type was a type error. An alternative fix for this would be to avoid the assertion in const eval for type errors, allowing those to both be mutable and immutable. This is implemented in a follow-up commit as well. --- compiler/rustc_hir/src/hir.rs | 18 ++++++++++++++++++ .../rustc_hir_analysis/src/collect/type_of.rs | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 2f4dcdbdf2b17..ff24271e9904e 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2509,6 +2509,24 @@ impl<'hir> Ty<'hir> { _ => false, } } + + pub fn references_error(&self) -> Option { + use crate::intravisit::Visitor; + struct ErrVisitor(Option); + impl<'v> Visitor<'v> for ErrVisitor { + fn visit_ty(&mut self, t: &'v Ty<'v>) { + if let TyKind::Err(guar) = t.kind { + self.0 = Some(guar); + return; + } + crate::intravisit::walk_ty(self, t); + } + } + + let mut err_visitor = ErrVisitor(None); + err_visitor.visit_ty(self); + err_visitor.0 + } } /// Not represented directly in the AST; referred to by name through a `ty_path`. diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index 1475e53c47c29..ee9137fed5942 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -403,7 +403,7 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder match item.kind { ItemKind::Static(ty, .., body_id) => { - if ty.is_suggestable_infer_ty() { + if ty.is_suggestable_infer_ty() || ty.references_error().is_some() { infer_placeholder_type( tcx, def_id, From 7314f82d30773469c716d7c4275ea5a9b1879453 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sat, 25 May 2024 22:36:16 +0200 Subject: [PATCH 2/3] Better recover static/const type in type_of When the type is an inferred placeholder or an error, we used to infer the type for an error message and then return `TyKind::Error`. Now we return the proper type, which helps follow-up code do more checking. The primary motivation for this was fixing the const-eval mutability assertion bug, because this commit forwards the inferred type to that assertion, but it is also just nicer in general. --- .../rustc_hir_analysis/src/collect/type_of.rs | 14 ++++++-- tests/crashes/124164.rs | 4 --- .../assoc-const-missing-type.rs | 1 + .../assoc-const-missing-type.stderr | 24 ++++++++++--- tests/ui/impl-trait/issues/issue-86642.rs | 4 ++- tests/ui/impl-trait/issues/issue-86642.stderr | 35 +++++++++++++++++-- .../ui/static/error-type-interior-mutable.rs | 8 +++++ .../static/error-type-interior-mutable.stderr | 8 +++++ .../typeck_type_placeholder_item.stderr | 5 ++- 9 files changed, 88 insertions(+), 15 deletions(-) delete mode 100644 tests/crashes/124164.rs create mode 100644 tests/ui/static/error-type-interior-mutable.rs create mode 100644 tests/ui/static/error-type-interior-mutable.stderr diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index ee9137fed5942..74b5a03f9a142 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -571,8 +571,7 @@ fn infer_placeholder_type<'a>( // then the user may have written e.g. `const A = 42;`. // In this case, the parser has stashed a diagnostic for // us to improve in typeck so we do that now. - let guar = tcx - .dcx() + tcx.dcx() .try_steal_modify_and_emit_err(span, StashKey::ItemNoType, |err| { if !ty.references_error() { // Only suggest adding `:` if it was missing (and suggested by parsing diagnostic). @@ -619,7 +618,16 @@ fn infer_placeholder_type<'a>( } diag.emit() }); - Ty::new_error(tcx, guar) + + // Typeck returns regions as erased. We can't deal with erased regions here though, so we + // turn them into `&'static`, which is *generally* correct for statics and consts. + // Assoc consts can reference generic lifetimes from the parent generics, but treating them + // as static is unlikely to cause issues. + let ty = tcx.fold_regions(ty, |region, _| match region.kind() { + ty::ReErased => tcx.lifetimes.re_static, + _ => region, + }); + ty } fn check_feature_inherent_assoc_ty(tcx: TyCtxt<'_>, span: Span) { diff --git a/tests/crashes/124164.rs b/tests/crashes/124164.rs deleted file mode 100644 index 8c9b4bddbe80d..0000000000000 --- a/tests/crashes/124164.rs +++ /dev/null @@ -1,4 +0,0 @@ -//@ known-bug: #124164 -static S_COUNT: = std::sync::atomic::AtomicUsize::new(0); - -fn main() {} 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 93160f0b5758d..df39e90c455e4 100644 --- a/tests/ui/generic-const-items/assoc-const-missing-type.rs +++ b/tests/ui/generic-const-items/assoc-const-missing-type.rs @@ -13,6 +13,7 @@ impl Trait for () { //~^ ERROR missing type for `const` item //~| ERROR mismatched types //~| ERROR mismatched types + //~| ERROR implemented const `K` has an incompatible type for trait const Q = ""; //~^ ERROR missing type for `const` item //~| ERROR lifetime parameters or bounds on const `Q` do not match the trait declaration 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 6f35c0958d45c..20328fd7303c3 100644 --- a/tests/ui/generic-const-items/assoc-const-missing-type.stderr +++ b/tests/ui/generic-const-items/assoc-const-missing-type.stderr @@ -15,8 +15,24 @@ error: missing type for `const` item LL | const K = (); | ^ help: provide a type for the associated constant: `()` +error[E0326]: implemented const `K` has an incompatible type for trait + --> $DIR/assoc-const-missing-type.rs:12:15 + | +LL | const K = (); + | - ^ expected type parameter `T`, found `()` + | | + | expected this type parameter + | +note: type in trait + --> $DIR/assoc-const-missing-type.rs:7:17 + | +LL | const K: T; + | ^ + = note: expected type parameter `T` + found unit type `()` + error[E0195]: lifetime parameters or bounds on const `Q` do not match the trait declaration - --> $DIR/assoc-const-missing-type.rs:16:12 + --> $DIR/assoc-const-missing-type.rs:17:12 | LL | const Q<'a>: &'a str; | ---- lifetimes in impl do not match this const in trait @@ -25,7 +41,7 @@ LL | const Q = ""; | ^ lifetimes do not match const in trait error: missing type for `const` item - --> $DIR/assoc-const-missing-type.rs:16:12 + --> $DIR/assoc-const-missing-type.rs:17:12 | LL | const Q = ""; | ^ help: provide a type for the associated constant: `: &str` @@ -42,7 +58,7 @@ LL | const K = (); found unit type `()` = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors -Some errors have detailed explanations: E0195, E0308. +Some errors have detailed explanations: E0195, E0308, E0326. For more information about an error, try `rustc --explain E0195`. diff --git a/tests/ui/impl-trait/issues/issue-86642.rs b/tests/ui/impl-trait/issues/issue-86642.rs index 74be8779d4458..463919c415ff0 100644 --- a/tests/ui/impl-trait/issues/issue-86642.rs +++ b/tests/ui/impl-trait/issues/issue-86642.rs @@ -1,5 +1,7 @@ static x: impl Fn(&str) -> Result<&str, ()> = move |source| { - //~^ `impl Trait` is not allowed in static types + //~^ ERROR `impl Trait` is not allowed in static types + //~| ERROR cycle detected when computing type of `x` + //~| ERROR the placeholder `_` is not allowed within types on item signatures for static variables let res = (move |source| Ok(source))(source); let res = res.or((move |source| Ok(source))(source)); res diff --git a/tests/ui/impl-trait/issues/issue-86642.stderr b/tests/ui/impl-trait/issues/issue-86642.stderr index 19fd5bc0c1ca4..e97270466d7cf 100644 --- a/tests/ui/impl-trait/issues/issue-86642.stderr +++ b/tests/ui/impl-trait/issues/issue-86642.stderr @@ -6,6 +6,37 @@ LL | static x: impl Fn(&str) -> Result<&str, ()> = move |source| { | = note: `impl Trait` is only allowed in arguments and return types of functions and methods -error: aborting due to 1 previous error +error[E0391]: cycle detected when computing type of `x` + --> $DIR/issue-86642.rs:1:1 + | +LL | static x: impl Fn(&str) -> Result<&str, ()> = move |source| { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: ...which requires type-checking `x`... + --> $DIR/issue-86642.rs:1:1 + | +LL | static x: impl Fn(&str) -> Result<&str, ()> = move |source| { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: ...which requires computing the opaque types defined by `x`... + --> $DIR/issue-86642.rs:1:1 + | +LL | static x: impl Fn(&str) -> Result<&str, ()> = move |source| { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: ...which again requires computing type of `x`, completing the cycle +note: cycle used when checking that `x` is well-formed + --> $DIR/issue-86642.rs:1:1 + | +LL | static x: impl Fn(&str) -> Result<&str, ()> = move |source| { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information + +error[E0121]: the placeholder `_` is not allowed within types on item signatures for static variables + --> $DIR/issue-86642.rs:1:11 + | +LL | static x: impl Fn(&str) -> Result<&str, ()> = move |source| { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not allowed in type signatures + +error: aborting due to 3 previous errors -For more information about this error, try `rustc --explain E0562`. +Some errors have detailed explanations: E0121, E0391, E0562. +For more information about an error, try `rustc --explain E0121`. diff --git a/tests/ui/static/error-type-interior-mutable.rs b/tests/ui/static/error-type-interior-mutable.rs new file mode 100644 index 0000000000000..90b19f0d196f3 --- /dev/null +++ b/tests/ui/static/error-type-interior-mutable.rs @@ -0,0 +1,8 @@ +// A regression test for #124164 +// const-eval used to complain because the allocation was mutable (due to the atomic) +// while it expected `{type error}` allocations to be immutable. + +static S_COUNT: = std::sync::atomic::AtomicUsize::new(0); +//~^ ERROR missing type for `static` item + +fn main() {} diff --git a/tests/ui/static/error-type-interior-mutable.stderr b/tests/ui/static/error-type-interior-mutable.stderr new file mode 100644 index 0000000000000..975f9dcaf7d05 --- /dev/null +++ b/tests/ui/static/error-type-interior-mutable.stderr @@ -0,0 +1,8 @@ +error: missing type for `static` item + --> $DIR/error-type-interior-mutable.rs:5:16 + | +LL | static S_COUNT: = std::sync::atomic::AtomicUsize::new(0); + | ^ help: provide a type for the static variable: `AtomicUsize` + +error: aborting due to 1 previous error + diff --git a/tests/ui/typeck/typeck_type_placeholder_item.stderr b/tests/ui/typeck/typeck_type_placeholder_item.stderr index 7977504dae1d6..e907c9ebe9233 100644 --- a/tests/ui/typeck/typeck_type_placeholder_item.stderr +++ b/tests/ui/typeck/typeck_type_placeholder_item.stderr @@ -583,7 +583,10 @@ error[E0121]: the placeholder `_` is not allowed within types on item signatures --> $DIR/typeck_type_placeholder_item.rs:209:14 | LL | const D: _ = 42; - | ^ not allowed in type signatures + | ^ + | | + | not allowed in type signatures + | help: replace with the correct type: `i32` error[E0046]: not all trait items implemented, missing: `F` --> $DIR/typeck_type_placeholder_item.rs:200:1 From 44a576c53187a74dd37f8f46a21cd1dc3dd7c8b9 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sat, 25 May 2024 22:38:09 +0200 Subject: [PATCH 3/3] Do not assert mutability of allocations for type errors We do not know whether a type error was supposed to be mutable or not. --- .../src/interpret/validity.rs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index cf6027a312fa2..f9a86c927653c 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -20,7 +20,7 @@ use rustc_middle::mir::interpret::{ ValidationErrorInfo, ValidationErrorKind, ValidationErrorKind::*, }; use rustc_middle::ty::layout::{LayoutOf, TyAndLayout}; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, TypeVisitableExt}; use rustc_span::symbol::{sym, Symbol}; use rustc_target::abi::{ Abi, FieldIdx, Scalar as ScalarAbi, Size, VariantIdx, Variants, WrappingRange, @@ -724,20 +724,21 @@ fn mutability<'mir, 'tcx: 'mir>( // so just use the declared mutability. mutability } else { + let ty = ecx + .tcx + .type_of(did) + .no_bound_vars() + .expect("statics should not have generic parameters"); let mutability = match mutability { - Mutability::Not - if !ecx - .tcx - .type_of(did) - .no_bound_vars() - .expect("statics should not have generic parameters") - .is_freeze(*ecx.tcx, ty::ParamEnv::reveal_all()) => - { + Mutability::Not if !ty.is_freeze(*ecx.tcx, ty::ParamEnv::reveal_all()) => { Mutability::Mut } _ => mutability, }; - if let Some((_, alloc)) = ecx.memory.alloc_map.get(alloc_id) { + if let Some((_, alloc)) = ecx.memory.alloc_map.get(alloc_id) + // For type errors, we do not know whether they are supposed to be mutable or not. + && !ty.references_error() + { assert_eq!(alloc.mutability, mutability); } mutability