bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #137180 - compiler-errors:sym-regions, r=oli-obk

Browse source 
Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes

There are a few intertwined problems with `sym fn` operands in both inline and global asm macros.

Specifically, unlike other anon consts, they may evaluate to a type with free regions in them without actually having an item-level type annotation to give them a "proper" type. This is in contrast to named constants, which always have an item-level type annotation, or unnamed constants which are constrained by their position (e.g. a const arg in a turbofish, or a const array length).

Today, we infer the type of the operand by looking at the HIR typeck results; however, those results are region-erased, so during borrowck we ICE since we don't expect to encounter erased regions. We can't just fill this type with something like `'static`, since we may want to use real (free) regions:

```rust
fn foo<'a>() {
  asm!("/* ... */", sym bar::<&'a ()>);
}
```

The first idea may be to represent `sym fn` operands using *inline* consts instead of anon consts. This makes sense, since inline consts can reference regions from the parent body (like the `'a` in the example above). However, this introduces a problem with `global_asm!`, which doesn't *have* a parent body; inline consts *must* be associated with a parent body since they are not a body owner of their own. In #116087, I attempted to fix this by using two separate `sym` operands for global and inline asm. However, this led to a lot of confusion and also some unattractive code duplication.

In this PR, I adjust the lowering of `global_asm!` so that it's lowered in a "fake" HIR body. This body contains a single expression which is `ExprKind::InlineAsm`; we don't *use* this HIR body, but it's used in typeck and borrowck so that we can properly infer and validate the the lifetimes of `sym fn` operands.

I then adjust the lowering of `sym fn` to instead be represented with a HIR expression. This is both because it's no longer necessary to represent this operand as an anon const, since it's *just* a path expression, and also more importantly to sidestep yet another ICE (https://github.com/rust-lang/rust/issues/137179), which has to do with the existing code breaking an invariant of def-id creation and anon consts. Specifically, we are not allowed to synthesize a def-id for an anon const when that anon const contains expressions with def-ids whose parent is *not* that anon const. This is somewhat related to https://github.com/rust-lang/rust/pull/130443#issuecomment-2445678945, which is also a place in the compiler where synthesizing anon consts leads to def-id parenting issue.

As a side-effect, this consolidates the type checking for inline and global asm, so it allows us to simplify `InlineAsmCtxt` a bit. It also allows us to delete a bit of hacky code from anon const `type_of` which was there to detect `sym fn` operands specifically. This also could be generalized to support `const` asm operands with types with lifetimes in them. Since we specifically reject these consts today, I'm not going to change the representation of those consts (but they'd just be turned into inline consts).

r? oli-obk -- mostly b/c you're patient and also understand the breadth of the code that this touches, please reassign if you don't want to review this.

Fixes #111709
Fixes #96304
Fixes #137179
This commit is contained in:
Matthias Krüger 2025-02-23 00:16:19 +01:00 • committed by GitHub
commit 4115f51d15
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
49 changed files with 284 additions and 291 deletions
Download patch file Download diff file Expand all files Collapse all files

9
compiler/rustc_hir_analysis/src/check/check.rs
View file

@ -15,7 +15,6 @@ use rustc_lint_defs::builtin::{
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::span_bug;
use rustc_middle::ty::error::TypeErrorToStringExt;
use rustc_middle::ty::fold::{BottomUpFolder, fold_regions};
use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES};
@ -35,7 +34,6 @@ use {rustc_attr_parsing as attr, rustc_hir as hir};
use super::compare_impl_item::check_type_bounds;
use super::*;
use crate::check::intrinsicck::InlineAsmCtxt;
pub fn check_abi(tcx: TyCtxt<'_>, span: Span, abi: ExternAbi) {
if !tcx.sess.target.is_abi_supported(abi) {
@ -895,13 +893,6 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
}
}
}
DefKind::GlobalAsm => {
let it = tcx.hir().expect_item(def_id);
let hir::ItemKind::GlobalAsm(asm) = it.kind else {
span_bug!(it.span, "DefKind::GlobalAsm but got {:#?}", it)
};
InlineAsmCtxt::new_global_asm(tcx).check_asm(asm, def_id);
}
_ => {}
}
}

116
compiler/rustc_hir_analysis/src/check/intrinsicck.rs
View file

@ -6,7 +6,7 @@ use rustc_data_structures::fx::FxIndexSet;
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, LangItem};
use rustc_middle::bug;
use rustc_middle::ty::{self, FloatTy, IntTy, Ty, TyCtxt, TypeVisitableExt, UintTy};
use rustc_middle::ty::{self, Article, FloatTy, IntTy, Ty, TyCtxt, TypeVisitableExt, UintTy};
use rustc_session::lint;
use rustc_span::def_id::LocalDefId;
use rustc_span::{Symbol, sym};
@ -16,10 +16,11 @@ use rustc_target::asm::{
use crate::errors::RegisterTypeUnstable;
pub struct InlineAsmCtxt<'a, 'tcx> {
pub struct InlineAsmCtxt<'a, 'tcx: 'a> {
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
get_operand_ty: Box<dyn Fn(&'tcx hir::Expr<'tcx>) -> Ty<'tcx> + 'a>,
target_features: &'tcx FxIndexSet<Symbol>,
expr_ty: Box<dyn Fn(&hir::Expr<'tcx>) -> Ty<'tcx> + 'a>,
}
enum NonAsmTypeReason<'tcx> {
@ -29,25 +30,24 @@ enum NonAsmTypeReason<'tcx> {
}
impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
pub fn new_global_asm(tcx: TyCtxt<'tcx>) -> Self {
pub fn new(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
get_operand_ty: impl Fn(&hir::Expr<'tcx>) -> Ty<'tcx> + 'a,
) -> Self {
InlineAsmCtxt {
tcx,
typing_env: ty::TypingEnv {
typing_mode: ty::TypingMode::non_body_analysis(),
param_env: ty::ParamEnv::empty(),
},
get_operand_ty: Box::new(|e| bug!("asm operand in global asm: {e:?}")),
target_features: tcx.asm_target_features(def_id),
expr_ty: Box::new(get_operand_ty),
}
}
// FIXME(#132279): This likely causes us to incorrectly handle opaque types in their
// defining scope.
pub fn new_in_fn(
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
get_operand_ty: impl Fn(&'tcx hir::Expr<'tcx>) -> Ty<'tcx> + 'a,
) -> Self {
InlineAsmCtxt { tcx, typing_env, get_operand_ty: Box::new(get_operand_ty) }
fn expr_ty(&self, expr: &hir::Expr<'tcx>) -> Ty<'tcx> {
(self.expr_ty)(expr)
}
// FIXME(compiler-errors): This could use `<$ty as Pointee>::Metadata == ()`
@ -139,9 +139,8 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
template: &[InlineAsmTemplatePiece],
is_input: bool,
tied_input: Option<(&'tcx hir::Expr<'tcx>, Option<InlineAsmType>)>,
target_features: &FxIndexSet<Symbol>,
) -> Option<InlineAsmType> {
let ty = (self.get_operand_ty)(expr);
let ty = self.expr_ty(expr);
if ty.has_non_region_infer() {
bug!("inference variable in asm operand ty: {:?} {:?}", expr, ty);
}
@ -229,7 +228,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
if let Some((in_expr, Some(in_asm_ty))) = tied_input {
if in_asm_ty != asm_ty {
let msg = "incompatible types for asm inout argument";
let in_expr_ty = (self.get_operand_ty)(in_expr);
let in_expr_ty = self.expr_ty(in_expr);
self.tcx
.dcx()
.struct_span_err(vec![in_expr.span, expr.span], msg)
@ -291,7 +290,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
// (!). In that case we still need the earlier check to verify that the
// register class is usable at all.
if let Some(feature) = feature {
if !target_features.contains(feature) {
if !self.target_features.contains(feature) {
let msg = format!("`{feature}` target feature is not enabled");
self.tcx
.dcx()
@ -351,14 +350,13 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
Some(asm_ty)
}
pub fn check_asm(&self, asm: &hir::InlineAsm<'tcx>, enclosing_id: LocalDefId) {
let target_features = self.tcx.asm_target_features(enclosing_id.to_def_id());
pub fn check_asm(&self, asm: &hir::InlineAsm<'tcx>) {
let Some(asm_arch) = self.tcx.sess.asm_arch else {
self.tcx.dcx().delayed_bug("target architecture does not support asm");
return;
};
let allow_experimental_reg = self.tcx.features().asm_experimental_reg();
for (idx, (op, op_sp)) in asm.operands.iter().enumerate() {
for (idx, &(op, op_sp)) in asm.operands.iter().enumerate() {
// Validate register classes against currently enabled target
// features. We check that at least one type is available for
// the enabled features.
@ -381,12 +379,12 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
if let Err(msg) = reg.validate(
asm_arch,
self.tcx.sess.relocation_model(),
target_features,
self.target_features,
&self.tcx.sess.target,
op.is_clobber(),
) {
let msg = format!("cannot use register `{}`: {}", reg.name(), msg);
self.tcx.dcx().span_err(*op_sp, msg);
self.tcx.dcx().span_err(op_sp, msg);
continue;
}
}
@ -401,7 +399,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
{
match feature {
Some(feature) => {
if target_features.contains(&feature) {
if self.target_features.contains(&feature) {
missing_required_features.clear();
break;
} else {
@ -426,7 +424,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
reg_class.name(),
feature
);
self.tcx.dcx().span_err(*op_sp, msg);
self.tcx.dcx().span_err(op_sp, msg);
// register isn't enabled, don't do more checks
continue;
}
@ -440,7 +438,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
.intersperse(", ")
.collect::<String>(),
);
self.tcx.dcx().span_err(*op_sp, msg);
self.tcx.dcx().span_err(op_sp, msg);
// register isn't enabled, don't do more checks
continue;
}
@ -448,52 +446,21 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
}
}
match *op {
match op {
hir::InlineAsmOperand::In { reg, expr } => {
self.check_asm_operand_type(
idx,
reg,
expr,
asm.template,
true,
None,
target_features,
);
self.check_asm_operand_type(idx, reg, expr, asm.template, true, None);
}
hir::InlineAsmOperand::Out { reg, late: _, expr } => {
if let Some(expr) = expr {
self.check_asm_operand_type(
idx,
reg,
expr,
asm.template,
false,
None,
target_features,
);
self.check_asm_operand_type(idx, reg, expr, asm.template, false, None);
}
}
hir::InlineAsmOperand::InOut { reg, late: _, expr } => {
self.check_asm_operand_type(
idx,
reg,
expr,
asm.template,
false,
None,
target_features,
);
self.check_asm_operand_type(idx, reg, expr, asm.template, false, None);
}
hir::InlineAsmOperand::SplitInOut { reg, late: _, in_expr, out_expr } => {
let in_ty = self.check_asm_operand_type(
idx,
reg,
in_expr,
asm.template,
true,
None,
target_features,
);
let in_ty =
self.check_asm_operand_type(idx, reg, in_expr, asm.template, true, None);
if let Some(out_expr) = out_expr {
self.check_asm_operand_type(
idx,
@ -502,7 +469,6 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
asm.template,
false,
Some((in_expr, in_ty)),
target_features,
);
}
}
@ -514,11 +480,25 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
);
}
// Typeck has checked that SymFn refers to a function.
hir::InlineAsmOperand::SymFn { anon_const } => {
debug_assert_matches!(
self.tcx.type_of(anon_const.def_id).instantiate_identity().kind(),
ty::Error(_) | ty::FnDef(..)
);
hir::InlineAsmOperand::SymFn { expr } => {
let ty = self.expr_ty(expr);
match ty.kind() {
ty::FnDef(..) => {}
ty::Error(_) => {}
_ => {
self.tcx
.dcx()
.struct_span_err(op_sp, "invalid `sym` operand")
.with_span_label(
expr.span,
format!("is {} `{}`", ty.kind().article(), ty),
)
.with_help(
"`sym` operands must refer to either a function or a static",
)
.emit();
}
}
}
// AST lowering guarantees that SymStatic points to a static.
hir::InlineAsmOperand::SymStatic { .. } => {}

2
compiler/rustc_hir_analysis/src/collect.rs
View file

@ -680,7 +680,7 @@ fn lower_item(tcx: TyCtxt<'_>, item_id: hir::ItemId) {
| hir::ItemKind::Use(..)
| hir::ItemKind::Macro(..)
| hir::ItemKind::Mod(_)
| hir::ItemKind::GlobalAsm(_) => {}
| hir::ItemKind::GlobalAsm { .. } => {}
hir::ItemKind::ForeignMod { items, .. } => {
for item in *items {
let item = tcx.hir_foreign_item(item.id);

3
compiler/rustc_hir_analysis/src/collect/generics_of.rs
View file

@ -189,8 +189,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics {
// Exclude `GlobalAsm` here which cannot have generics.
Node::Expr(&Expr { kind: ExprKind::InlineAsm(asm), .. })
if asm.operands.iter().any(|(op, _op_sp)| match op {
hir::InlineAsmOperand::Const { anon_const }
| hir::InlineAsmOperand::SymFn { anon_const } => {
hir::InlineAsmOperand::Const { anon_const } => {
anon_const.hir_id == hir_id
}
_ => false,

2
compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs
View file

@ -623,7 +623,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
| hir::ItemKind::Mod(..)
| hir::ItemKind::ForeignMod { .. }
| hir::ItemKind::Static(..)
| hir::ItemKind::GlobalAsm(..) => {
| hir::ItemKind::GlobalAsm { .. } => {
// These sorts of items have no lifetime parameters at all.
intravisit::walk_item(self, item);
}

35
compiler/rustc_hir_analysis/src/collect/type_of.rs
View file

@ -35,13 +35,6 @@ fn anon_const_type_of<'tcx>(icx: &ItemCtxt<'tcx>, def_id: LocalDefId) -> Ty<'tcx
let parent_node_id = tcx.parent_hir_id(hir_id);
let parent_node = tcx.hir_node(parent_node_id);
let find_sym_fn = |&(op, op_sp)| match op {
hir::InlineAsmOperand::SymFn { anon_const } if anon_const.hir_id == hir_id => {
Some((anon_const, op_sp))
}
_ => None,
};
let find_const = |&(op, op_sp)| match op {
hir::InlineAsmOperand::Const { anon_const } if anon_const.hir_id == hir_id => {
Some((anon_const, op_sp))
@ -59,31 +52,7 @@ fn anon_const_type_of<'tcx>(icx: &ItemCtxt<'tcx>, def_id: LocalDefId) -> Ty<'tcx
// Anon consts outside the type system.
Node::Expr(&Expr { kind: ExprKind::InlineAsm(asm), .. })
| Node::Item(&Item { kind: ItemKind::GlobalAsm(asm), .. })
if let Some((anon_const, op_sp)) = asm.operands.iter().find_map(find_sym_fn) =>
{
let ty = tcx.typeck(def_id).node_type(hir_id);
match ty.kind() {
ty::Error(_) => ty,
ty::FnDef(..) => ty,
_ => {
let guar = tcx
.dcx()
.struct_span_err(op_sp, "invalid `sym` operand")
.with_span_label(
tcx.def_span(anon_const.def_id),
format!("is {} `{}`", ty.kind().article(), ty),
)
.with_help("`sym` operands must refer to either a function or a static")
.emit();
Ty::new_error(tcx, guar)
}
}
}
Node::Expr(&Expr { kind: ExprKind::InlineAsm(asm), .. })
| Node::Item(&Item { kind: ItemKind::GlobalAsm(asm), .. })
| Node::Item(&Item { kind: ItemKind::GlobalAsm { asm, .. }, .. })
if let Some((anon_const, op_sp)) = asm.operands.iter().find_map(find_const) =>
{
let ty = tcx.typeck(def_id).node_type(hir_id);
@ -313,12 +282,12 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_
let args = ty::GenericArgs::identity_for_item(tcx, def_id);
Ty::new_adt(tcx, def, args)
}
ItemKind::GlobalAsm { .. } => tcx.typeck(def_id).node_type(hir_id),
ItemKind::Trait(..)
| ItemKind::TraitAlias(..)
| ItemKind::Macro(..)
| ItemKind::Mod(..)
| ItemKind::ForeignMod { .. }
| ItemKind::GlobalAsm(..)
| ItemKind::ExternCrate(..)
| ItemKind::Use(..) => {
span_bug!(item.span, "compute_type_of_item: unexpected item type: {:?}", item.kind);