Skip to content

Tweak type inference for const operands in inline asm #125558

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Aug 6, 2024
Merged

Tweak type inference for const operands in inline asm #125558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4d74712
Tweak type inference for `const` operands in inline asm
Amanieu May 25, 2024
575fc72
Work around #96304 by ignoring one test case
Amanieu Jun 11, 2024
97738e1
apply fix suggested by lcnr
folkertdev Jul 21, 2024
c77b569
Apply suggestions from code review
folkertdev Jul 22, 2024
d4ca1ac
rustfmt
Amanieu Jul 23, 2024
be66415
use `ErrorGuaranteed` from emit
folkertdev Jul 25, 2024
b73077e
separate test file for invalid sym operand
folkertdev Aug 1, 2024
47e6db5
separate test file for invalid const operand
folkertdev Aug 1, 2024
eb726a5
add `needs-asm-support` to invalid-sym-operand
folkertdev Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 15 additions & 25 deletions compiler/rustc_hir_analysis/src/check/intrinsicck.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_ast::InlineAsmTemplatePiece;
use rustc_data_structures::fx::FxIndexSet;
use rustc_hir::{self as hir, LangItem};
use rustc_middle::bug;
use rustc_middle::ty::{self, Article, FloatTy, IntTy, Ty, TyCtxt, TypeVisitableExt, UintTy};
use rustc_middle::ty::{self, FloatTy, IntTy, Ty, TyCtxt, TypeVisitableExt, UintTy};
use rustc_session::lint;
use rustc_span::def_id::LocalDefId;
use rustc_span::Symbol;
Expand Down Expand Up @@ -455,32 +455,22 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> {
);
}
}
// No special checking is needed for these:
// - Typeck has checked that Const operands are integers.
// - AST lowering guarantees that SymStatic points to a static.
hir::InlineAsmOperand::Const { .. } | hir::InlineAsmOperand::SymStatic { .. } => {}
// Check that sym actually points to a function. Later passes
// depend on this.
// Typeck has checked that Const operands are integers.
hir::InlineAsmOperand::Const { anon_const } => {
Copy link
Contributor

Choose a reason for hiding this comment

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

a suggestion: you can move this check into fn anon_const_type_of and return the returned type with ty::Error if it's not an int. THis should also fix the ICE in mir borrowck

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried this here master...folkertdev:rust:const-asm-type-fix and that makes the test work! However, error messages are now emitted out of order. I'm not sure how big a problem that is, and how to fix it if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

as an extra note, this issue appears to be entirely unrelated to #96304, lifetimes work fine in these const expressions.

core::arch::global_asm!("/* {} */", const Foo::<'static>::X);

struct Foo<'a>(&'a ());

impl<'a> Foo<'a> {
    const X: u64 = 42;
}

Copy link
Contributor

@lcnr lcnr Jul 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

👍 yeah, that's my preferred solution, though I'd maybe keep the check in intrinsicck as an assert in case type_of gets changed in the future. I don't think the message order is something we should worry about here.

debug_assert!(matches!(
self.tcx.type_of(anon_const.def_id).instantiate_identity().kind(),
ty::Error(_) | ty::Int(_) | ty::Uint(_)
));
}
// Typeck has checked that SymFn refers to a function.
hir::InlineAsmOperand::SymFn { anon_const } => {
let ty = self.tcx.type_of(anon_const.def_id).instantiate_identity();
match ty.kind() {
ty::Never | ty::Error(_) => {}
ty::FnDef(..) => {}
_ => {
self.tcx
.dcx()
.struct_span_err(*op_sp, "invalid `sym` operand")
.with_span_label(
self.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();
}
};
debug_assert!(matches!(
self.tcx.type_of(anon_const.def_id).instantiate_identity().kind(),
ty::Error(_) | ty::FnDef(..)
));
}
// AST lowering guarantees that SymStatic points to a static.
hir::InlineAsmOperand::SymStatic { .. } => {}
// No special checking is needed for labels.
hir::InlineAsmOperand::Label { .. } => {}
}
Expand Down
66 changes: 59 additions & 7 deletions compiler/rustc_hir_analysis/src/collect/type_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_hir::HirId;
use rustc_middle::query::plumbing::CyclePlaceholder;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{self, Article, IsSuggestable, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::Ident;
use rustc_span::{Span, DUMMY_SP};
Expand Down Expand Up @@ -35,6 +35,20 @@ fn anon_const_type_of<'tcx>(tcx: TyCtxt<'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))
}
_ => None,
};

match parent_node {
// Anon consts "inside" the type system.
Node::ConstArg(&ConstArg {
Expand All @@ -46,13 +60,51 @@ fn anon_const_type_of<'tcx>(tcx: TyCtxt<'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 asm.operands.iter().any(|(op, _op_sp)| match op {
hir::InlineAsmOperand::Const { anon_const }
| hir::InlineAsmOperand::SymFn { anon_const } => anon_const.hir_id == hir_id,
_ => false,
}) =>
if let Some((anon_const, op_sp)) = asm.operands.iter().find_map(find_sym_fn) =>
{
tcx.typeck(def_id).node_type(hir_id)
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), .. })
if let Some((anon_const, op_sp)) = asm.operands.iter().find_map(find_const) =>
{
let ty = tcx.typeck(def_id).node_type(hir_id);

match ty.kind() {
ty::Error(_) => ty,
ty::Int(_) | ty::Uint(_) => ty,
_ => {
let guar = tcx
.dcx()
.struct_span_err(op_sp, "invalid type for `const` operand")
.with_span_label(
tcx.def_span(anon_const.def_id),
format!("is {} `{}`", ty.kind().article(), ty),
)
.with_help("`const` operands must be of an integer type")
.emit();

Ty::new_error(tcx, guar)
}
}
}
Node::Variant(Variant { disr_expr: Some(ref e), .. }) if e.hir_id == hir_id => {
tcx.adt_def(tcx.hir().get_parent_item(hir_id)).repr().discr_type().to_ty(tcx)
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_hir_typeck/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,10 @@ fn infer_type_if_missing<'tcx>(fcx: &FnCtxt<'_, 'tcx>, node: Node<'tcx>) -> Opti
Node::Expr(&hir::Expr { kind: hir::ExprKind::InlineAsm(asm), span, .. })
| Node::Item(&hir::Item { kind: hir::ItemKind::GlobalAsm(asm), span, .. }) => {
asm.operands.iter().find_map(|(op, _op_sp)| match op {
hir::InlineAsmOperand::Const { anon_const } if anon_const.hir_id == id => {
// Inline assembly constants must be integers.
Some(fcx.next_int_var())
}
hir::InlineAsmOperand::SymFn { anon_const } if anon_const.hir_id == id => {
hir::InlineAsmOperand::Const { anon_const }
| hir::InlineAsmOperand::SymFn { anon_const }
if anon_const.hir_id == id =>
{
Some(fcx.next_ty_var(span))
}
_ => None,
Expand Down
18 changes: 0 additions & 18 deletions tests/ui/asm/aarch64/type-check-2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,6 @@ fn main() {
unsafe {
// Inputs must be initialized

// Sym operands must point to a function or static

const C: i32 = 0;
static S: i32 = 0;
asm!("{}", sym S);
asm!("{}", sym main);
asm!("{}", sym C);
//~^ ERROR invalid `sym` operand

// Register operands must be Copy

asm!("{:v}", in(vreg) SimdNonCopy(0.0, 0.0, 0.0, 0.0));
Expand Down Expand Up @@ -65,12 +56,3 @@ fn main() {
asm!("{}", in(reg) u);
}
}

// Sym operands must point to a function or static

const C: i32 = 0;
static S: i32 = 0;
global_asm!("{}", sym S);
global_asm!("{}", sym main);
global_asm!("{}", sym C);
//~^ ERROR invalid `sym` operand
34 changes: 9 additions & 25 deletions tests/ui/asm/aarch64/type-check-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,21 @@
error: invalid `sym` operand
--> $DIR/type-check-2.rs:75:19
|
LL | global_asm!("{}", sym C);
| ^^^^^ is an `i32`
|
= help: `sym` operands must refer to either a function or a static

error: invalid `sym` operand
--> $DIR/type-check-2.rs:24:20
|
LL | asm!("{}", sym C);
| ^^^^^ is an `i32`
|
= help: `sym` operands must refer to either a function or a static

error: arguments for inline assembly must be copyable
--> $DIR/type-check-2.rs:29:31
--> $DIR/type-check-2.rs:20:31
|
LL | asm!("{:v}", in(vreg) SimdNonCopy(0.0, 0.0, 0.0, 0.0));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `SimdNonCopy` does not implement the Copy trait

error: cannot use value of type `{closure@$DIR/type-check-2.rs:41:28: 41:36}` for inline assembly
--> $DIR/type-check-2.rs:41:28
error: cannot use value of type `{closure@$DIR/type-check-2.rs:32:28: 32:36}` for inline assembly
--> $DIR/type-check-2.rs:32:28
|
LL | asm!("{}", in(reg) |x: i32| x);
| ^^^^^^^^^^
|
= note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly

error: cannot use value of type `Vec<i32>` for inline assembly
--> $DIR/type-check-2.rs:43:28
--> $DIR/type-check-2.rs:34:28
|
LL | asm!("{}", in(reg) vec![0]);
| ^^^^^^^
Expand All @@ -40,36 +24,36 @@ LL | asm!("{}", in(reg) vec![0]);
= note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)

error: cannot use value of type `(i32, i32, i32)` for inline assembly
--> $DIR/type-check-2.rs:45:28
--> $DIR/type-check-2.rs:36:28
|
LL | asm!("{}", in(reg) (1, 2, 3));
| ^^^^^^^^^
|
= note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly

error: cannot use value of type `[i32; 3]` for inline assembly
--> $DIR/type-check-2.rs:47:28
--> $DIR/type-check-2.rs:38:28
|
LL | asm!("{}", in(reg) [1, 2, 3]);
| ^^^^^^^^^
|
= note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly

error: cannot use value of type `fn() {main}` for inline assembly
--> $DIR/type-check-2.rs:55:31
--> $DIR/type-check-2.rs:46:31
|
LL | asm!("{}", inout(reg) f);
| ^
|
= note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly

error: cannot use value of type `&mut i32` for inline assembly
--> $DIR/type-check-2.rs:58:31
--> $DIR/type-check-2.rs:49:31
|
LL | asm!("{}", inout(reg) r);
| ^
|
= note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly

error: aborting due to 9 previous errors
error: aborting due to 7 previous errors

51 changes: 51 additions & 0 deletions tests/ui/asm/invalid-const-operand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//@ needs-asm-support
//@ ignore-nvptx64
//@ ignore-spirv

#![feature(asm_const)]

use std::arch::{asm, global_asm};

// Const operands must be integers and must be constants.

global_asm!("{}", const 0);
global_asm!("{}", const 0i32);
global_asm!("{}", const 0i128);
global_asm!("{}", const 0f32);
//~^ ERROR invalid type for `const` operand
global_asm!("{}", const 0 as *mut u8);
//~^ ERROR invalid type for `const` operand

fn main() {
unsafe {
// Const operands must be integers and must be constants.

asm!("{}", const 0);
asm!("{}", const 0i32);
asm!("{}", const 0i128);
asm!("{}", const 0f32);
//~^ ERROR invalid type for `const` operand
asm!("{}", const 0 as *mut u8);
//~^ ERROR invalid type for `const` operand
asm!("{}", const &0);
//~^ ERROR invalid type for `const` operand

// Constants must be... constant

let x = 0;
const fn const_foo(x: i32) -> i32 {
x
}
const fn const_bar<T>(x: T) -> T {
x
}
asm!("{}", const x);
//~^ ERROR attempt to use a non-constant value in a constant
asm!("{}", const const_foo(0));
asm!("{}", const const_foo(x));
//~^ ERROR attempt to use a non-constant value in a constant
asm!("{}", const const_bar(0));
asm!("{}", const const_bar(x));
//~^ ERROR attempt to use a non-constant value in a constant
}
}
Loading