Add targeted suggestion for incompatible {un,}safe keywords when parsing fn-frontmatter-like fragments

[jieyouxu]

Fixes #133586¹, where previously we had two suggestions for unsafe safe extern {} and safe unsafe extern {} to shuffle the order of unsafe and safe ad infinitum.

This PR introduces a special case for incompatible keywords (currently only limited to unsafe and safe) when parsing fn-frontmatter-like fragments, and produces a specialized help message for that case to use only one of them. In cases where incompatible keywords are used, no automated fix is offered because user intention is unclear.

After this PR, the diagnostics will look like:

error: expected one of `extern` or `fn`, found keyword `unsafe`
  --> $DIR/incompatible-safe-unsafe-keywords-extern-block-1.rs:28:6
   |
LL | safe unsafe extern {}
   |      ^^^^^^ expected one of `extern` or `fn`
   |
help: `safe` and `unsafe` are incompatible, use only one of the keywords
  --> $DIR/incompatible-safe-unsafe-keywords-extern-block-1.rs:28:1
   |
LL | safe unsafe extern {}
   | ^^^^^^^^^^^

error: aborting due to 1 previous error

Note that this is still not super precise. Extern blocks cannot have safe keyword, so in theory one can provide a machine-applicable suggestion to remove the safe keyword on extern block, at the cost of even more complexity that I don't think pulls the extra weight².

Footnotes

1. it's not perfect, but I hope it's at least less confusing ↩

2. it's complicated when the prefixes to the block/fn shaped fragments can include a bunch of other keywords like const or async or whatever, then you can also have optional ABI-strings thrown into the mix. ↩

[rustbot]

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[jieyouxu]

Remark: this is more of a band-aid fix, because this recovery code path is taken when we just so happen to be trying to parse anything that's fn-frontmatter-like that does not immediately include an fn keyword (as part of trying to parse item shaped things). So it's kinda best-effort anyway.

[ionicmc-rs]

safe isnt supposed to be incompatible with unsafe in this context; safe is not valid in this context

safe unsafe extern {} // safe is only valid inside of this block
// Should give the err: safe is only valid inside of an unsafe extern block

so this error should be for the extern block

unsafe extern {
    unsafe safe fn foo() // Err: safe and unsafe are incompatible
}

this is atleast what i understand
to sum up:

safe /* unsafe */ extern { // err: safe is only valid inside of `unsafe extern` blocks
   // ...
}

unsafe extern {
    safe fn foo() -> i32;
    /* implicit unsafe */ fn bar() -> i32;
    unsafe safe fn x() -> i32; // err: safe and unsafe are incompatible
}
// Now foo can be called safely
println!("{}", foo()); // out: {some number, e.g. 42}
// but not bar
println!("{}", bar()) // err: this operation is unsafe

The desired output:

help: safe is only valid inside of `unsafe extern` blocks

error: safe and unsafe are incompatible # at x
help: if you want to mark this function as safe, remove unsafe

# ...

[ionicmc-rs]

I have updated the issue (#133586) to include other cases where safe is recognized as a keyword in one order, but not in the other

pub unsafe safe extern "C" fn x(/* ... */) {
    // ...
}

with the above code, safe is not recognized as a keyword

read the issue for more information

is this a special case or do the fixes fix this issue too?

[jieyouxu]

I suggest filing a separate issue because they are from different recoveries / parsing contexts. The problem is that the safe contextual keyword is located in a prefix position making it very hard to tell what the "actual" fragment is.

[jieyouxu]

Closing this PR because I think the mitigation here isn't correct, the parser might need to be properly fixed to handle the contextual safe keyword properly.