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 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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_hir::LangItem;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir;
use rustc_middle::mir::*;
use rustc_middle::ty::TypeVisitableExt;
use rustc_middle::ty::{self, subst::SubstsRef, AdtDef, Ty};
use rustc_trait_selection::traits::{
self, ImplSource, Obligation, ObligationCause, ObligationCtxt, SelectionContext,
Expand Down Expand Up @@ -91,7 +92,8 @@ impl Qualif for HasMutInterior {
}

fn in_any_value_of_ty<'tcx>(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool {
!ty.is_freeze(cx.tcx, cx.param_env)
// Pessimistically assume opaque types are `!Freeze`
ty.has_opaque_types() || !ty.is_freeze(cx.tcx, cx.param_env)
}

fn in_adt_inherently<'tcx>(
Expand Down
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
17 changes: 17 additions & 0 deletions tests/ui/consts/const-fn-cycle-tait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(type_alias_impl_trait)]

//! Test that we pessimistically assume the `Drop` impl of
//! a hidden type is not const.

pub struct Parser<H>(H);

type Tait = impl Sized;

const fn constrain() -> Tait {}

pub const fn take(_: Tait) {}
//~^ ERROR: destructor of `Tait` cannot be evaluated at compile-time

fn main() {
println!("Hello, world!");
}
11 changes: 11 additions & 0 deletions tests/ui/consts/const-fn-cycle-tait.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0493]: destructor of `Tait` cannot be evaluated at compile-time
--> $DIR/const-fn-cycle-tait.rs:12:19
|
LL | pub const fn take(_: Tait) {}
| ^ - value is dropped here
| |
| the destructor for this type cannot be evaluated in constant functions

error: aborting due to previous error

For more information about this error, try `rustc --explain E0493`.
29 changes: 29 additions & 0 deletions tests/ui/consts/const-fn-cycle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//! 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.

// check-pass

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> {
Parser::new(|_| unimplemented!())
}
}

fn main() {
println!("Hello, world!");
}
41 changes: 41 additions & 0 deletions tests/ui/consts/const-promoted-opaque.atomic.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
error[E0493]: destructor of `Foo` cannot be evaluated at compile-time
--> $DIR/const-promoted-opaque.rs:25:26
|
LL | let _: &'static _ = &FOO;
| ^^^ the destructor for this type cannot be evaluated in constants
...
LL | };
| - value is dropped here

error[E0716]: temporary value dropped while borrowed
--> $DIR/const-promoted-opaque.rs:25:26
|
LL | let _: &'static _ = &FOO;
| ---------- ^^^ creates a temporary value which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
...
LL | };
| - temporary value is freed at the end of this statement

error[E0492]: constants cannot refer to interior mutable data
--> $DIR/const-promoted-opaque.rs:30:19
|
LL | const BAZ: &Foo = &FOO;
| ^^^^ this borrow of an interior mutable value may end up in the final value

error[E0716]: temporary value dropped while borrowed
--> $DIR/const-promoted-opaque.rs:34:26
|
LL | let _: &'static _ = &FOO;
| ---------- ^^^ creates a temporary value which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
LL |
LL | }
| - temporary value is freed at the end of this statement

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0492, E0493, E0716.
For more information about an error, try `rustc --explain E0492`.
36 changes: 36 additions & 0 deletions tests/ui/consts/const-promoted-opaque.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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;

#[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;
//[string,atomic]~^ ERROR: destructor of `Foo` cannot be evaluated at compile-time
//[string,atomic]~| ERROR: temporary value dropped while borrowed
};

const BAZ: &Foo = &FOO;
//[string,atomic]~^ ERROR: constants cannot refer to interior mutable data

fn main() {
let _: &'static _ = &FOO;
//[string,atomic]~^ ERROR: temporary value dropped while borrowed
}
41 changes: 41 additions & 0 deletions tests/ui/consts/const-promoted-opaque.string.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
error[E0493]: destructor of `Foo` cannot be evaluated at compile-time
--> $DIR/const-promoted-opaque.rs:25:26
|
LL | let _: &'static _ = &FOO;
| ^^^ the destructor for this type cannot be evaluated in constants
...
LL | };
| - value is dropped here

error[E0716]: temporary value dropped while borrowed
--> $DIR/const-promoted-opaque.rs:25:26
|
LL | let _: &'static _ = &FOO;
| ---------- ^^^ creates a temporary value which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
...
LL | };
| - temporary value is freed at the end of this statement

error[E0492]: constants cannot refer to interior mutable data
--> $DIR/const-promoted-opaque.rs:30:19
|
LL | const BAZ: &Foo = &FOO;
| ^^^^ this borrow of an interior mutable value may end up in the final value

error[E0716]: temporary value dropped while borrowed
--> $DIR/const-promoted-opaque.rs:34:26
|
LL | let _: &'static _ = &FOO;
| ---------- ^^^ creates a temporary value which is freed while still in use
| |
| type annotation requires that borrow lasts for `'static`
LL |
LL | }
| - temporary value is freed at the end of this statement

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0492, E0493, E0716.
For more information about an error, try `rustc --explain E0492`.