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

Do not try to reveal hidden types when trying to prove auto-traits in the defining scope #122192

Merged
merged 4 commits into from
Jul 25, 2024
Merged
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
28 changes: 27 additions & 1 deletion compiler/rustc_const_eval/src/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,33 @@ 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)
// Avoid selecting for simple cases, such as builtin types.
if ty.is_trivially_freeze() {
return false;
}

// We do not use `ty.is_freeze` here, because that requires revealing opaque types, which
// requires borrowck, which in turn will invoke mir_const_qualifs again, causing a cycle error.
// Instead we invoke an obligation context manually, and provide the opaque type inference settings
// that allow the trait solver to just error out instead of cycling.
let freeze_def_id = cx.tcx.require_lang_item(LangItem::Freeze, Some(cx.body.span));

let obligation = Obligation::new(
cx.tcx,
ObligationCause::dummy_with_span(cx.body.span),
cx.param_env,
ty::TraitRef::new(cx.tcx, freeze_def_id, [ty::GenericArg::from(ty)]),
);

let infcx = cx
.tcx
.infer_ctxt()
.with_opaque_type_inference(cx.body.source.def_id().expect_local())
.build();
let ocx = ObligationCtxt::new(&infcx);
ocx.register_obligation(obligation);
let errors = ocx.select_all_or_error();
!errors.is_empty()
}

fn in_adt_inherently<'tcx>(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1268,7 +1268,7 @@ impl<'tcx> Ty<'tcx> {
///
/// Returning true means the type is known to be `Freeze`. Returning
/// `false` means nothing -- could be `Freeze`, might not be.
fn is_trivially_freeze(self) -> bool {
pub fn is_trivially_freeze(self) -> bool {
match self.kind() {
ty::Int(_)
| ty::Uint(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
);
}

ty::Alias(ty::Opaque, _) => {
ty::Alias(ty::Opaque, alias) => {
if candidates.vec.iter().any(|c| matches!(c, ProjectionCandidate(_))) {
// We do not generate an auto impl candidate for `impl Trait`s which already
// reference our auto trait.
Expand All @@ -787,6 +787,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// We do not emit auto trait candidates for opaque types in coherence.
// Doing so can result in weird dependency cycles.
candidates.ambiguous = true;
} else if self.infcx.can_define_opaque_ty(alias.def_id) {
// We do not emit auto trait candidates for opaque types in their defining scope, as
// we need to know the hidden type first, which we can't reliably know within the defining
// scope.
candidates.ambiguous = true;
} else {
candidates.vec.push(AutoImplCandidate)
}
Expand Down
18 changes: 11 additions & 7 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2386,13 +2386,17 @@ impl<'tcx> SelectionContext<'_, 'tcx> {
}

ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => {
// We can resolve the `impl Trait` to its concrete type,
// which enforces a DAG between the functions requiring
// the auto trait bounds in question.
match self.tcx().type_of_opaque(def_id) {
Ok(ty) => t.rebind(vec![ty.instantiate(self.tcx(), args)]),
Err(_) => {
return Err(SelectionError::OpaqueTypeAutoTraitLeakageUnknown(def_id));
if self.infcx.can_define_opaque_ty(def_id) {
Copy link
Contributor

@lcnr lcnr Jun 4, 2024

Choose a reason for hiding this comment

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

this applies to all auto traits, not just Freeze, so it can impact selection in an incomplete way. I feel a bit uncomfortable about this, as it will break in the new solver due to ambiguity. Can you explicitly limit this to Freeze and add the following as a test (with a revision for next solver)

fn is_trait<T: Trait<U>, U: Default>(_: T) -> U { Default::default() }

trait Trait<T> {}
impl<T: Send> Trait<u32> for T {}
impl<T> Trait<i32> for T {}
fn foo() -> impl Sized {
    if false {
        is_trait(foo())
    } else {
        Default::default()
    }
}

Copy link
Contributor

@lcnr lcnr Jun 4, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... we just recently made Freeze available for user defined bounds: #121675

Copy link
Contributor

Choose a reason for hiding this comment

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

hopefully the new solver is stable before freeeze is :3

alternatively: can you change this to force ambiguity instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did a thing, but it seems incomplete, I need to do some log analysis

Copy link
Contributor

Choose a reason for hiding this comment

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

can't do that in confirmnation, you have to change assemble_candidates_from_auto_impls instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

argml right. Old solver -.-

unreachable!()
} else {
// We can resolve the `impl Trait` to its concrete type,
// which enforces a DAG between the functions requiring
// the auto trait bounds in question.
match self.tcx().type_of_opaque(def_id) {
Ok(ty) => t.rebind(vec![ty.instantiate(self.tcx(), args)]),
Err(_) => {
return Err(SelectionError::OpaqueTypeAutoTraitLeakageUnknown(def_id));
}
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions tests/ui/const-generics/opaque_types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,6 @@ note: ...which requires const checking `main::{constant#0}`...
|
LL | foo::<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 opaque `Foo::{opaque#0}`, completing the cycle
note: cycle used when computing type of `Foo::{opaque#0}`
--> $DIR/opaque_types.rs:3:12
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/consts/const-fn-cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
/// 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>
Expand All @@ -18,7 +20,6 @@ where
}

pub const fn many<'s>(&'s self) -> Parser<impl for<'a> Fn(&'a str) -> Vec<T> + 's> {
//~^ ERROR: cycle detected
Parser::new(|_| unimplemented!())
}
}
Expand Down
34 changes: 0 additions & 34 deletions tests/ui/consts/const-fn-cycle.stderr

This file was deleted.

45 changes: 7 additions & 38 deletions tests/ui/consts/const-promoted-opaque.atomic.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0658]: cannot borrow here, since the borrowed element may contain interior mutability
--> $DIR/const-promoted-opaque.rs:29:25
--> $DIR/const-promoted-opaque.rs:28:25
|
LL | let _: &'static _ = &FOO;
| ^^^^
Expand All @@ -9,7 +9,7 @@ LL | let _: &'static _ = &FOO;
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0493]: destructor of `helper::Foo` cannot be evaluated at compile-time
--> $DIR/const-promoted-opaque.rs:29:26
--> $DIR/const-promoted-opaque.rs:28:26
|
LL | let _: &'static _ = &FOO;
| ^^^ the destructor for this type cannot be evaluated in constants
Expand All @@ -18,13 +18,13 @@ LL | };
| - value is dropped here

error[E0492]: constants cannot refer to interior mutable data
--> $DIR/const-promoted-opaque.rs:34:19
--> $DIR/const-promoted-opaque.rs:33: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:38:26
--> $DIR/const-promoted-opaque.rs:37:26
|
LL | let _: &'static _ = &FOO;
| ---------- ^^^ creates a temporary value which is freed while still in use
Expand All @@ -34,38 +34,7 @@ LL |
LL | }
| - temporary value is freed at the end of this statement

error[E0391]: cycle detected when computing type of opaque `helper::Foo::{opaque#0}`
--> $DIR/const-promoted-opaque.rs:14:20
|
LL | pub type Foo = impl Sized;
| ^^^^^^^^^^
|
note: ...which requires borrow-checking `helper::FOO`...
--> $DIR/const-promoted-opaque.rs:21:5
|
LL | pub const FOO: Foo = std::sync::atomic::AtomicU8::new(42);
| ^^^^^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `helper::FOO`...
--> $DIR/const-promoted-opaque.rs:21:5
|
LL | pub const FOO: Foo = std::sync::atomic::AtomicU8::new(42);
| ^^^^^^^^^^^^^^^^^^
note: ...which requires const checking `helper::FOO`...
--> $DIR/const-promoted-opaque.rs:21:5
|
LL | pub const FOO: Foo = std::sync::atomic::AtomicU8::new(42);
| ^^^^^^^^^^^^^^^^^^
= note: ...which requires computing whether `helper::Foo` is freeze...
= note: ...which requires evaluating trait selection obligation `helper::Foo: core::marker::Freeze`...
= note: ...which again requires computing type of opaque `helper::Foo::{opaque#0}`, completing the cycle
note: cycle used when computing type of `helper::Foo::{opaque#0}`
--> $DIR/const-promoted-opaque.rs:14:20
|
LL | pub type Foo = impl Sized;
| ^^^^^^^^^^
= 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: aborting due to 5 previous errors
error: aborting due to 4 previous errors

Some errors have detailed explanations: E0391, E0492, E0493, E0658, E0716.
For more information about an error, try `rustc --explain E0391`.
Some errors have detailed explanations: E0492, E0493, E0658, E0716.
For more information about an error, try `rustc --explain E0492`.
5 changes: 2 additions & 3 deletions tests/ui/consts/const-promoted-opaque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

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

#[cfg(string)]
pub const FOO: Foo = String::new();
Expand All @@ -28,11 +27,11 @@ use helper::*;
const BAR: () = {
let _: &'static _ = &FOO;
//[string,atomic]~^ ERROR: destructor of `helper::Foo` cannot be evaluated at compile-time
//[string,atomic]~| ERROR: cannot borrow here
//[atomic]~| ERROR: cannot borrow here
};

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

fn main() {
let _: &'static _ = &FOO;
Expand Down
57 changes: 5 additions & 52 deletions tests/ui/consts/const-promoted-opaque.string.stderr
Original file line number Diff line number Diff line change
@@ -1,30 +1,14 @@
error[E0658]: cannot borrow here, since the borrowed element may contain interior mutability
--> $DIR/const-promoted-opaque.rs:29:25
|
LL | let _: &'static _ = &FOO;
| ^^^^
|
= note: see issue #80384 <https://github.com/rust-lang/rust/issues/80384> for more information
= help: add `#![feature(const_refs_to_cell)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0493]: destructor of `helper::Foo` cannot be evaluated at compile-time
--> $DIR/const-promoted-opaque.rs:29:26
--> $DIR/const-promoted-opaque.rs:28:26
|
LL | let _: &'static _ = &FOO;
| ^^^ the destructor for this type cannot be evaluated in constants
...
LL | };
| - value is dropped here

error[E0492]: constants cannot refer to interior mutable data
--> $DIR/const-promoted-opaque.rs:34: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:38:26
--> $DIR/const-promoted-opaque.rs:37:26
|
LL | let _: &'static _ = &FOO;
| ---------- ^^^ creates a temporary value which is freed while still in use
Expand All @@ -34,38 +18,7 @@ LL |
LL | }
| - temporary value is freed at the end of this statement

error[E0391]: cycle detected when computing type of opaque `helper::Foo::{opaque#0}`
--> $DIR/const-promoted-opaque.rs:14:20
|
LL | pub type Foo = impl Sized;
| ^^^^^^^^^^
|
note: ...which requires borrow-checking `helper::FOO`...
--> $DIR/const-promoted-opaque.rs:18:5
|
LL | pub const FOO: Foo = String::new();
| ^^^^^^^^^^^^^^^^^^
note: ...which requires promoting constants in MIR for `helper::FOO`...
--> $DIR/const-promoted-opaque.rs:18:5
|
LL | pub const FOO: Foo = String::new();
| ^^^^^^^^^^^^^^^^^^
note: ...which requires const checking `helper::FOO`...
--> $DIR/const-promoted-opaque.rs:18:5
|
LL | pub const FOO: Foo = String::new();
| ^^^^^^^^^^^^^^^^^^
= note: ...which requires computing whether `helper::Foo` is freeze...
= note: ...which requires evaluating trait selection obligation `helper::Foo: core::marker::Freeze`...
= note: ...which again requires computing type of opaque `helper::Foo::{opaque#0}`, completing the cycle
note: cycle used when computing type of `helper::Foo::{opaque#0}`
--> $DIR/const-promoted-opaque.rs:14:20
|
LL | pub type Foo = impl Sized;
| ^^^^^^^^^^
= 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: aborting due to 5 previous errors
error: aborting due to 2 previous errors

Some errors have detailed explanations: E0391, E0492, E0493, E0658, E0716.
For more information about an error, try `rustc --explain E0391`.
Some errors have detailed explanations: E0493, E0716.
For more information about an error, try `rustc --explain E0493`.
22 changes: 22 additions & 0 deletions tests/ui/impl-trait/auto-trait-selection-freeze.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0283]: type annotations needed
--> $DIR/auto-trait-selection-freeze.rs:19:16
|
LL | if false { is_trait(foo()) } else { Default::default() }
| ^^^^^^^^ ----- type must be known at this point
| |
| cannot infer type of the type parameter `T` declared on the function `is_trait`
|
= note: cannot satisfy `_: Trait<_>`
note: required by a bound in `is_trait`
--> $DIR/auto-trait-selection-freeze.rs:11:16
|
LL | fn is_trait<T: Trait<U>, U: Default>(_: T) -> U {
| ^^^^^^^^ required by this bound in `is_trait`
help: consider specifying the generic arguments
|
LL | if false { is_trait::<T, U>(foo()) } else { Default::default() }
| ++++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0283`.
26 changes: 26 additions & 0 deletions tests/ui/impl-trait/auto-trait-selection-freeze.old.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
error[E0283]: type annotations needed
--> $DIR/auto-trait-selection-freeze.rs:19:16
|
LL | if false { is_trait(foo()) } else { Default::default() }
| ^^^^^^^^ cannot infer type of the type parameter `U` declared on the function `is_trait`
|
note: multiple `impl`s satisfying `impl Sized: Trait<_>` found
--> $DIR/auto-trait-selection-freeze.rs:16:1
|
LL | impl<T: Freeze> Trait<u32> for T {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | impl<T> Trait<i32> for T {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
note: required by a bound in `is_trait`
--> $DIR/auto-trait-selection-freeze.rs:11:16
|
LL | fn is_trait<T: Trait<U>, U: Default>(_: T) -> U {
| ^^^^^^^^ required by this bound in `is_trait`
help: consider specifying the generic arguments
|
LL | if false { is_trait::<_, U>(foo()) } else { Default::default() }
| ++++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0283`.
23 changes: 23 additions & 0 deletions tests/ui/impl-trait/auto-trait-selection-freeze.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//! This test shows how we fail selection in a way that can influence
//! selection in a code path that succeeds.

//@ revisions: next old
//@[next] compile-flags: -Znext-solver

#![feature(freeze)]

use std::marker::Freeze;

fn is_trait<T: Trait<U>, U: Default>(_: T) -> U {
Default::default()
}

trait Trait<T> {}
impl<T: Freeze> Trait<u32> for T {}
impl<T> Trait<i32> for T {}
fn foo() -> impl Sized {
if false { is_trait(foo()) } else { Default::default() }
//~^ ERROR: type annotations needed
}

fn main() {}
Loading
Loading