Auto merge of rust-lang#119072 - fee1-dead-contrib:effects-fixes, r=compiler-errors

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 rust-lang#117244 (UI test is in rust-lang#119099 for now).

r? compiler-errors

Author: bors

compiler/rustc_const_eval/src/transform/check_consts/check.rs

@@ -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;
@@ -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...
@@ -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

compiler/rustc_hir_analysis/src/astconv/mod.rs

@@ -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,
@@ -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,
@@ -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)
     }
 

tests/ui/consts/auxiliary/closure-in-foreign-crate.rs

@@ -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() {

tests/ui/consts/const-float-classify.rs

@@ -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));
         }
     };
 }

tests/ui/consts/const-float-classify.stderr

@@ -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
@@ -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
 

tests/ui/consts/const-try.stderr

@@ -1,6 +1,6 @@
 // known-bug: #110395
 #![feature(const_type_id)]
-#![feature(const_trait_impl)]
+#![feature(const_trait_impl, effects)]
 
 use std::any::TypeId;