Skip to content

Commit

Permalink
treat host effect params as erased generics in codegen
Browse files Browse the repository at this point in the history
This fixes the changes brought to codegen tests when effect params are
added to libcore, by not attempting to monomorphize functions that get
the host param by being `const fn`.
  • Loading branch information
fee1-dead committed Sep 14, 2023
1 parent 7e0261e commit 58c9946
Show file tree
Hide file tree
Showing 16 changed files with 70 additions and 40 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ pub fn get_fn<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, instance: Instance<'tcx>)
// whether we are sharing generics or not. The important thing here is
// that the visibility we apply to the declaration is the same one that
// has been applied to the definition (wherever that definition may be).
let is_generic = instance.args.non_erasable_generics().next().is_some();
let is_generic = instance.non_erasable_generics(tcx).next().is_some();

if is_generic {
// This is a monomorphization. Its expected visibility depends
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_llvm/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ pub fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) ->
unsafe {
llvm::LLVMRustSetLinkage(llfn, llvm::Linkage::ExternalLinkage);

let is_generic = instance.args.non_erasable_generics().next().is_some();
let is_generic =
instance.args.non_erasable_generics(tcx, instance.def_id()).next().is_some();

if is_generic {
// This is a monomorphization. Its expected visibility depends
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
type_names::push_generic_params(
tcx,
tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), args),
enclosing_fn_def_id,
&mut name,
);

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ fn exported_symbols_provider_local(

match *mono_item {
MonoItem::Fn(Instance { def: InstanceDef::Item(def), args }) => {
if args.non_erasable_generics().next().is_some() {
if args.non_erasable_generics(tcx, def).next().is_some() {
let symbol = ExportedSymbol::Generic(def, args);
symbols.push((
symbol,
Expand All @@ -346,10 +346,10 @@ fn exported_symbols_provider_local(
));
}
}
MonoItem::Fn(Instance { def: InstanceDef::DropGlue(_, Some(ty)), args }) => {
MonoItem::Fn(Instance { def: InstanceDef::DropGlue(def_id, Some(ty)), args }) => {
// A little sanity-check
debug_assert_eq!(
args.non_erasable_generics().next(),
args.non_erasable_generics(tcx, def_id).next(),
Some(GenericArgKind::Type(ty))
);
symbols.push((
Expand Down
36 changes: 24 additions & 12 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,14 @@ fn push_debuginfo_type_name<'tcx>(
ty_and_layout,
&|output, visited| {
push_item_name(tcx, def.did(), true, output);
push_generic_params_internal(tcx, args, output, visited);
push_generic_params_internal(tcx, args, def.did(), output, visited);
},
output,
visited,
);
} else {
push_item_name(tcx, def.did(), qualified, output);
push_generic_params_internal(tcx, args, output, visited);
push_generic_params_internal(tcx, args, def.did(), output, visited);
}
}
ty::Tuple(component_types) => {
Expand Down Expand Up @@ -237,8 +237,13 @@ fn push_debuginfo_type_name<'tcx>(
let principal =
tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), principal);
push_item_name(tcx, principal.def_id, qualified, output);
let principal_has_generic_params =
push_generic_params_internal(tcx, principal.args, output, visited);
let principal_has_generic_params = push_generic_params_internal(
tcx,
principal.args,
principal.def_id,
output,
visited,
);

let projection_bounds: SmallVec<[_; 4]> = trait_data
.projection_bounds()
Expand Down Expand Up @@ -516,7 +521,13 @@ pub fn compute_debuginfo_vtable_name<'tcx>(
tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), trait_ref);
push_item_name(tcx, trait_ref.def_id, true, &mut vtable_name);
visited.clear();
push_generic_params_internal(tcx, trait_ref.args, &mut vtable_name, &mut visited);
push_generic_params_internal(
tcx,
trait_ref.args,
trait_ref.def_id,
&mut vtable_name,
&mut visited,
);
} else {
vtable_name.push('_');
}
Expand Down Expand Up @@ -610,20 +621,20 @@ fn push_unqualified_item_name(
fn push_generic_params_internal<'tcx>(
tcx: TyCtxt<'tcx>,
args: GenericArgsRef<'tcx>,
def_id: DefId,
output: &mut String,
visited: &mut FxHashSet<Ty<'tcx>>,
) -> bool {
if args.non_erasable_generics().next().is_none() {
debug_assert_eq!(args, tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), args));
let mut args = args.non_erasable_generics(tcx, def_id).peekable();
if args.peek().is_none() {
return false;
}

debug_assert_eq!(args, tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), args));

let cpp_like_debuginfo = cpp_like_debuginfo(tcx);

output.push('<');

for type_parameter in args.non_erasable_generics() {
for type_parameter in args {
match type_parameter {
GenericArgKind::Type(type_parameter) => {
push_debuginfo_type_name(tcx, type_parameter, true, output, visited);
Expand Down Expand Up @@ -691,11 +702,12 @@ fn push_const_param<'tcx>(tcx: TyCtxt<'tcx>, ct: ty::Const<'tcx>, output: &mut S
pub fn push_generic_params<'tcx>(
tcx: TyCtxt<'tcx>,
args: GenericArgsRef<'tcx>,
def_id: DefId,
output: &mut String,
) {
let _prof = tcx.prof.generic_activity("compute_debuginfo_type_name");
let mut visited = FxHashSet::default();
push_generic_params_internal(tcx, args, output, &mut visited);
push_generic_params_internal(tcx, args, def_id, output, &mut visited);
}

fn push_closure_or_generator_name<'tcx>(
Expand Down Expand Up @@ -738,7 +750,7 @@ fn push_closure_or_generator_name<'tcx>(
// Truncate the args to the length of the above generics. This will cut off
// anything closure- or generator-specific.
let args = args.truncate_to(tcx, generics);
push_generic_params_internal(tcx, args, output, visited);
push_generic_params_internal(tcx, args, enclosing_fn_def_id, output, visited);
}

fn push_close_angle_bracket(cpp_like_debuginfo: bool, output: &mut String) {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1642,8 +1642,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
ValuePairs::Terms(infer::ExpectedFound { expected, found }) => {
match (expected.unpack(), found.unpack()) {
(ty::TermKind::Ty(expected), ty::TermKind::Ty(found)) => {
let is_simple_err =
expected.is_simple_text() && found.is_simple_text();
let is_simple_err = expected.is_simple_text(self.tcx)
&& found.is_simple_text(self.tcx);
OpaqueTypesVisitor::visit_expected_found(
self.tcx, expected, found, span,
)
Expand Down Expand Up @@ -1885,7 +1885,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}
s
};
if !(values.expected.is_simple_text() && values.found.is_simple_text())
if !(values.expected.is_simple_text(self.tcx) && values.found.is_simple_text(self.tcx))
|| (exp_found.is_some_and(|ef| {
// This happens when the type error is a subset of the expectation,
// like when you have two references but one is `usize` and the other
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,11 @@ impl<'tcx> MonoItem<'tcx> {
}
}

pub fn is_generic_fn(&self) -> bool {
match *self {
MonoItem::Fn(ref instance) => instance.args.non_erasable_generics().next().is_some(),
pub fn is_generic_fn(&self, tcx: TyCtxt<'tcx>) -> bool {
match self {
MonoItem::Fn(instance) => {
instance.args.non_erasable_generics(tcx, instance.def_id()).next().is_some()
}
MonoItem::Static(..) | MonoItem::GlobalAsm(..) => false,
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ impl<'tcx> Ty<'tcx> {
/// description in error messages. This is used in the primary span label. Beyond what
/// `is_simple_ty` includes, it also accepts ADTs with no type arguments and references to
/// ADTs with no type arguments.
pub fn is_simple_text(self) -> bool {
pub fn is_simple_text(self, tcx: TyCtxt<'tcx>) -> bool {
match self.kind() {
Adt(_, args) => args.non_erasable_generics().next().is_none(),
Ref(_, ty, _) => ty.is_simple_text(),
Adt(def, args) => args.non_erasable_generics(tcx, def.did()).next().is_none(),
Ref(_, ty, _) => ty.is_simple_text(tcx),
_ => self.is_simple_ty(),
}
}
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_middle/src/ty/generic_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,12 +379,17 @@ impl<'tcx> GenericArgs<'tcx> {
self.iter().filter_map(|k| k.as_const())
}

/// Returns generic arguments that are not lifetimes or host effect params.
#[inline]
pub fn non_erasable_generics(
&'tcx self,
tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> impl DoubleEndedIterator<Item = GenericArgKind<'tcx>> + 'tcx {
self.iter().filter_map(|k| match k.unpack() {
GenericArgKind::Lifetime(_) => None,
let generics = tcx.generics_of(def_id);
self.iter().enumerate().filter_map(|(i, k)| match k.unpack() {
_ if Some(i) == generics.host_effect_index => None,
ty::GenericArgKind::Lifetime(_) => None,
generic => Some(generic),
})
}
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/ty/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,12 @@ impl<'tcx> Generics {
pub fn own_requires_monomorphization(&self) -> bool {
for param in &self.params {
match param.kind {
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
GenericParamDefKind::Type { .. }
| GenericParamDefKind::Const { is_host_effect: false, .. } => {
return true;
}
GenericParamDefKind::Lifetime => {}
GenericParamDefKind::Lifetime
| GenericParamDefKind::Const { is_host_effect: true, .. } => {}
}
}
false
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl<'tcx> Instance<'tcx> {
}

// If this a non-generic instance, it cannot be a shared monomorphization.
self.args.non_erasable_generics().next()?;
self.args.non_erasable_generics(tcx, self.def_id()).next()?;

match self.def {
InstanceDef::Item(def) => tcx
Expand Down Expand Up @@ -344,6 +344,7 @@ impl<'tcx> Instance<'tcx> {
pub fn mono(tcx: TyCtxt<'tcx>, def_id: DefId) -> Instance<'tcx> {
let args = GenericArgs::for_item(tcx, def_id, |param, _| match param.kind {
ty::GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(),
ty::GenericParamDefKind::Const { is_host_effect: true, .. } => tcx.consts.true_.into(),
ty::GenericParamDefKind::Type { .. } => {
bug!("Instance::mono: {:?} has type parameters", def_id)
}
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,12 @@ impl<'tcx> Inliner<'tcx> {

// Reachability pass defines which functions are eligible for inlining. Generally inlining
// other functions is incorrect because they could reference symbols that aren't exported.
let is_generic = callsite.callee.args.non_erasable_generics().next().is_some();
let is_generic = callsite
.callee
.args
.non_erasable_generics(self.tcx, callsite.callee.def_id())
.next()
.is_some();
if !is_generic && !callee_attrs.requests_inline() {
return Err("not exported");
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ fn collect_items_rec<'tcx>(
// Check for PMEs and emit a diagnostic if one happened. To try to show relevant edges of the
// mono item graph.
if tcx.sess.diagnostic().err_count() > error_count
&& starting_item.node.is_generic_fn()
&& starting_item.node.is_generic_fn(tcx)
&& starting_item.node.is_user_defined()
{
let formatted_item = with_no_trimmed_paths!(starting_item.node.to_string());
Expand Down Expand Up @@ -1315,6 +1315,7 @@ fn create_mono_items_for_default_impls<'tcx>(
// it, to validate whether or not the impl is legal to instantiate at all.
let only_region_params = |param: &ty::GenericParamDef, _: &_| match param.kind {
GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(),
GenericParamDefKind::Const { is_host_effect: true, .. } => tcx.consts.true_.into(),
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
unreachable!(
"`own_requires_monomorphization` check means that \
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_monomorphize/src/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ where
}

let characteristic_def_id = characteristic_def_id_of_mono_item(cx.tcx, mono_item);
let is_volatile = is_incremental_build && mono_item.is_generic_fn();
let is_volatile = is_incremental_build && mono_item.is_generic_fn(cx.tcx);

let cgu_name = match characteristic_def_id {
Some(def_id) => compute_codegen_unit_name(
Expand Down Expand Up @@ -801,7 +801,7 @@ fn mono_item_visibility<'tcx>(
return Visibility::Hidden;
}

let is_generic = instance.args.non_erasable_generics().next().is_some();
let is_generic = instance.args.non_erasable_generics(tcx, def_id).next().is_some();

// Upstream `DefId` instances get different handling than local ones.
let Some(def_id) = def_id.as_local() else {
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_symbol_mangling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
use rustc_middle::mir::mono::{InstantiationMode, MonoItem};
use rustc_middle::query::Providers;
use rustc_middle::ty::GenericArgsRef;
use rustc_middle::ty::{self, Instance, TyCtxt};
use rustc_session::config::SymbolManglingVersion;

Expand Down Expand Up @@ -144,7 +143,7 @@ fn symbol_name_provider<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> ty
// This closure determines the instantiating crate for instances that
// need an instantiating-crate-suffix for their symbol name, in order
// to differentiate between local copies.
if is_generic(instance.args) {
if is_generic(instance, tcx) {
// For generics we might find re-usable upstream instances. If there
// is one, we rely on the symbol being instantiated locally.
instance.upstream_monomorphization(tcx).unwrap_or(LOCAL_CRATE)
Expand Down Expand Up @@ -246,7 +245,7 @@ fn compute_symbol_name<'tcx>(
// the ID of the instantiating crate. This avoids symbol conflicts
// in case the same instances is emitted in two crates of the same
// project.
let avoid_cross_crate_conflicts = is_generic(args) || is_globally_shared_function;
let avoid_cross_crate_conflicts = is_generic(instance, tcx) || is_globally_shared_function;

let instantiating_crate = avoid_cross_crate_conflicts.then(compute_instantiating_crate);

Expand Down Expand Up @@ -278,6 +277,6 @@ fn compute_symbol_name<'tcx>(
symbol
}

fn is_generic(args: GenericArgsRef<'_>) -> bool {
args.non_erasable_generics().next().is_some()
fn is_generic<'tcx>(instance: Instance<'tcx>, tcx: TyCtxt<'tcx>) -> bool {
instance.args.non_erasable_generics(tcx, instance.def_id()).next().is_some()
}
1 change: 1 addition & 0 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ fn resolve_instance<'tcx>(
}
} else {
debug!(" => free item");
// FIXME(effects): we may want to erase the effect param if that is present on this item.
ty::InstanceDef::Item(def_id)
};

Expand Down

0 comments on commit 58c9946

Please sign in to comment.