Skip to content
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

reimplement ~const Trait bounds via a fourth kind of generic param #101900

Closed
wants to merge 13 commits into from
Closed
21 changes: 16 additions & 5 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@ use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
use rustc_target::abi::VariantIdx;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{
type_known_to_meet_bound_modulo_regions, Obligation, ObligationCause,
pred_known_to_hold_modulo_regions, Obligation, ObligationCause,
};

use super::borrow_set::BorrowData;
@@ -1075,12 +1075,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let suggest = match tcx.get_diagnostic_item(sym::IntoIterator) {
Some(def_id) => {
let infcx = self.infcx.tcx.infer_ctxt().build();
type_known_to_meet_bound_modulo_regions(
pred_known_to_hold_modulo_regions(
&infcx,
self.param_env,
tcx.mk_imm_ref(tcx.lifetimes.re_erased, tcx.erase_regions(ty)),
def_id,
DUMMY_SP,
ty::Binder::dummy(
tcx.mk_trait_ref(
def_id,
[tcx.mk_imm_ref(
tcx.lifetimes.re_erased,
tcx.erase_regions(ty),
)
.into()]
.into_iter()
.chain(tcx.host_effect()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.chain(tcx.host_effect()),
.chain(tcx.default_host_effect()),

or something like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

why are u using mk_trait_ref instead of mk_trait_ref_with_effect here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... I do get the naming confusing between this not returning the effect kind, but a value of the host effect... I'll try out a few namings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why are u using mk_trait_ref instead of mk_trait_ref_with_effect here?

for diagnostics I opted to just always fall back to host-on. Many times we don't have any context information available anyway

),
)
.without_const(),
span,
)
}
_ => false,
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
@@ -612,6 +612,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
(
GenericArgKind::Lifetime(_)
| GenericArgKind::Type(_)
| GenericArgKind::Effect(_)
| GenericArgKind::Const(_),
_,
) => {
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
@@ -382,6 +382,7 @@ fn check_opaque_type_parameter_valid(
matches!(*lt, ty::ReEarlyBound(_) | ty::ReFree(_))
}
GenericArgKind::Const(ct) => matches!(ct.kind(), ty::ConstKind::Param(_)),
GenericArgKind::Effect(e) => matches!(e.val, ty::EffectValue::Param { .. }),
};

if arg_is_param {
Original file line number Diff line number Diff line change
@@ -169,7 +169,7 @@ impl<'a, 'tcx> ConstraintConversion<'a, 'tcx> {
.type_must_outlive(origin, t1, r2, constraint_category);
}

GenericArgKind::Const(_) => unreachable!(),
GenericArgKind::Effect(_) | GenericArgKind::Const(_) => unreachable!(),
}
}

18 changes: 18 additions & 0 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
@@ -623,6 +623,9 @@ fn push_generic_params_internal<'tcx>(
GenericArgKind::Const(ct) => {
push_const_param(tcx, ct, output);
}
GenericArgKind::Effect(e) => {
push_effect_param(tcx, e, output);
}
other => bug!("Unexpected non-erasable generic: {:?}", other),
}

@@ -634,6 +637,21 @@ fn push_generic_params_internal<'tcx>(
true
}

fn push_effect_param<'tcx>(_tcx: TyCtxt<'tcx>, e: ty::Effect<'tcx>, output: &mut String) {
match e.kind {
ty::EffectKind::Host => match e.val {
// nothing to add, the host effect is our default symbol
ty::EffectValue::Rigid { on: true } => {}
ty::EffectValue::Rigid { on: false } => write!(output, "const").unwrap(),
ty::EffectValue::Param { index } => write!(output, "~const<{index}>").unwrap(),
ty::EffectValue::Infer(_)
| ty::EffectValue::Bound(_, _)
| ty::EffectValue::Placeholder(_)
| ty::EffectValue::Err(_) => unreachable!(),
},
};
}

fn push_const_param<'tcx>(tcx: TyCtxt<'tcx>, ct: ty::Const<'tcx>, output: &mut String) {
match ct.kind() {
ty::ConstKind::Param(param) => {
28 changes: 16 additions & 12 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ use rustc_hir::{LangItem, CRATE_HIR_ID};
use rustc_middle::mir;
use rustc_middle::mir::interpret::PointerArithmetic;
use rustc_middle::ty::layout::FnAbiOf;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_session::lint::builtin::INVALID_ALIGNMENT;
use std::borrow::Borrow;
use std::hash::Hash;
@@ -202,14 +202,16 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
} else if Some(def_id) == self.tcx.lang_items().panic_fmt() {
// For panic_fmt, call const_panic_fmt instead.
let const_def_id = self.tcx.require_lang_item(LangItem::ConstPanicFmt, None);
let new_instance = ty::Instance::resolve(
*self.tcx,
ty::ParamEnv::reveal_all(),
const_def_id,
instance.substs,
)
.unwrap()
.unwrap();
let substs = if self.tcx.effects() {
self.tcx
.mk_substs(instance.substs.iter().chain([self.tcx.effects.always_const.into()]))
Copy link
Contributor

Choose a reason for hiding this comment

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

host_on or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I explicitly named it this way (it would be host_off fwiw) and I dislike inverses.

} else {
instance.substs
};
let new_instance =
ty::Instance::resolve(*self.tcx, ty::ParamEnv::reveal_all(), const_def_id, substs)
.unwrap()
.unwrap();

return Ok(Some(new_instance));
} else if Some(def_id) == self.tcx.lang_items().align_offset_fn() {
@@ -425,8 +427,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,

if new_instance != instance {
// We call another const fn instead.
// However, we return the *original* instance to make backtraces work out
// (and we hope this does not confuse the FnAbi checks too much).
return Ok(Self::find_mir_or_eval_fn(
ecx,
new_instance,
@@ -436,7 +436,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
ret,
_unwind,
)?
.map(|(body, _instance)| (body, instance)));
.map(|(body, new_instance)| {
// However, we return the *original* instance to make backtraces work out
// (and we hope this does not confuse the FnAbi checks too much).
(body, Instance::new(instance.def_id(), new_instance.substs))
}));
}
}

7 changes: 7 additions & 0 deletions compiler/rustc_const_eval/src/interpret/util.rs
Original file line number Diff line number Diff line change
@@ -61,6 +61,13 @@ where
_ => c.super_visit_with(self),
}
}

fn visit_effect(&mut self, e: ty::Effect<'tcx>) -> ControlFlow<Self::BreakTy> {
match e.val {
ty::EffectValue::Param { .. } => ControlFlow::Break(FoundParam),
_ => e.super_visit_with(self),
}
}
}

let mut vis = UsedParamsNeedSubstVisitor { tcx };
8 changes: 5 additions & 3 deletions compiler/rustc_const_eval/src/transform/check_consts/check.rs
Original file line number Diff line number Diff line change
@@ -344,9 +344,11 @@ impl<'mir, 'tcx> Checker<'mir, 'tcx> {
let ty = match ty.unpack() {
GenericArgKind::Type(ty) => ty,

// No constraints on lifetimes or constants, except potentially
// No constraints on lifetimes, effects or constants, except potentially
// constants' types, but `walk` will get to them as well.
GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => continue,
GenericArgKind::Effect(_)
| GenericArgKind::Lifetime(_)
| GenericArgKind::Const(_) => continue,
};

match *ty.kind() {
@@ -806,7 +808,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
return;
}
}
_ if !tcx.is_const_fn_raw(callee) => {
_ if !self.tcx.effects() && !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;
12 changes: 10 additions & 2 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
@@ -157,8 +157,16 @@ impl Qualif for NeedsNonConstDrop {
cx.tcx,
ObligationCause::dummy_with_span(cx.body.span),
cx.param_env,
ty::Binder::dummy(cx.tcx.at(cx.body.span).mk_trait_ref(LangItem::Destruct, [ty]))
.with_constness(ty::BoundConstness::ConstIfConst),
ty::Binder::dummy(cx.tcx.at(cx.body.span).mk_trait_ref_with_effect(
LangItem::Destruct,
[ty],
cx.def_id().to_def_id(),
))
.with_constness(if cx.tcx.effects() {
ty::BoundConstness::NotConst
} else {
ty::BoundConstness::ConstIfConst
}),
);

let infcx = cx.tcx.infer_ctxt().build();
5 changes: 5 additions & 0 deletions compiler/rustc_const_eval/src/util/type_name.rs
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> {
type Type = Self;
type DynExistential = Self;
type Const = Self;
type Effect = Self;

fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
@@ -71,6 +72,10 @@ impl<'tcx> Printer<'tcx> for AbsolutePathPrinter<'tcx> {
self.pretty_print_const(ct, false)
}

fn print_effect(self, _: ty::Effect<'tcx>) -> Result<Self::Effect, Self::Error> {
unimplemented!()
}

fn print_dyn_existential(
self,
predicates: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
4 changes: 2 additions & 2 deletions compiler/rustc_error_messages/locales/en-US/middle.ftl
Original file line number Diff line number Diff line change
@@ -32,5 +32,5 @@ middle_strict_coherence_needs_negative_coherence =
to use `strict_coherence` on this trait, the `with_negative_coherence` feature must be enabled
.label = due to this attribute

middle_const_not_used_in_type_alias =
const parameter `{$ct}` is part of concrete type but not used in parameter list for the `impl Trait` type alias
middle_param_not_used_in_type_alias =
parameter `{$param}` is part of concrete type but not used in parameter list for the `impl Trait` type alias
3 changes: 3 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1272,6 +1272,7 @@ impl HandlerInner {
}

// FIXME(eddyb) this should ideally take `diagnostic` by value.
#[track_caller]
fn emit_diagnostic(&mut self, diagnostic: &mut Diagnostic) -> Option<ErrorGuaranteed> {
// The `LintExpectationId` can be stable or unstable depending on when it was created.
// Diagnostics created before the definition of `HirId`s are unstable and can not yet
@@ -1540,6 +1541,7 @@ impl HandlerInner {
panic::panic_any(ExplicitBug);
}

#[track_caller]
fn emit_diag_at_span(&mut self, mut diag: Diagnostic, sp: impl Into<MultiSpan>) {
self.emit_diagnostic(diag.set_span(sp));
}
@@ -1653,6 +1655,7 @@ impl HandlerInner {
self.warn_count += 1;
}

#[track_caller]
fn panic_if_treat_err_as_bug(&self) {
if self.treat_err_as_bug() {
match (
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
@@ -384,6 +384,8 @@ declare_features! (
(active, doc_masked, "1.21.0", Some(44027), None),
/// Allows `dyn* Trait` objects.
(incomplete, dyn_star, "1.65.0", Some(102425), None),
// Uses generic effect parameters for ~const bounds
(active, effects, "1.62.0", Some(102090), None),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
(active, effects, "1.62.0", Some(102090), None),
(incomplete, effects, "1.62.0", Some(102090), None),

?

/// Allows `X..Y` patterns.
(active, exclusive_range_pattern, "1.11.0", Some(37854), None),
/// Allows exhaustive pattern matching on types that contain uninhabited types.
37 changes: 37 additions & 0 deletions compiler/rustc_hir/src/def.rs
Original file line number Diff line number Diff line change
@@ -264,6 +264,43 @@ impl DefKind {
| DefKind::ExternCrate => false,
}
}

pub fn require_const(self) -> bool {
match self {
DefKind::Const
| DefKind::Static(_)
| DefKind::AssocConst
| DefKind::AnonConst
| DefKind::InlineConst => true,
DefKind::Fn
| DefKind::AssocFn
| DefKind::Ctor(..)
| DefKind::Closure
| DefKind::Generator
| DefKind::Mod
| DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::Variant
| DefKind::Trait
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TraitAlias
| DefKind::AssocTy
| DefKind::Macro(..)
| DefKind::Use
| DefKind::ForeignMod
| DefKind::OpaqueTy
| DefKind::ImplTraitPlaceholder
| DefKind::Impl
| DefKind::Field
| DefKind::TyParam
| DefKind::ConstParam
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::ExternCrate => false,
}
}
}

/// The resolution of a path or export.
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
@@ -212,6 +212,7 @@ language_item_table! {
Fn, kw::Fn, fn_trait, Target::Trait, GenericRequirement::Exact(1);
FnMut, sym::fn_mut, fn_mut_trait, Target::Trait, GenericRequirement::Exact(1);
FnOnce, sym::fn_once, fn_once_trait, Target::Trait, GenericRequirement::Exact(1);
Callable, sym::callable, callable_trait, Target::Trait, GenericRequirement::Exact(1);

FnOnceOutput, sym::fn_once_output, fn_once_output, Target::AssocTy, GenericRequirement::None;

Loading