Skip to content
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

Pessimistically assume opaque types are !Freeze #113617

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
25 changes: 22 additions & 3 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ extern crate tracing;
#[macro_use]
extern crate rustc_middle;

use hir::ConstContext;
use required_consts::RequiredConstsVisitor;
use rustc_const_eval::util;
use rustc_data_structures::fx::FxIndexSet;
Expand Down Expand Up @@ -231,8 +232,12 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def: LocalDefId) -> ConstQualifs {
let const_kind = tcx.hir().body_const_context(def);

// No need to const-check a non-const `fn`.
if const_kind.is_none() {
return Default::default();
match const_kind {
Some(ConstContext::Const | ConstContext::Static(_)) | Some(ConstContext::ConstFn) => {}
None => span_bug!(
tcx.def_span(def),
"`mir_const_qualif` should only be called on const fns and const items"
),
}

// N.B., this `borrow()` is guaranteed to be valid (i.e., the value
Expand Down Expand Up @@ -297,7 +302,21 @@ fn mir_promoted(
// Ensure that we compute the `mir_const_qualif` for constants at
// this point, before we steal the mir-const result.
// Also this means promotion can rely on all const checks having been done.
let const_qualifs = tcx.mir_const_qualif(def);

let const_qualifs = match tcx.def_kind(def) {
DefKind::Fn | DefKind::AssocFn | DefKind::Closure
if tcx.constness(def) == hir::Constness::Const
|| tcx.is_const_default_method(def.to_def_id()) =>
{
tcx.mir_const_qualif(def)
}
DefKind::AssocConst
| DefKind::Const
| DefKind::Static(_)
| DefKind::InlineConst
| DefKind::AnonConst => tcx.mir_const_qualif(def),
_ => ConstQualifs::default(),
};
Comment on lines -300 to +319
Copy link
Contributor Author

@oli-obk oli-obk Jul 12, 2023

Choose a reason for hiding this comment

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

This change does not affect any logic and just reduces the number of mir_const_qualif calls, and thus hopefully also reduces the cost of caching things

let mut body = tcx.mir_const(def).steal();
if let Some(error_reported) = const_qualifs.tainted_by_errors {
body.tainted_by_errors = Some(error_reported);
Expand Down
28 changes: 28 additions & 0 deletions tests/ui/consts/const-fn-cycle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// Discovered in https://github.com/rust-lang/rust/issues/112602.
/// This caused a cycle error, which made no sense.
/// Removing the `const` part of the `many` function would make the
/// test pass again.
/// The issue was that we were running const qualif checks on
/// `const fn`s, but never using them. During const qualif checks we tend
/// to end up revealing opaque types (the RPIT in `many`'s return type),
/// which can quickly lead to cycles.

pub struct Parser<H>(H);

impl<H, T> Parser<H>
where
H: for<'a> Fn(&'a str) -> T,
{
pub const fn new(handler: H) -> Parser<H> {
Parser(handler)
}

pub const fn many<'s>(&'s self) -> Parser<impl for<'a> Fn(&'a str) -> Vec<T> + 's> {
//~^ ERROR: cycle detected
Parser::new(|_| unimplemented!())
}
}

fn main() {
println!("Hello, world!");
}
39 changes: 39 additions & 0 deletions tests/ui/consts/const-fn-cycle.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0391]: cycle detected when computing type of `<impl at $DIR/const-fn-cycle.rs:12:1: 12:21>::many::{opaque#0}`
--> $DIR/const-fn-cycle.rs:20:47
|
LL | pub const fn many<'s>(&'s self) -> Parser<impl for<'a> Fn(&'a str) -> Vec<T> + 's> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: ...which requires borrow-checking `<impl at $DIR/const-fn-cycle.rs:12:1: 12:21>::many`...
--> $DIR/const-fn-cycle.rs:20:5
|
LL | pub const fn many<'s>(&'s self) -> Parser<impl for<'a> Fn(&'a str) -> Vec<T> + 's> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `<impl at $DIR/const-fn-cycle.rs:12:1: 12:21>::many`...
--> $DIR/const-fn-cycle.rs:20:5
|
LL | pub const fn many<'s>(&'s self) -> Parser<impl for<'a> Fn(&'a str) -> Vec<T> + 's> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: ...which requires const checking `<impl at $DIR/const-fn-cycle.rs:12:1: 12:21>::many`...
--> $DIR/const-fn-cycle.rs:20:5
|
LL | pub const fn many<'s>(&'s self) -> Parser<impl for<'a> Fn(&'a str) -> Vec<T> + 's> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which requires computing whether `Parser<<impl at $DIR/const-fn-cycle.rs:12:1: 12:21>::many::{opaque#0}>` is freeze...
= note: ...which requires evaluating trait selection obligation `Parser<<impl at $DIR/const-fn-cycle.rs:12:1: 12:21>::many::{opaque#0}>: core::marker::Freeze`...
= note: ...which again requires computing type of `<impl at $DIR/const-fn-cycle.rs:12:1: 12:21>::many::{opaque#0}`, completing the cycle
note: cycle used when checking item types in top-level module
--> $DIR/const-fn-cycle.rs:1:1
|
LL | / /// Discovered in https://github.com/rust-lang/rust/issues/112602.
LL | | /// This caused a cycle error, which made no sense.
LL | | /// Removing the `const` part of the `many` function would make the
LL | | /// test pass again.
... |
LL | | println!("Hello, world!");
LL | | }
| |_^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
39 changes: 39 additions & 0 deletions tests/ui/consts/const-promoted-opaque.atomic.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0391]: cycle detected when computing type of `Foo::{opaque#0}`
--> $DIR/const-promoted-opaque.rs:13:12
|
LL | type Foo = impl Sized;
| ^^^^^^^^^^
|
note: ...which requires borrow-checking `FOO`...
--> $DIR/const-promoted-opaque.rs:20:1
|
LL | const FOO: Foo = std::sync::atomic::AtomicU8::new(42);
| ^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `FOO`...
--> $DIR/const-promoted-opaque.rs:20:1
|
LL | const FOO: Foo = std::sync::atomic::AtomicU8::new(42);
| ^^^^^^^^^^^^^^
note: ...which requires const checking `FOO`...
--> $DIR/const-promoted-opaque.rs:20:1
|
LL | const FOO: Foo = std::sync::atomic::AtomicU8::new(42);
| ^^^^^^^^^^^^^^
= note: ...which requires computing whether `Foo` is freeze...
= note: ...which requires evaluating trait selection obligation `Foo: core::marker::Freeze`...
= note: ...which again requires computing type of `Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in top-level module
--> $DIR/const-promoted-opaque.rs:2:1
|
LL | / #![feature(type_alias_impl_trait)]
LL | |
LL | | //! Check that we do not cause cycle errors when trying to
LL | | //! obtain information about interior mutability of an opaque type.
... |
LL | | let _: &'static _ = &FOO;
LL | | }
| |_^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
33 changes: 33 additions & 0 deletions tests/ui/consts/const-promoted-opaque.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// revisions: string unit atomic
#![feature(type_alias_impl_trait)]

//! Check that we do not cause cycle errors when trying to
//! obtain information about interior mutability of an opaque type.
//! This used to happen, because when the body-analysis failed, we
//! checked the type instead, but the constant was also defining the
//! hidden type of the opaque type. Thus we ended up relying on the
//! result of our analysis to compute the result of our analysis.

//[unit] check-pass

type Foo = impl Sized;
//[string,atomic]~^ ERROR cycle detected

#[cfg(string)]
const FOO: Foo = String::new();

#[cfg(atomic)]
const FOO: Foo = std::sync::atomic::AtomicU8::new(42);

#[cfg(unit)]
const FOO: Foo = ();

const BAR: () = {
let _: &'static _ = &FOO;
};

const BAZ: &Foo = &FOO;

fn main() {
let _: &'static _ = &FOO;
}
39 changes: 39 additions & 0 deletions tests/ui/consts/const-promoted-opaque.string.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error[E0391]: cycle detected when computing type of `Foo::{opaque#0}`
--> $DIR/const-promoted-opaque.rs:13:12
|
LL | type Foo = impl Sized;
| ^^^^^^^^^^
|
note: ...which requires borrow-checking `FOO`...
--> $DIR/const-promoted-opaque.rs:17:1
|
LL | const FOO: Foo = String::new();
| ^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `FOO`...
--> $DIR/const-promoted-opaque.rs:17:1
|
LL | const FOO: Foo = String::new();
| ^^^^^^^^^^^^^^
note: ...which requires const checking `FOO`...
--> $DIR/const-promoted-opaque.rs:17:1
|
LL | const FOO: Foo = String::new();
| ^^^^^^^^^^^^^^
= note: ...which requires computing whether `Foo` is freeze...
= note: ...which requires evaluating trait selection obligation `Foo: core::marker::Freeze`...
= note: ...which again requires computing type of `Foo::{opaque#0}`, completing the cycle
note: cycle used when checking item types in top-level module
--> $DIR/const-promoted-opaque.rs:2:1
|
LL | / #![feature(type_alias_impl_trait)]
LL | |
LL | | //! Check that we do not cause cycle errors when trying to
LL | | //! obtain information about interior mutability of an opaque type.
... |
LL | | let _: &'static _ = &FOO;
LL | | }
| |_^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.