-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Double-check conditional constness in MIR #132276
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,6 @@ use rustc_hir::def_id::DefId; | |
use rustc_hir::{self as hir, LangItem}; | ||
use rustc_index::bit_set::BitSet; | ||
use rustc_infer::infer::TyCtxtInferExt; | ||
use rustc_infer::traits::ObligationCause; | ||
use rustc_middle::mir::visit::Visitor; | ||
use rustc_middle::mir::*; | ||
use rustc_middle::span_bug; | ||
|
@@ -20,9 +19,11 @@ use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TypeVisitableExt}; | |
use rustc_mir_dataflow::Analysis; | ||
use rustc_mir_dataflow::impls::MaybeStorageLive; | ||
use rustc_mir_dataflow::storage::always_storage_live_locals; | ||
use rustc_span::{DUMMY_SP, Span, Symbol, sym}; | ||
use rustc_span::{Span, Symbol, sym}; | ||
use rustc_trait_selection::error_reporting::InferCtxtErrorExt; | ||
use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt}; | ||
use rustc_trait_selection::traits::{ | ||
Obligation, ObligationCause, ObligationCauseCode, ObligationCtxt, | ||
}; | ||
use tracing::{debug, instrument, trace}; | ||
|
||
use super::ops::{self, NonConstOp, Status}; | ||
|
@@ -360,6 +361,73 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> { | |
// end of evaluation. | ||
!is_transient | ||
} | ||
|
||
fn revalidate_conditional_constness( | ||
&mut self, | ||
callee: DefId, | ||
callee_args: ty::GenericArgsRef<'tcx>, | ||
call_source: CallSource, | ||
call_span: Span, | ||
) { | ||
let tcx = self.tcx; | ||
if !tcx.is_conditionally_const(callee) { | ||
return; | ||
} | ||
|
||
let const_conditions = tcx.const_conditions(callee).instantiate(tcx, callee_args); | ||
// If there are any const conditions on this fn and `const_trait_impl` | ||
// is not enabled, simply bail. We shouldn't be able to call conditionally | ||
// const functions on stable. | ||
if !const_conditions.is_empty() && !tcx.features().const_trait_impl() { | ||
self.check_op(ops::FnCallNonConst { | ||
callee, | ||
args: callee_args, | ||
span: call_span, | ||
call_source, | ||
feature: Some(sym::const_trait_impl), | ||
}); | ||
return; | ||
} | ||
|
||
let infcx = tcx.infer_ctxt().build(self.body.typing_mode(tcx)); | ||
let ocx = ObligationCtxt::new_with_diagnostics(&infcx); | ||
|
||
let body_id = self.body.source.def_id().expect_local(); | ||
let host_polarity = match self.const_kind() { | ||
hir::ConstContext::ConstFn => ty::BoundConstness::Maybe, | ||
hir::ConstContext::Static(_) | hir::ConstContext::Const { .. } => { | ||
ty::BoundConstness::Const | ||
} | ||
}; | ||
let const_conditions = ocx.normalize( | ||
&ObligationCause::misc(call_span, body_id), | ||
self.param_env, | ||
const_conditions, | ||
); | ||
ocx.register_obligations(const_conditions.into_iter().map(|(trait_ref, span)| { | ||
Obligation::new( | ||
tcx, | ||
ObligationCause::new( | ||
call_span, | ||
body_id, | ||
ObligationCauseCode::WhereClause(callee, span), | ||
), | ||
self.param_env, | ||
trait_ref.to_host_effect_clause(tcx, host_polarity), | ||
) | ||
})); | ||
|
||
let errors = ocx.select_all_or_error(); | ||
if !errors.is_empty() { | ||
// FIXME(effects): Soon this should be unconditionally delaying a bug. | ||
if matches!(call_source, CallSource::Normal) && tcx.features().effects() { | ||
tcx.dcx() | ||
.span_delayed_bug(call_span, "this should have reported a ~const error in HIR"); | ||
} else { | ||
infcx.err_ctxt().report_fulfillment_errors(errors); | ||
} | ||
} | ||
} | ||
} | ||
|
||
impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { | ||
|
@@ -566,7 +634,6 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { | |
}; | ||
|
||
let ConstCx { tcx, body, param_env, .. } = *self.ccx; | ||
let caller = self.def_id(); | ||
|
||
let fn_ty = func.ty(body, tcx); | ||
|
||
|
@@ -584,31 +651,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { | |
} | ||
}; | ||
|
||
// Check that all trait bounds that are marked as `~const` can be satisfied. | ||
// | ||
// Typeck only does a "non-const" check since it operates on HIR and cannot distinguish | ||
// which path expressions are getting called on and which path expressions are only used | ||
// as function pointers. This is required for correctness. | ||
let infcx = tcx.infer_ctxt().build(body.typing_mode(tcx)); | ||
let ocx = ObligationCtxt::new_with_diagnostics(&infcx); | ||
|
||
let predicates = tcx.predicates_of(callee).instantiate(tcx, fn_args); | ||
let cause = ObligationCause::new( | ||
terminator.source_info.span, | ||
self.body.source.def_id().expect_local(), | ||
ObligationCauseCode::WhereClause(callee, DUMMY_SP), | ||
); | ||
let normalized_predicates = ocx.normalize(&cause, param_env, predicates); | ||
ocx.register_obligations(traits::predicates_for_generics( | ||
|_, _| cause.clone(), | ||
self.param_env, | ||
normalized_predicates, | ||
)); | ||
|
||
let errors = ocx.select_all_or_error(); | ||
if !errors.is_empty() { | ||
infcx.err_ctxt().report_fulfillment_errors(errors); | ||
} | ||
self.revalidate_conditional_constness(callee, fn_args, call_source, *fn_span); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am confused, why is this called outside the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes? We can put ~const bounds on free const functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think you're getting too stuck on the naming of the The feature gate enables the declaration of I don't want to rename the feature gate, since that basically is just churn. the feature gate could be called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a poorly chosen name. :( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤷 I'm sorry, but I didn't choose it, and I'm not really interested in doing a rename here. That can be done later, and ideally not when there are like 3 other PRs in flight concerning it. |
||
|
||
let mut is_trait = false; | ||
// Attempting to call a trait method? | ||
|
@@ -648,7 +691,6 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { | |
None | ||
}; | ||
self.check_op(ops::FnCallNonConst { | ||
caller, | ||
callee, | ||
args: fn_args, | ||
span: *fn_span, | ||
|
@@ -738,7 +780,6 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> { | |
// Trait functions are not `const fn` so we have to skip them here. | ||
if !tcx.is_const_fn(callee) && !is_trait { | ||
self.check_op(ops::FnCallNonConst { | ||
caller, | ||
callee, | ||
args: fn_args, | ||
span: *fn_span, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
error[E0277]: the trait bound `(): ~const Tr` is not satisfied | ||
--> $DIR/default-method-body-is-const-body-checking.rs:12:9 | ||
| | ||
LL | foo::<()>(); | ||
| ^^^^^^^^^^^ | ||
| | ||
note: required by a bound in `foo` | ||
--> $DIR/default-method-body-is-const-body-checking.rs:7:28 | ||
| | ||
LL | const fn foo<T>() where T: ~const Tr {} | ||
| ^^^^^^ required by this bound in `foo` | ||
|
||
error: aborting due to 1 previous error | ||
|
||
For more information about this error, try `rustc --explain E0277`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should never call
tcx.features()
in this code, as that won't properly check forrustc_allow_const_fn_unstable
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the user still needs to enable the feature gate globally so that we can properly enforce the feature's validity earlier in the compiler.