Skip to content

Commit 376165a

Browse files
authored
Rollup merge of rust-lang#137180 - compiler-errors:sym-regions, r=oli-obk
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 rust-lang#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 (rust-lang#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 rust-lang#130443 (comment), 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 rust-lang#111709 Fixes rust-lang#96304 Fixes rust-lang#137179
2 parents aec41ef + 6ba39f7 commit 376165a

File tree

49 files changed

+284
-291
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+284
-291
lines changed

compiler/rustc_ast_lowering/src/asm.rs

+2-16
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
use std::collections::hash_map::Entry;
22
use std::fmt::Write;
33

4-
use rustc_ast::ptr::P;
54
use rustc_ast::*;
65
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
76
use rustc_hir as hir;
87
use rustc_hir::def::{DefKind, Res};
98
use rustc_session::parse::feature_err;
10-
use rustc_span::{Span, kw, sym};
9+
use rustc_span::{Span, sym};
1110
use rustc_target::asm;
1211

1312
use super::LoweringContext;
@@ -230,20 +229,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
230229
tokens: None,
231230
};
232231

233-
// Wrap the expression in an AnonConst.
234-
let parent_def_id = self.current_hir_id_owner.def_id;
235-
let node_id = self.next_node_id();
236-
self.create_def(
237-
parent_def_id,
238-
node_id,
239-
kw::Empty,
240-
DefKind::AnonConst,
241-
*op_sp,
242-
);
243-
let anon_const = AnonConst { id: node_id, value: P(expr) };
244-
hir::InlineAsmOperand::SymFn {
245-
anon_const: self.lower_anon_const_to_anon_const(&anon_const),
246-
}
232+
hir::InlineAsmOperand::SymFn { expr: self.lower_expr(&expr) }
247233
}
248234
}
249235
InlineAsmOperand::Label { block } => {

compiler/rustc_ast_lowering/src/item.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
251251
.arena
252252
.alloc_from_iter(fm.items.iter().map(|x| self.lower_foreign_item_ref(x))),
253253
},
254-
ItemKind::GlobalAsm(asm) => hir::ItemKind::GlobalAsm(self.lower_inline_asm(span, asm)),
254+
ItemKind::GlobalAsm(asm) => {
255+
let asm = self.lower_inline_asm(span, asm);
256+
let fake_body =
257+
self.lower_body(|this| (&[], this.expr(span, hir::ExprKind::InlineAsm(asm))));
258+
hir::ItemKind::GlobalAsm { asm, fake_body }
259+
}
255260
ItemKind::TyAlias(box TyAlias { generics, where_clauses, ty, .. }) => {
256261
// We lower
257262
//

compiler/rustc_borrowck/src/universal_regions.rs

+24-5
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,11 @@ pub(crate) enum DefiningTy<'tcx> {
126126
/// The MIR represents an inline const. The signature has no inputs and a
127127
/// single return value found via `InlineConstArgs::ty`.
128128
InlineConst(DefId, GenericArgsRef<'tcx>),
129+
130+
// Fake body for a global asm. Not particularly useful or interesting,
131+
// but we need it so we can properly store the typeck results of the asm
132+
// operands, which aren't associated with a body otherwise.
133+
GlobalAsm(DefId),
129134
}
130135

131136
impl<'tcx> DefiningTy<'tcx> {
@@ -138,9 +143,10 @@ impl<'tcx> DefiningTy<'tcx> {
138143
DefiningTy::Closure(_, args) => args.as_closure().upvar_tys(),
139144
DefiningTy::CoroutineClosure(_, args) => args.as_coroutine_closure().upvar_tys(),
140145
DefiningTy::Coroutine(_, args) => args.as_coroutine().upvar_tys(),
141-
DefiningTy::FnDef(..) | DefiningTy::Const(..) | DefiningTy::InlineConst(..) => {
142-
ty::List::empty()
143-
}
146+
DefiningTy::FnDef(..)
147+
| DefiningTy::Const(..)
148+
| DefiningTy::InlineConst(..)
149+
| DefiningTy::GlobalAsm(_) => ty::List::empty(),
144150
}
145151
}
146152

@@ -152,7 +158,10 @@ impl<'tcx> DefiningTy<'tcx> {
152158
DefiningTy::Closure(..)
153159
| DefiningTy::CoroutineClosure(..)
154160
| DefiningTy::Coroutine(..) => 1,
155-
DefiningTy::FnDef(..) | DefiningTy::Const(..) | DefiningTy::InlineConst(..) => 0,
161+
DefiningTy::FnDef(..)
162+
| DefiningTy::Const(..)
163+
| DefiningTy::InlineConst(..)
164+
| DefiningTy::GlobalAsm(_) => 0,
156165
}
157166
}
158167

@@ -171,7 +180,8 @@ impl<'tcx> DefiningTy<'tcx> {
171180
| DefiningTy::Coroutine(def_id, ..)
172181
| DefiningTy::FnDef(def_id, ..)
173182
| DefiningTy::Const(def_id, ..)
174-
| DefiningTy::InlineConst(def_id, ..) => def_id,
183+
| DefiningTy::InlineConst(def_id, ..)
184+
| DefiningTy::GlobalAsm(def_id) => def_id,
175185
}
176186
}
177187
}
@@ -411,6 +421,7 @@ impl<'tcx> UniversalRegions<'tcx> {
411421
tcx.def_path_str_with_args(def_id, args),
412422
));
413423
}
424+
DefiningTy::GlobalAsm(_) => unreachable!(),
414425
}
415426
}
416427

@@ -633,6 +644,8 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
633644
DefiningTy::InlineConst(self.mir_def.to_def_id(), args)
634645
}
635646
}
647+
648+
BodyOwnerKind::GlobalAsm => DefiningTy::GlobalAsm(self.mir_def.to_def_id()),
636649
}
637650
}
638651

@@ -666,6 +679,8 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
666679
}
667680

668681
DefiningTy::FnDef(_, args) | DefiningTy::Const(_, args) => args,
682+
683+
DefiningTy::GlobalAsm(_) => ty::List::empty(),
669684
};
670685

671686
let global_mapping = iter::once((tcx.lifetimes.re_static, fr_static));
@@ -802,6 +817,10 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
802817
let ty = args.as_inline_const().ty();
803818
ty::Binder::dummy(tcx.mk_type_list(&[ty]))
804819
}
820+
821+
DefiningTy::GlobalAsm(def_id) => {
822+
ty::Binder::dummy(tcx.mk_type_list(&[tcx.type_of(def_id).instantiate_identity()]))
823+
}
805824
};
806825

807826
// FIXME(#129952): We probably want a more principled approach here.

compiler/rustc_codegen_cranelift/src/global_asm.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::prelude::*;
1616

1717
pub(crate) fn codegen_global_asm_item(tcx: TyCtxt<'_>, global_asm: &mut String, item_id: ItemId) {
1818
let item = tcx.hir_item(item_id);
19-
if let rustc_hir::ItemKind::GlobalAsm(asm) = item.kind {
19+
if let rustc_hir::ItemKind::GlobalAsm { asm, .. } = item.kind {
2020
let is_x86 =
2121
matches!(tcx.sess.asm_arch.unwrap(), InlineAsmArch::X86 | InlineAsmArch::X86_64);
2222

@@ -55,15 +55,15 @@ pub(crate) fn codegen_global_asm_item(tcx: TyCtxt<'_>, global_asm: &mut String,
5555
}
5656
}
5757
}
58-
InlineAsmOperand::SymFn { anon_const } => {
58+
InlineAsmOperand::SymFn { expr } => {
5959
if cfg!(not(feature = "inline_asm_sym")) {
6060
tcx.dcx().span_err(
6161
item.span,
6262
"asm! and global_asm! sym operands are not yet supported",
6363
);
6464
}
6565

66-
let ty = tcx.typeck_body(anon_const.body).node_type(anon_const.hir_id);
66+
let ty = tcx.typeck(item_id.owner_id).expr_ty(expr);
6767
let instance = match ty.kind() {
6868
&ty::FnDef(def_id, args) => Instance::new(def_id, args),
6969
_ => span_bug!(op_sp, "asm sym is not a function"),

compiler/rustc_codegen_ssa/src/mono_item.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> {
3636
}
3737
MonoItem::GlobalAsm(item_id) => {
3838
let item = cx.tcx().hir_item(item_id);
39-
if let hir::ItemKind::GlobalAsm(asm) = item.kind {
39+
if let hir::ItemKind::GlobalAsm { asm, .. } = item.kind {
4040
let operands: Vec<_> = asm
4141
.operands
4242
.iter()
@@ -71,11 +71,8 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> {
7171
}
7272
}
7373
}
74-
hir::InlineAsmOperand::SymFn { ref anon_const } => {
75-
let ty = cx
76-
.tcx()
77-
.typeck_body(anon_const.body)
78-
.node_type(anon_const.hir_id);
74+
hir::InlineAsmOperand::SymFn { expr } => {
75+
let ty = cx.tcx().typeck(item_id.owner_id).expr_ty(expr);
7976
let instance = match ty.kind() {
8077
&ty::FnDef(def_id, args) => Instance::new(def_id, args),
8178
_ => span_bug!(*op_sp, "asm sym is not a function"),

compiler/rustc_hir/src/hir.rs

+22-5
Original file line numberDiff line numberDiff line change
@@ -1913,13 +1913,18 @@ pub enum BodyOwnerKind {
19131913

19141914
/// Initializer of a `static` item.
19151915
Static(Mutability),
1916+
1917+
/// Fake body for a global asm to store its const-like value types.
1918+
GlobalAsm,
19161919
}
19171920

19181921
impl BodyOwnerKind {
19191922
pub fn is_fn_or_closure(self) -> bool {
19201923
match self {
19211924
BodyOwnerKind::Fn | BodyOwnerKind::Closure => true,
1922-
BodyOwnerKind::Const { .. } | BodyOwnerKind::Static(_) => false,
1925+
BodyOwnerKind::Const { .. } | BodyOwnerKind::Static(_) | BodyOwnerKind::GlobalAsm => {
1926+
false
1927+
}
19231928
}
19241929
}
19251930
}
@@ -3420,7 +3425,7 @@ pub enum InlineAsmOperand<'hir> {
34203425
anon_const: &'hir AnonConst,
34213426
},
34223427
SymFn {
3423-
anon_const: &'hir AnonConst,
3428+
expr: &'hir Expr<'hir>,
34243429
},
34253430
SymStatic {
34263431
path: QPath<'hir>,
@@ -3848,7 +3853,7 @@ impl<'hir> Item<'hir> {
38483853
expect_foreign_mod, (ExternAbi, &'hir [ForeignItemRef]),
38493854
ItemKind::ForeignMod { abi, items }, (*abi, items);
38503855

3851-
expect_global_asm, &'hir InlineAsm<'hir>, ItemKind::GlobalAsm(asm), asm;
3856+
expect_global_asm, &'hir InlineAsm<'hir>, ItemKind::GlobalAsm { asm, .. }, asm;
38523857

38533858
expect_ty_alias, (&'hir Ty<'hir>, &'hir Generics<'hir>),
38543859
ItemKind::TyAlias(ty, generics), (ty, generics);
@@ -4015,7 +4020,15 @@ pub enum ItemKind<'hir> {
40154020
/// An external module, e.g. `extern { .. }`.
40164021
ForeignMod { abi: ExternAbi, items: &'hir [ForeignItemRef] },
40174022
/// Module-level inline assembly (from `global_asm!`).
4018-
GlobalAsm(&'hir InlineAsm<'hir>),
4023+
GlobalAsm {
4024+
asm: &'hir InlineAsm<'hir>,
4025+
/// A fake body which stores typeck results for the global asm's sym_fn
4026+
/// operands, which are represented as path expressions. This body contains
4027+
/// a single [`ExprKind::InlineAsm`] which points to the asm in the field
4028+
/// above, and which is typechecked like a inline asm expr just for the
4029+
/// typeck results.
4030+
fake_body: BodyId,
4031+
},
40194032
/// A type alias, e.g., `type Foo = Bar<u8>`.
40204033
TyAlias(&'hir Ty<'hir>, &'hir Generics<'hir>),
40214034
/// An enum definition, e.g., `enum Foo<A, B> {C<A>, D<B>}`.
@@ -4081,7 +4094,7 @@ impl ItemKind<'_> {
40814094
ItemKind::Macro(..) => "macro",
40824095
ItemKind::Mod(..) => "module",
40834096
ItemKind::ForeignMod { .. } => "extern block",
4084-
ItemKind::GlobalAsm(..) => "global asm item",
4097+
ItemKind::GlobalAsm { .. } => "global asm item",
40854098
ItemKind::TyAlias(..) => "type alias",
40864099
ItemKind::Enum(..) => "enum",
40874100
ItemKind::Struct(..) => "struct",
@@ -4540,6 +4553,10 @@ impl<'hir> Node<'hir> {
45404553
..
45414554
}) => Some((owner_id.def_id, *body)),
45424555

4556+
Node::Item(Item {
4557+
owner_id, kind: ItemKind::GlobalAsm { asm: _, fake_body }, ..
4558+
}) => Some((owner_id.def_id, *fake_body)),
4559+
45434560
Node::Expr(Expr { kind: ExprKind::Closure(Closure { def_id, body, .. }), .. }) => {
45444561
Some((*def_id, *body))
45454562
}

compiler/rustc_hir/src/intravisit.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -573,9 +573,13 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V::
573573
try_visit!(visitor.visit_id(item.hir_id()));
574574
walk_list!(visitor, visit_foreign_item_ref, items);
575575
}
576-
ItemKind::GlobalAsm(asm) => {
576+
ItemKind::GlobalAsm { asm: _, fake_body } => {
577577
try_visit!(visitor.visit_id(item.hir_id()));
578-
try_visit!(visitor.visit_inline_asm(asm, item.hir_id()));
578+
// Visit the fake body, which contains the asm statement.
579+
// Therefore we should not visit the asm statement again
580+
// outside of the body, or some visitors won't have their
581+
// typeck results set correctly.
582+
try_visit!(visitor.visit_nested_body(fake_body));
579583
}
580584
ItemKind::TyAlias(ref ty, ref generics) => {
581585
try_visit!(visitor.visit_id(item.hir_id()));
@@ -1442,10 +1446,12 @@ pub fn walk_inline_asm<'v, V: Visitor<'v>>(
14421446
try_visit!(visitor.visit_expr(in_expr));
14431447
visit_opt!(visitor, visit_expr, out_expr);
14441448
}
1445-
InlineAsmOperand::Const { anon_const, .. }
1446-
| InlineAsmOperand::SymFn { anon_const, .. } => {
1449+
InlineAsmOperand::Const { anon_const, .. } => {
14471450
try_visit!(visitor.visit_anon_const(anon_const));
14481451
}
1452+
InlineAsmOperand::SymFn { expr, .. } => {
1453+
try_visit!(visitor.visit_expr(expr));
1454+
}
14491455
InlineAsmOperand::SymStatic { path, .. } => {
14501456
try_visit!(visitor.visit_qpath(path, id, *op_sp));
14511457
}

compiler/rustc_hir/src/target.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl Target {
110110
ItemKind::Macro(..) => Target::MacroDef,
111111
ItemKind::Mod(..) => Target::Mod,
112112
ItemKind::ForeignMod { .. } => Target::ForeignMod,
113-
ItemKind::GlobalAsm(..) => Target::GlobalAsm,
113+
ItemKind::GlobalAsm { .. } => Target::GlobalAsm,
114114
ItemKind::TyAlias(..) => Target::TyAlias,
115115
ItemKind::Enum(..) => Target::Enum,
116116
ItemKind::Struct(..) => Target::Struct,

compiler/rustc_hir_analysis/src/check/check.rs

-9
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use rustc_lint_defs::builtin::{
1515
use rustc_middle::hir::nested_filter;
1616
use rustc_middle::middle::resolve_bound_vars::ResolvedArg;
1717
use rustc_middle::middle::stability::EvalResult;
18-
use rustc_middle::span_bug;
1918
use rustc_middle::ty::error::TypeErrorToStringExt;
2019
use rustc_middle::ty::fold::{BottomUpFolder, fold_regions};
2120
use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES};
@@ -35,7 +34,6 @@ use {rustc_attr_parsing as attr, rustc_hir as hir};
3534

3635
use super::compare_impl_item::check_type_bounds;
3736
use super::*;
38-
use crate::check::intrinsicck::InlineAsmCtxt;
3937

4038
pub fn check_abi(tcx: TyCtxt<'_>, span: Span, abi: ExternAbi) {
4139
if !tcx.sess.target.is_abi_supported(abi) {
@@ -895,13 +893,6 @@ pub(crate) fn check_item_type(tcx: TyCtxt<'_>, def_id: LocalDefId) {
895893
}
896894
}
897895
}
898-
DefKind::GlobalAsm => {
899-
let it = tcx.hir().expect_item(def_id);
900-
let hir::ItemKind::GlobalAsm(asm) = it.kind else {
901-
span_bug!(it.span, "DefKind::GlobalAsm but got {:#?}", it)
902-
};
903-
InlineAsmCtxt::new_global_asm(tcx).check_asm(asm, def_id);
904-
}
905896
_ => {}
906897
}
907898
}

0 commit comments

Comments
 (0)