Skip to content

Fix PartialEq args when #[const_trait] is enabled #118379

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

Merged
merged 3 commits into from
Nov 30, 2023
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
39 changes: 3 additions & 36 deletions compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ use crate::errors;
use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_errors::{Applicability, Diagnostic, ErrorGuaranteed, StashKey};
use rustc_hir as hir;
use rustc_hir::def::{self, CtorKind, DefKind, Namespace, Res};
use rustc_hir::def::{self, CtorKind, Namespace, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::HirId;
use rustc_hir_analysis::autoderef::Autoderef;
use rustc_infer::{
infer,
Expand Down Expand Up @@ -373,7 +372,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> Ty<'tcx> {
let (fn_sig, def_id) = match *callee_ty.kind() {
ty::FnDef(def_id, args) => {
self.enforce_context_effects(call_expr.hir_id, call_expr.span, def_id, args);
self.enforce_context_effects(call_expr.span, def_id, args);
let fn_sig = self.tcx.fn_sig(def_id).instantiate(self.tcx, args);

// Unit testing: function items annotated with
Expand Down Expand Up @@ -770,7 +769,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
#[tracing::instrument(level = "debug", skip(self, span))]
pub(super) fn enforce_context_effects(
&self,
call_expr_hir: HirId,
span: Span,
callee_did: DefId,
callee_args: GenericArgsRef<'tcx>,
Expand All @@ -781,38 +779,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let generics = tcx.generics_of(callee_did);
let Some(host_effect_index) = generics.host_effect_index else { return };

// if the callee does have the param, we need to equate the param to some const
// value no matter whether the effects feature is enabled in the local crate,
// because inference will fail if we don't.
let mut host_always_on =
!tcx.features().effects || tcx.sess.opts.unstable_opts.unleash_the_miri_inside_of_you;

// Compute the constness required by the context.
let context = tcx.hir().enclosing_body_owner(call_expr_hir);
let const_context = tcx.hir().body_const_context(context);

let kind = tcx.def_kind(context.to_def_id());
debug_assert_ne!(kind, DefKind::ConstParam);

if tcx.has_attr(context.to_def_id(), sym::rustc_do_not_const_check) {
trace!("do not const check this context");
host_always_on = true;
}

let effect = match const_context {
_ if host_always_on => tcx.consts.true_,
Some(hir::ConstContext::Static(_) | hir::ConstContext::Const { .. }) => {
tcx.consts.false_
}
Some(hir::ConstContext::ConstFn) => {
let host_idx = tcx
.generics_of(context)
.host_effect_index
.expect("ConstContext::Maybe must have host effect param");
ty::GenericArgs::identity_for_item(tcx, context).const_at(host_idx)
}
None => tcx.consts.true_,
};
let effect = tcx.expected_const_effect_param_for_body(self.body_id);
Copy link
Member

Choose a reason for hiding this comment

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

we probably should have an assert here that self.body_id == tcx.hir().enclosing_body_owner(call_expr_hir)


trace!(?effect, ?generics, ?callee_args);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: Span,
method: MethodCallee<'tcx>,
) {
self.enforce_context_effects(hir_id, span, method.def_id, method.args);
self.enforce_context_effects(span, method.def_id, method.args);
self.write_resolution(hir_id, Ok((DefKind::AssocFn, method.def_id)));
self.write_args(hir_id, method.args);
}
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,12 +282,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: provided_arg.span,
});
} else {
self.enforce_context_effects(
provided_arg.hir_id,
provided_arg.span,
def_id,
args,
)
self.enforce_context_effects(provided_arg.span, def_id, args)
}
}
} else {
Expand Down
51 changes: 51 additions & 0 deletions compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,57 @@ impl<'tcx> TyCtxt<'tcx> {
// the language.
|| self.extern_crate(key.as_def_id()).is_some_and(|e| e.is_direct())
}

pub fn expected_const_effect_param_for_body(self, def_id: LocalDefId) -> ty::Const<'tcx> {
// if the callee does have the param, we need to equate the param to some const
// value no matter whether the effects feature is enabled in the local crate,
// because inference will fail if we don't.
let mut host_always_on =
!self.features().effects || self.sess.opts.unstable_opts.unleash_the_miri_inside_of_you;

// Compute the constness required by the context.
let const_context = self.hir().body_const_context(def_id);

let kind = self.def_kind(def_id);
debug_assert_ne!(kind, DefKind::ConstParam);

if self.has_attr(def_id, sym::rustc_do_not_const_check) {
trace!("do not const check this context");
host_always_on = true;
}

match const_context {
_ if host_always_on => self.consts.true_,
Some(hir::ConstContext::Static(_) | hir::ConstContext::Const { .. }) => {
self.consts.false_
}
Some(hir::ConstContext::ConstFn) => {
let host_idx = self
.generics_of(def_id)
.host_effect_index
.expect("ConstContext::Maybe must have host effect param");
ty::GenericArgs::identity_for_item(self, def_id).const_at(host_idx)
}
None => self.consts.true_,
}
}

/// Constructs generic args for an item, optionally appending a const effect param type
pub fn with_opt_const_effect_param(
self,
caller_def_id: LocalDefId,
callee_def_id: DefId,
args: impl IntoIterator<Item: Into<ty::GenericArg<'tcx>>>,
) -> ty::GenericArgsRef<'tcx> {
let generics = self.generics_of(callee_def_id);
assert_eq!(generics.parent, None);

let opt_const_param = generics.host_effect_index.is_some().then(|| {
ty::GenericArg::from(self.expected_const_effect_param_for_body(caller_def_id))
});

self.mk_args_from_iter(args.into_iter().map(|arg| arg.into()).chain(opt_const_param))
}
}

struct OpaqueTypeExpander<'tcx> {
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_mir_build/src/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}

let eq_def_id = self.tcx.require_lang_item(LangItem::PartialEq, Some(source_info.span));
let method = trait_method(self.tcx, eq_def_id, sym::eq, [ty, ty]);
let method = trait_method(
self.tcx,
eq_def_id,
sym::eq,
self.tcx.with_opt_const_effect_param(self.def_id, eq_def_id, [ty, ty]),
);

let bool_ty = self.tcx.types.bool;
let eq_result = self.temp(bool_ty, source_info.span);
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,18 +258,26 @@ impl<'tcx> ConstToPat<'tcx> {

#[instrument(level = "trace", skip(self), ret)]
fn type_has_partial_eq_impl(&self, ty: Ty<'tcx>) -> bool {
let tcx = self.tcx();
// double-check there even *is* a semantic `PartialEq` to dispatch to.
//
// (If there isn't, then we can safely issue a hard
// error, because that's never worked, due to compiler
// using `PartialEq::eq` in this scenario in the past.)
let tcx = self.tcx();
let partial_eq_trait_id = tcx.require_lang_item(hir::LangItem::PartialEq, Some(self.span));
let partial_eq_obligation = Obligation::new(
tcx,
ObligationCause::dummy(),
self.param_env,
ty::TraitRef::new(tcx, partial_eq_trait_id, [ty, ty]),
ty::TraitRef::new(
tcx,
partial_eq_trait_id,
tcx.with_opt_const_effect_param(
tcx.hir().enclosing_body_owner(self.id),
partial_eq_trait_id,
[ty, ty],
),
),
);

// This *could* accept a type that isn't actually `PartialEq`, because region bounds get
Expand Down
11 changes: 10 additions & 1 deletion tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,14 +336,23 @@ fn from_str(s: &str) -> Result<bool, ()> {
}

#[lang = "eq"]
// FIXME #[const_trait]
#[const_trait]
trait PartialEq<Rhs: ?Sized = Self> {
fn eq(&self, other: &Rhs) -> bool;
fn ne(&self, other: &Rhs) -> bool {
!self.eq(other)
}
}

impl<A: ?Sized, B: ?Sized> const PartialEq<&B> for &A
where
A: ~const PartialEq<B>,
{
fn eq(&self, other: &&B) -> bool {
PartialEq::eq(*self, *other)
}
}

// FIXME(effects): again, this should not error without Rhs specified
impl PartialEq<str> for str {
fn eq(&self, other: &str) -> bool {
Expand Down
47 changes: 3 additions & 44 deletions tests/ui/rfcs/rfc-2632-const-trait-impl/effects/minicore.stderr
Original file line number Diff line number Diff line change
@@ -1,24 +1,5 @@
warning: to use a constant of type `&str` in a pattern, the type must implement `PartialEq`
--> $DIR/minicore.rs:332:9
|
LL | "true" => Ok(true),
| ^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
= note: `#[warn(const_patterns_without_partial_eq)]` on by default

warning: to use a constant of type `&str` in a pattern, the type must implement `PartialEq`
--> $DIR/minicore.rs:333:9
|
LL | "false" => Ok(false),
| ^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>

error[E0493]: destructor of `Self` cannot be evaluated at compile-time
--> $DIR/minicore.rs:494:9
--> $DIR/minicore.rs:503:9
|
LL | *self = source.clone()
| ^^^^^
Expand All @@ -27,35 +8,13 @@ LL | *self = source.clone()
| value is dropped here

error[E0493]: destructor of `T` cannot be evaluated at compile-time
--> $DIR/minicore.rs:504:35
--> $DIR/minicore.rs:513:35
|
LL | const fn drop<T: ~const Destruct>(_: T) {}
| ^ - value is dropped here
| |
| the destructor for this type cannot be evaluated in constant functions

error: aborting due to 2 previous errors; 2 warnings emitted
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0493`.
Future incompatibility report: Future breakage diagnostic:
warning: to use a constant of type `&str` in a pattern, the type must implement `PartialEq`
--> $DIR/minicore.rs:332:9
|
LL | "true" => Ok(true),
| ^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
= note: `#[warn(const_patterns_without_partial_eq)]` on by default

Future breakage diagnostic:
warning: to use a constant of type `&str` in a pattern, the type must implement `PartialEq`
--> $DIR/minicore.rs:333:9
|
LL | "false" => Ok(false),
| ^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #116122 <https://github.com/rust-lang/rust/issues/116122>
= note: `#[warn(const_patterns_without_partial_eq)]` on by default