Skip to content

Commit

Permalink
Rollup merge of #95553 - jam1garner:naked-function-compile-error, r=t…
Browse files Browse the repository at this point in the history
…miasko

Don't emit non-asm contents error for naked function composed of errors

## Motivation

For naked functions an error is emitted when they are composed of anything other than a single asm!() block. However, this error triggers in a couple situations in which it adds no additional information or is actively misleading.

One example is if you do have an asm!() block but simply one with a syntax error:
```rust
#[naked]
unsafe extern "C" fn compiler_errors() {
    asm!(invalid_syntax)
}
```

This results in two errors, one for the syntax error itself and another telling you that you need an asm block in your function:

```rust
error[E0787]: naked functions must contain a single asm block
 --> src/main.rs:6:1
  |
6 | / unsafe extern "C" fn naked_compile_error() {
7 | |     asm!(blah)
8 | | }
  | |_^
```

This issue also comes up when [utilizing `compile_error!()` for improving your diagnostics](https://twitter.com/steveklabnik/status/1509538243020218372), such as raising a compiler error when compiling for an unsupported target.

## Implementation

The rules this PR implements are as follows:

1. If any non-erroneous  non-asm statement is included, an error will still occur
2. If multiple asm statements are included, an error will still occur
3. If 0 or 1 asm statements are present, as well as any non-zero number of erroneous statements, then this error will *not* be raised as it is likely either redundant or incorrect

The rule of thumb is effectively "if an error is present and its correction could change things, don't raise an error".
  • Loading branch information
Dylan-DPC authored Apr 3, 2022
2 parents 796bc7e + bf26d87 commit 19a90c7
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 4 deletions.
25 changes: 22 additions & 3 deletions compiler/rustc_passes/src/naked_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl<'tcx> Visitor<'tcx> for CheckParameters<'tcx> {
fn check_asm<'tcx>(tcx: TyCtxt<'tcx>, body: &'tcx hir::Body<'tcx>, fn_span: Span) {
let mut this = CheckInlineAssembly { tcx, items: Vec::new() };
this.visit_body(body);
if let [(ItemKind::Asm, _)] = this.items[..] {
if let [(ItemKind::Asm | ItemKind::Err, _)] = this.items[..] {
// Ok.
} else {
let mut diag = struct_span_err!(
Expand All @@ -156,19 +156,33 @@ fn check_asm<'tcx>(tcx: TyCtxt<'tcx>, body: &'tcx hir::Body<'tcx>, fn_span: Span
E0787,
"naked functions must contain a single asm block"
);

let mut must_show_error = false;
let mut has_asm = false;
let mut has_err = false;
for &(kind, span) in &this.items {
match kind {
ItemKind::Asm if has_asm => {
must_show_error = true;
diag.span_label(span, "multiple asm blocks are unsupported in naked functions");
}
ItemKind::Asm => has_asm = true,
ItemKind::NonAsm => {
must_show_error = true;
diag.span_label(span, "non-asm is unsupported in naked functions");
}
ItemKind::Err => has_err = true,
}
}
diag.emit();

// If the naked function only contains a single asm block and a non-zero number of
// errors, then don't show an additional error. This allows for appending/prepending
// `compile_error!("...")` statements and reduces error noise.
if must_show_error || !has_err {
diag.emit();
} else {
diag.cancel();
}
}
}

Expand All @@ -181,6 +195,7 @@ struct CheckInlineAssembly<'tcx> {
enum ItemKind {
Asm,
NonAsm,
Err,
}

impl<'tcx> CheckInlineAssembly<'tcx> {
Expand Down Expand Up @@ -222,9 +237,13 @@ impl<'tcx> CheckInlineAssembly<'tcx> {
self.check_inline_asm(asm, span);
}

ExprKind::DropTemps(..) | ExprKind::Block(..) | ExprKind::Err => {
ExprKind::DropTemps(..) | ExprKind::Block(..) => {
hir::intravisit::walk_expr(self, expr);
}

ExprKind::Err => {
self.items.push((ItemKind::Err, span));
}
}
}

Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/asm/naked-functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,22 @@ pub unsafe extern "C" fn inline_never() {
pub unsafe extern "C" fn inline_all() {
asm!("", options(noreturn));
}

#[naked]
pub unsafe extern "C" fn allow_compile_error(a: u32) -> u32 {
compile_error!("this is a user specified error")
//~^ ERROR this is a user specified error
}

#[naked]
pub unsafe extern "C" fn allow_compile_error_and_asm(a: u32) -> u32 {
compile_error!("this is a user specified error");
//~^ ERROR this is a user specified error
asm!("", options(noreturn))
}

#[naked]
pub unsafe extern "C" fn invalid_asm_syntax(a: u32) -> u32 {
asm!(invalid_syntax)
//~^ ERROR asm template must be a string literal
}
20 changes: 19 additions & 1 deletion src/test/ui/asm/naked-functions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,24 @@ error: asm with the `pure` option must have at least one output
LL | asm!("", options(readonly, nostack), options(pure));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^

error: this is a user specified error
--> $DIR/naked-functions.rs:202:5
|
LL | compile_error!("this is a user specified error")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: this is a user specified error
--> $DIR/naked-functions.rs:208:5
|
LL | compile_error!("this is a user specified error");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: asm template must be a string literal
--> $DIR/naked-functions.rs:215:10
|
LL | asm!(invalid_syntax)
| ^^^^^^^^^^^^^^

error: patterns not allowed in naked function parameters
--> $DIR/naked-functions.rs:20:5
|
Expand Down Expand Up @@ -280,6 +298,6 @@ error: naked functions cannot be inlined
LL | #[inline(never)]
| ^^^^^^^^^^^^^^^^

error: aborting due to 30 previous errors; 2 warnings emitted
error: aborting due to 33 previous errors; 2 warnings emitted

For more information about this error, try `rustc --explain E0787`.

0 comments on commit 19a90c7

Please sign in to comment.