Disable calling all const traits in stable const

[compiler-errors]

r? @RalfJung

I hesitate to make this a separate NonConstOp since its error reporting would be basically totally identical to the existing FnCallNonConst. Instead, if a trait is const, we set the status to Status::Unstable; otherwise it remains Status::Forbidden. Am I missing something about this not being sufficient?

[rustbot]

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

[RalfJung]

I hesitate to make this a separate NonConstOp since its error reporting would be basically totally identical to the existing FnCallNonConst

How that? It's checking a quite different thing, so the diagnostics should be rather different. So IMO it makes a lot more sense for this to be a new Op.

[compiler-errors]

so the diagnostics should be rather different.

I'm not certain what about the diagnostic would differ?

Out of the matrix of the feature gate being enabled (yes/no) and the trait being const (yes/no), in all cases we're basically just saying that it's not possible to call a trait method in a const context. And most of what goes into FnCallNonConst is describing what kind of thing is being called. While there's some machinery about missing ~const bounds that could be cut out, I basically don't want to just replicate this whole match statement twice:

rust/compiler/rustc_const_eval/src/check_consts/ops.rs

Lines 139 to 280 in 59cec72

	let mut err = match call_kind {
	CallKind::Normal { desugaring: Some((kind, self_ty)), .. } => {
	macro_rules! error {
	($err:ident) => {
	tcx.dcx().create_err(errors::$err {
	span,
	ty: self_ty,
	kind: ccx.const_kind(),
	})
	};
	}
	let mut err = match kind {
	CallDesugaringKind::ForLoopIntoIter => {
	error!(NonConstForLoopIntoIter)
	}
	CallDesugaringKind::QuestionBranch => {
	error!(NonConstQuestionBranch)
	}
	CallDesugaringKind::QuestionFromResidual => {
	error!(NonConstQuestionFromResidual)
	}
	CallDesugaringKind::TryBlockFromOutput => {
	error!(NonConstTryBlockFromOutput)
	}
	CallDesugaringKind::Await => {
	error!(NonConstAwait)
	}
	};
	diag_trait(&mut err, self_ty, kind.trait_def_id(tcx));
	err
	}
	CallKind::FnCall { fn_trait_id, self_ty } => {
	let note = match self_ty.kind() {
	FnDef(def_id, ..) => {
	let span = tcx.def_span(*def_id);
	if ccx.tcx.is_const_fn(*def_id) {
	span_bug!(span, "calling const FnDef errored when it shouldn't");
	}
	Some(errors::NonConstClosureNote::FnDef { span })
	}
	FnPtr(..) => Some(errors::NonConstClosureNote::FnPtr),
	Closure(..) => Some(errors::NonConstClosureNote::Closure),
	_ => None,
	};
	let mut err = tcx.dcx().create_err(errors::NonConstClosure {
	span,
	kind: ccx.const_kind(),
	note,
	});
	diag_trait(&mut err, self_ty, fn_trait_id);
	err
	}
	CallKind::Operator { trait_id, self_ty, .. } => {
	let mut err = if let CallSource::MatchCmp = call_source {
	tcx.dcx().create_err(errors::NonConstMatchEq {
	span,
	kind: ccx.const_kind(),
	ty: self_ty,
	})
	} else {
	let mut sugg = None;
	if ccx.tcx.is_lang_item(trait_id, LangItem::PartialEq) {
	match (args[0].unpack(), args[1].unpack()) {
	(GenericArgKind::Type(self_ty), GenericArgKind::Type(rhs_ty))
	if self_ty == rhs_ty
	&& self_ty.is_ref()
	&& self_ty.peel_refs().is_primitive() =>
	{
	let mut num_refs = 0;
	let mut tmp_ty = self_ty;
	while let rustc_middle::ty::Ref(_, inner_ty, _) = tmp_ty.kind() {
	num_refs += 1;
	tmp_ty = *inner_ty;
	}
	let deref = "*".repeat(num_refs);
	if let Ok(call_str) =
	ccx.tcx.sess.source_map().span_to_snippet(span)
	{
	if let Some(eq_idx) = call_str.find("==") {
	if let Some(rhs_idx) = call_str[(eq_idx + 2)..]
	.find(|c: char| !c.is_whitespace())
	{
	let rhs_pos = span.lo()
	+ BytePos::from_usize(eq_idx + 2 + rhs_idx);
	let rhs_span = span.with_lo(rhs_pos).with_hi(rhs_pos);
	sugg = Some(errors::ConsiderDereferencing {
	deref,
	span: span.shrink_to_lo(),
	rhs_span,
	});
	}
	}
	}
	}
	_ => {}
	}
	}
	tcx.dcx().create_err(errors::NonConstOperator {
	span,
	kind: ccx.const_kind(),
	sugg,
	})
	};
	diag_trait(&mut err, self_ty, trait_id);
	err
	}
	CallKind::DerefCoercion { deref_target, deref_target_ty, self_ty } => {
	// Check first whether the source is accessible (issue #87060)
	let target = if tcx.sess.source_map().is_span_accessible(deref_target) {
	Some(deref_target)
	} else {
	None
	};
	let mut err = tcx.dcx().create_err(errors::NonConstDerefCoercion {
	span,
	ty: self_ty,
	kind: ccx.const_kind(),
	target_ty: deref_target_ty,
	deref_target: target,
	});
	diag_trait(&mut err, self_ty, tcx.require_lang_item(LangItem::Deref, Some(span)));
	err
	}
	_ if tcx.opt_parent(callee) == tcx.get_diagnostic_item(sym::ArgumentMethods) => {
	ccx.dcx().create_err(errors::NonConstFmtMacroCall { span, kind: ccx.const_kind() })
	}
	_ => ccx.dcx().create_err(errors::NonConstFnCall {
	span,
	def_path_str: ccx.tcx.def_path_str_with_args(callee, args),
	kind: ccx.const_kind(),
	}),
	};

[RalfJung]

FnCallNonConst is for non-const functions. It just makes logically no sense to use it for "the function is const but there's a lang feature involved that is not enabled".

The entire idea of these "ops" is to have one Op per thing-that-needs-to-be-enabled. With const trait calls, there are two things that need to be enabled:

• the ability to do trait calls at all
• the specific feature gate for the trait being called (not sure if that's already implemented, we talked about having a per-trait control here eventually -- but that is not required for the first iteration)

I can see no reason at all for why one would want to merge these into a single check. That's just confusing.

[RalfJung]

This is confusing, it conflates two entirely orthogonal checks:

• is this trait const-callable
• are we allowed to do const trait calls

Why is the first check even here, didn't revalidate_conditional_constness already check that?

[compiler-errors]

revalidate_conditional_constness just checks that if the item is conditionally const then the trait bounds ~const trait bounds hold. For example, if a free function item is not conditionally const, then it will have no ~const trait bounds, so revalidate_conditional_constness is not enforcing anything in that case.

[RalfJung]

Suggested change

	// FIXME: apply more fine-grained per-trait const stability checks
	// (see <https://github.com/rust-lang/project-const-traits/issues/5>).

This is repeated below, no need to have it twice.

[RalfJung]

FWIW this will conflict with #132541

[RalfJung]

Something has gone very wrong here. This should complain about the missing feature gate -- const_trait_impl can't be used here since the function does not have #[rustc_const_unstable]. That's the entire point!

[compiler-errors]

I think you're misinterpreting what I did to this file lol. I marked the whole thing as a known-bug, which requires that the test have no error annotation, because this whole test is basically useless if we have no staged apis for const traits.

[RalfJung]

We need a test to ensure that we can't call trait functions on stable. Why shouldn't that be this file?

[compiler-errors]

Because this test does a lot more than just ensure we can't call trait functions on stable. If we want a test to do that, then it can be like... 10 lines. It's got like... several revisions, an aux-build dep that is also broken due to these changes, etc.

[RalfJung]

You need an aux build to properly check the cross-crate part of this. And the revisions distinguish whether the trait callee is stable or not, which the old check was sensitive to when it could resolve the callee... I guess for now we don't need the revisions; once we have the ability to do a per-trait decision of "can this be const-called on stable", that will be relevant again.

[RalfJung]

In fact I think the feature field should be removed from FnCallNonConst. That NonConstOp is meant for functions that genuinely are not const, and there can't be a feature gate for calling those.