Skip to content

Recover statics better #125555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bad sign that this check is needed. We should never even evaluate a static whose type references an error. So something still seems wrong here.

{
assert_eq!(alloc.mutability, mutability);
}
mutability
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2509,6 +2509,24 @@ impl<'hir> Ty<'hir> {
_ => false,
}
}

pub fn references_error(&self) -> Option<rustc_span::ErrorGuaranteed> {
use crate::intravisit::Visitor;
struct ErrVisitor(Option<rustc_span::ErrorGuaranteed>);
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`.
Expand Down
16 changes: 12 additions & 4 deletions compiler/rustc_hir_analysis/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<Ty

Node::Item(item) => 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,
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ty::ReErased => tcx.lifetimes.re_static,
ty::ReErased => ty::Region::new_error(tcx, guar),

would also fix this issue without requiring any of the other changes, because then the error type is tainted, without replacing any of the types. So you'd still get the suggestions, but const eval won't attempt to run.

_ => region,
});
ty
}

fn check_feature_inherent_assoc_ty(tcx: TyCtxt<'_>, span: Span) {
Expand Down
4 changes: 0 additions & 4 deletions tests/crashes/124164.rs

This file was deleted.

1 change: 1 addition & 0 deletions tests/ui/generic-const-items/assoc-const-missing-type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 20 additions & 4 deletions tests/ui/generic-const-items/assoc-const-missing-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,24 @@ error: missing type for `const` item
LL | const K<T> = ();
| ^ 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<T> = ();
| - ^ 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>: 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
Expand All @@ -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`
Expand All @@ -42,7 +58,7 @@ LL | const K<T> = ();
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`.
4 changes: 3 additions & 1 deletion tests/ui/impl-trait/issues/issue-86642.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down
35 changes: 33 additions & 2 deletions tests/ui/impl-trait/issues/issue-86642.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
8 changes: 8 additions & 0 deletions tests/ui/static/error-type-interior-mutable.rs
Original file line number Diff line number Diff line change
@@ -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() {}
8 changes: 8 additions & 0 deletions tests/ui/static/error-type-interior-mutable.stderr
Original file line number Diff line number Diff line change
@@ -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

5 changes: 4 additions & 1 deletion tests/ui/typeck/typeck_type_placeholder_item.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading