From fff7827e2b02efaa849dbe9deda4eee143d65ae0 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 21 Jul 2024 18:02:59 +0200 Subject: [PATCH 1/2] apply fix suggested by lcnr --- .../src/check/intrinsicck.rs | 52 ++++---------- .../rustc_hir_analysis/src/collect/type_of.rs | 72 +++++++++++++++++-- tests/ui/asm/type-check-1.rs | 4 +- tests/ui/asm/type-check-1.stderr | 52 ++++++++------ tests/ui/asm/x86_64/type-check-2.stderr | 32 ++++----- 5 files changed, 127 insertions(+), 85 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/intrinsicck.rs b/compiler/rustc_hir_analysis/src/check/intrinsicck.rs index 9286d3d83bffc..6ea8e838d5cee 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsicck.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsicck.rs @@ -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; @@ -455,48 +455,22 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { ); } } + // Typeck has checked that Const operands are integers. hir::InlineAsmOperand::Const { anon_const } => { - let ty = self.tcx.type_of(anon_const.def_id).instantiate_identity(); - match ty.kind() { - ty::Error(_) => {} - ty::Int(_) | ty::Uint(_) => {} - _ => { - self.tcx - .dcx() - .struct_span_err(*op_sp, "invalid type for `const` operand") - .with_span_label( - self.tcx.def_span(anon_const.def_id), - format!("is {} `{}`", ty.kind().article(), ty), - ) - .with_help("`const` operands must be of an integer type") - .emit(); - } - }; + debug_assert!(matches!( + self.tcx.type_of(anon_const.def_id).instantiate_identity().kind(), + ty::Error(_) | ty::Int(_) | ty::Uint(_) + )); } - // AST lowering guarantees that SymStatic points to a static. - hir::InlineAsmOperand::SymStatic { .. } => {} - // Check that sym actually points to a function. Later passes - // depend on this. + // 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::Never | ty::FnDef(..) + )); } + // AST lowering guarantees that SymStatic points to a static. + hir::InlineAsmOperand::SymStatic { .. } => {} // No special checking is needed for labels. hir::InlineAsmOperand::Label { .. } => {} } diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index 9affd654366f1..a0d438caf8f93 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -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}; @@ -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 { @@ -46,13 +60,57 @@ 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::Never | ty::Error(_) => ty, + ty::FnDef(..) => ty, + _ => { + 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_with_message( + tcx, + span, + format!("invalid type for `const` operand"), + ) + } + } + } + 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, + _ => { + 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_with_message( + tcx, + span, + format!("invalid type for `const` operand"), + ) + } + } } 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) diff --git a/tests/ui/asm/type-check-1.rs b/tests/ui/asm/type-check-1.rs index 09aa4a584ce12..ad1a391539f1a 100644 --- a/tests/ui/asm/type-check-1.rs +++ b/tests/ui/asm/type-check-1.rs @@ -59,8 +59,8 @@ fn main() { asm!("{}", const 0 as *mut u8); //~^ ERROR invalid type for `const` operand - // FIXME: Currently ICEs due to #96304 - //asm!("{}", const &0); + asm!("{}", const &0); + //~^ ERROR invalid type for `const` operand } } diff --git a/tests/ui/asm/type-check-1.stderr b/tests/ui/asm/type-check-1.stderr index 30cd7fb42952e..bbed571284903 100644 --- a/tests/ui/asm/type-check-1.stderr +++ b/tests/ui/asm/type-check-1.stderr @@ -39,26 +39,6 @@ LL | asm!("{}", sym x); | = help: `sym` operands must refer to either a function or a static -error: invalid type for `const` operand - --> $DIR/type-check-1.rs:76:19 - | -LL | global_asm!("{}", const 0f32); - | ^^^^^^---- - | | - | is an `f32` - | - = help: `const` operands must be of an integer type - -error: invalid type for `const` operand - --> $DIR/type-check-1.rs:78:19 - | -LL | global_asm!("{}", const 0 as *mut u8); - | ^^^^^^------------ - | | - | is a `*mut u8` - | - = help: `const` operands must be of an integer type - error: invalid asm output --> $DIR/type-check-1.rs:14:29 | @@ -142,7 +122,37 @@ LL | asm!("{}", const 0 as *mut u8); | = help: `const` operands must be of an integer type -error: aborting due to 16 previous errors +error: invalid type for `const` operand + --> $DIR/type-check-1.rs:62:20 + | +LL | asm!("{}", const &0); + | ^^^^^^-- + | | + | is a `&i32` + | + = help: `const` operands must be of an integer type + +error: invalid type for `const` operand + --> $DIR/type-check-1.rs:76:19 + | +LL | global_asm!("{}", const 0f32); + | ^^^^^^---- + | | + | is an `f32` + | + = help: `const` operands must be of an integer type + +error: invalid type for `const` operand + --> $DIR/type-check-1.rs:78:19 + | +LL | global_asm!("{}", const 0 as *mut u8); + | ^^^^^^------------ + | | + | is a `*mut u8` + | + = help: `const` operands must be of an integer type + +error: aborting due to 17 previous errors Some errors have detailed explanations: E0277, E0435. For more information about an error, try `rustc --explain E0277`. diff --git a/tests/ui/asm/x86_64/type-check-2.stderr b/tests/ui/asm/x86_64/type-check-2.stderr index 6ae118b16e766..e82a7c92664e5 100644 --- a/tests/ui/asm/x86_64/type-check-2.stderr +++ b/tests/ui/asm/x86_64/type-check-2.stderr @@ -6,22 +6,6 @@ LL | asm!("{}", sym x); | = help: `sym` operands must refer to either a function or a static -error: invalid `sym` operand - --> $DIR/type-check-2.rs:89: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:36: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:43:32 | @@ -79,6 +63,14 @@ LL | asm!("{}", inout(reg) r); | = note: only integers, floats, SIMD vectors, pointers and function pointers can be used as arguments for inline assembly +error: invalid `sym` operand + --> $DIR/type-check-2.rs:36:20 + | +LL | asm!("{}", sym C); + | ^^^^^ is an `i32` + | + = help: `sym` operands must refer to either a function or a static + error[E0381]: used binding `x` isn't initialized --> $DIR/type-check-2.rs:15:28 | @@ -121,6 +113,14 @@ help: consider changing this to be mutable LL | let mut v: Vec = vec![0, 1, 2]; | +++ +error: invalid `sym` operand + --> $DIR/type-check-2.rs:89:19 + | +LL | global_asm!("{}", sym C); + | ^^^^^ is an `i32` + | + = help: `sym` operands must refer to either a function or a static + error: aborting due to 13 previous errors Some errors have detailed explanations: E0381, E0596. From ed35fe3c9d0825a5abdfda0295d054541bfb6f32 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Mon, 22 Jul 2024 20:44:32 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Amanieu d'Antras --- compiler/rustc_hir_analysis/src/check/intrinsicck.rs | 2 +- compiler/rustc_hir_analysis/src/collect/type_of.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/intrinsicck.rs b/compiler/rustc_hir_analysis/src/check/intrinsicck.rs index 6ea8e838d5cee..847a1e6470679 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsicck.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsicck.rs @@ -466,7 +466,7 @@ impl<'a, 'tcx> InlineAsmCtxt<'a, 'tcx> { hir::InlineAsmOperand::SymFn { anon_const } => { debug_assert!(matches!( self.tcx.type_of(anon_const.def_id).instantiate_identity().kind(), - ty::Error(_) | ty::Never | ty::FnDef(..) + ty::Error(_) | ty::FnDef(..) )); } // AST lowering guarantees that SymStatic points to a static. diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index a0d438caf8f93..001b2190fc715 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -65,7 +65,7 @@ fn anon_const_type_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Ty<'tcx> { let ty = tcx.typeck(def_id).node_type(hir_id); match ty.kind() { - ty::Never | ty::Error(_) => ty, + ty::Error(_) => ty, ty::FnDef(..) => ty, _ => { tcx.dcx() @@ -80,7 +80,7 @@ fn anon_const_type_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Ty<'tcx> { Ty::new_error_with_message( tcx, span, - format!("invalid type for `const` operand"), + format!("invalid type for `sym` operand"), ) } }