Skip to content

Commit

Permalink
Auto merge of #119072 - fee1-dead-contrib:effects-fixes, r=compiler-e…
Browse files Browse the repository at this point in the history
…rrors

Clean up `check_consts` and misc fixes

1. Remove most of the logic around erroring with trait methods. I have kept the part resolving it to a concrete impl, as that is used for const stability checks.
2. Turning on `effects` causes ICE with generic args, due to `~const Tr` when `Tr` is not `#[const_trait]` tripping up expectation in code that handles generic args, more specifically here:
https://github.com/rust-lang/rust/blob/8681e077b8afa99d60acf8f8470a012a3ce709a5/compiler/rustc_hir_analysis/src/astconv/generics.rs#L377

We set `arg_count.correct` to `Err` to correctly signal that an error has already been reported.

3. UI test blesses.

Edit(fmease): Fixes #117244 (UI test is in #119099 for now).

r? compiler-errors
  • Loading branch information
bors committed Dec 23, 2023
2 parents c03d978 + df1a4c6 commit edcbcc7
Show file tree
Hide file tree
Showing 37 changed files with 176 additions and 299 deletions.
187 changes: 40 additions & 147 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@ use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_index::bit_set::BitSet;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::{ImplSource, Obligation, ObligationCause};
use rustc_infer::traits::ObligationCause;
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::traits::BuiltinImplSource;
use rustc_middle::ty::GenericArgs;
use rustc_middle::ty::{self, adjustment::PointerCoercion, Instance, InstanceDef, Ty, TyCtxt};
use rustc_middle::ty::{TraitRef, TypeVisitableExt};
use rustc_middle::ty::{self, adjustment::PointerCoercion, Ty, TyCtxt};
use rustc_middle::ty::{Instance, InstanceDef, TypeVisitableExt};
use rustc_mir_dataflow::Analysis;
use rustc_span::{sym, Span, Symbol};
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt, SelectionContext};
use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt};
use rustc_type_ir::visit::{TypeSuperVisitable, TypeVisitor};

use std::mem;
Expand Down Expand Up @@ -756,143 +754,43 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
infcx.err_ctxt().report_fulfillment_errors(errors);
}

let mut is_trait = false;
// Attempting to call a trait method?
// FIXME(effects) do we need this?
if let Some(trait_id) = tcx.trait_of_item(callee) {
if tcx.trait_of_item(callee).is_some() {
trace!("attempting to call a trait method");
if !self.tcx.features().const_trait_impl {
// trait method calls are only permitted when `effects` is enabled.
// we don't error, since that is handled by typeck. We try to resolve
// the trait into the concrete method, and uses that for const stability
// checks.
// FIXME(effects) we might consider moving const stability checks to typeck as well.
if tcx.features().effects {
is_trait = true;

if let Ok(Some(instance)) =
Instance::resolve(tcx, param_env, callee, fn_args)
&& let InstanceDef::Item(def) = instance.def
{
// Resolve a trait method call to its concrete implementation, which may be in a
// `const` trait impl. This is only used for the const stability check below, since
// we want to look at the concrete impl's stability.
fn_args = instance.args;
callee = def;
}
} else {
self.check_op(ops::FnCallNonConst {
caller,
callee,
args: fn_args,
span: *fn_span,
call_source: *call_source,
feature: Some(sym::const_trait_impl),
feature: Some(if tcx.features().const_trait_impl {
sym::effects
} else {
sym::const_trait_impl
}),
});
return;
}

let trait_ref = TraitRef::from_method(tcx, trait_id, fn_args);
let obligation =
Obligation::new(tcx, ObligationCause::dummy(), param_env, trait_ref);

let implsrc = {
let infcx = tcx.infer_ctxt().build();
let mut selcx = SelectionContext::new(&infcx);
selcx.select(&obligation)
};

match implsrc {
Ok(Some(ImplSource::Param(_))) if tcx.features().effects => {
debug!(
"const_trait_impl: provided {:?} via where-clause in {:?}",
trait_ref, param_env
);
return;
}
// Closure: Fn{Once|Mut}
Ok(Some(ImplSource::Builtin(BuiltinImplSource::Misc, _)))
if trait_ref.self_ty().is_closure()
&& tcx.fn_trait_kind_from_def_id(trait_id).is_some() =>
{
let ty::Closure(closure_def_id, fn_args) = *trait_ref.self_ty().kind()
else {
unreachable!()
};
if !tcx.is_const_fn_raw(closure_def_id) {
self.check_op(ops::FnCallNonConst {
caller,
callee,
args: fn_args,
span: *fn_span,
call_source: *call_source,
feature: None,
});

return;
}
}
Ok(Some(ImplSource::UserDefined(data))) => {
let callee_name = tcx.item_name(callee);

if let hir::Constness::NotConst = tcx.constness(data.impl_def_id) {
self.check_op(ops::FnCallNonConst {
caller,
callee,
args: fn_args,
span: *fn_span,
call_source: *call_source,
feature: None,
});
return;
}

if let Some(&did) = tcx
.associated_item_def_ids(data.impl_def_id)
.iter()
.find(|did| tcx.item_name(**did) == callee_name)
{
// using internal args is ok here, since this is only
// used for the `resolve` call below
fn_args = GenericArgs::identity_for_item(tcx, did);
callee = did;
}
}
_ if !tcx.is_const_fn_raw(callee) => {
// At this point, it is only legal when the caller is in a trait
// marked with #[const_trait], and the callee is in the same trait.
let mut nonconst_call_permission = false;
if let Some(callee_trait) = tcx.trait_of_item(callee)
&& tcx.has_attr(callee_trait, sym::const_trait)
&& Some(callee_trait) == tcx.trait_of_item(caller.to_def_id())
// Can only call methods when it's `<Self as TheTrait>::f`.
&& tcx.types.self_param == fn_args.type_at(0)
{
nonconst_call_permission = true;
}

if !nonconst_call_permission {
let obligation = Obligation::new(
tcx,
ObligationCause::dummy_with_span(*fn_span),
param_env,
trait_ref,
);

// improve diagnostics by showing what failed. Our requirements are stricter this time
// as we are going to error again anyways.
let infcx = tcx.infer_ctxt().build();
if let Err(e) = implsrc {
infcx.err_ctxt().report_selection_error(
obligation.clone(),
&obligation,
&e,
);
}

self.check_op(ops::FnCallNonConst {
caller,
callee,
args: fn_args,
span: *fn_span,
call_source: *call_source,
feature: None,
});
return;
}
}
_ => {}
}

// Resolve a trait method call to its concrete implementation, which may be in a
// `const` trait impl.
let instance = Instance::resolve(tcx, param_env, callee, fn_args);
debug!("Resolving ({:?}) -> {:?}", callee, instance);
if let Ok(Some(func)) = instance {
if let InstanceDef::Item(def) = func.def {
callee = def;
}
}
}

// At this point, we are calling a function, `callee`, whose `DefId` is known...
Expand Down Expand Up @@ -925,21 +823,16 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
return;
}

if !tcx.is_const_fn_raw(callee) {
if !tcx.is_const_default_method(callee) {
// To get to here we must have already found a const impl for the
// trait, but for it to still be non-const can be that the impl is
// using default method bodies.
self.check_op(ops::FnCallNonConst {
caller,
callee,
args: fn_args,
span: *fn_span,
call_source: *call_source,
feature: None,
});
return;
}
if !tcx.is_const_fn_raw(callee) && !is_trait {
self.check_op(ops::FnCallNonConst {
caller,
callee,
args: fn_args,
span: *fn_span,
call_source: *call_source,
feature: None,
});
return;
}

// If the `const fn` we are trying to call is not const-stable, ensure that we have
Expand Down
17 changes: 9 additions & 8 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
assert!(self_ty.is_none());
}

let arg_count = check_generic_arg_count(
let mut arg_count = check_generic_arg_count(
tcx,
span,
def_id,
Expand Down Expand Up @@ -560,6 +560,14 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
inferred_params: vec![],
infer_args,
};
if let ty::BoundConstness::ConstIfConst = constness
&& generics.has_self
&& !tcx.has_attr(def_id, sym::const_trait)
{
let e = tcx.sess.emit_err(crate::errors::ConstBoundForNonConstTrait { span });
arg_count.correct =
Err(GenericArgCountMismatch { reported: Some(e), invalid_args: vec![] });
}
let args = create_args_for_parent_generic_args(
tcx,
def_id,
Expand All @@ -570,13 +578,6 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
&mut args_ctx,
);

if let ty::BoundConstness::ConstIfConst = constness
&& generics.has_self
&& !tcx.has_attr(def_id, sym::const_trait)
{
tcx.sess.emit_err(crate::errors::ConstBoundForNonConstTrait { span });
}

(args, arg_count)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/auxiliary/closure-in-foreign-crate.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![crate_type = "lib"]
#![feature(const_closures, const_trait_impl)]
#![feature(const_closures, const_trait_impl, effects)]
#![allow(incomplete_features)]

pub const fn test() {
Expand Down
29 changes: 6 additions & 23 deletions tests/ui/consts/const-float-classify.rs
Original file line number Diff line number Diff line change
@@ -1,41 +1,24 @@
// compile-flags: -Zmir-opt-level=0
// known-bug: #110395
// FIXME run-pass
// run-pass

#![feature(const_float_bits_conv)]
#![feature(const_float_classify)]
#![feature(const_trait_impl)]
#![feature(const_trait_impl, effects)]

// Don't promote
const fn nop<T>(x: T) -> T { x }

// FIXME(const-hack): replace with PartialEq
#[const_trait]
trait MyEq<T> {
fn eq(self, b: T) -> bool;
}

impl const MyEq<bool> for bool {
fn eq(self, b: bool) -> bool {
self == b
}
}

impl const MyEq<NonDet> for bool {
fn eq(self, _: NonDet) -> bool {
impl const PartialEq<NonDet> for bool {
fn eq(&self, _: &NonDet) -> bool {
true
}
}

const fn eq<A: ~const MyEq<B>, B>(x: A, y: B) -> bool {
x.eq(y)
}

macro_rules! const_assert {
($a:expr, $b:expr) => {
{
const _: () = assert!(eq($a, $b));
assert!(eq(nop($a), nop($b)));
const _: () = assert!($a == $b);
assert!(nop($a) == nop($b));
}
};
}
Expand Down
11 changes: 0 additions & 11 deletions tests/ui/consts/const-float-classify.stderr

This file was deleted.

2 changes: 2 additions & 0 deletions tests/ui/consts/const-try.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ note: impl defined here, but it is not `const`
LL | impl const Try for TryMe {
| ^^^^^^^^^^^^^^^^^^^^^^^^
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
= help: add `#![feature(effects)]` to the crate attributes to enable

error[E0015]: `?` cannot convert from residual of `TryMe` in constant functions
--> $DIR/const-try.rs:33:5
Expand All @@ -23,6 +24,7 @@ note: impl defined here, but it is not `const`
LL | impl const FromResidual<Error> for TryMe {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
= help: add `#![feature(effects)]` to the crate attributes to enable

error: aborting due to 2 previous errors

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const_cmp_type_id.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// known-bug: #110395
#![feature(const_type_id)]
#![feature(const_trait_impl)]
#![feature(const_trait_impl, effects)]

use std::any::TypeId;

Expand Down
Loading

0 comments on commit edcbcc7

Please sign in to comment.