Bad parse error on token sequences safe unsafe and unsafe safe

[ionicmc-rs]

Code

// #1
unsafe extern {
    safe unsafe fn foo();
    unsafe safe fn bar();
}
// #2
unsafe safe fn baz() {}
safe unsafe fn qux() {}
// #3
unsafe safe extern "C" fn ham() {}
safe unsafe extern "C" fn egg() {}

Current output

# example #1:
`safe` must come before `unsafe`: `safe unsafe`
`unsafe` must come before `safe`: `unsafe safe`
# Example #2
items outside an `unsafe extern {...} block may not be annotated with `safe``
`safe` must come before `unsafe`: `safe unsafe`
`unsafe` must come before `safe`: `unsafe safe`
# example #3 is the same as #2

Desired output

# Example #1
error: `safe` and `unsafe` are incompatible
note: extern functions are unsafe by default
help: if you want to make this function safe, remove `unsafe`
# Example #2
error: items outside an `unsafe extern {...}` block may not be annotated with `safe`
help: remove `safe`
# example #3
error: items outside an `unsafe extern {...}` block may not be annotated with `safe`
note: extern functions are safe by default
help: remove `safe`

Rationale and extra context

look at the errors for #2 and #3:

`safe` must come before `unsafe`: `safe unsafe`
`unsafe` must come before `safe`: `unsafe safe`

this means that it thinks that the order is incorrect

which is likely because they are both included in the check for order

Other cases

// #1
unsafe safe static FOO: i32 = 42;
unsafe safe const BAR: i32 = 42;
unsafe safe trait Baz {}
unsafe trait _X {}
unsafe safe impl _X for i32 {}
// #2 
unsafe extern {
    unsafe safe static QUX: usize = 42;
    // the rest of the examples just copy pasted
}

Output:

# #1 
items outside an `unsafe extern` blocks cannot be `safe`
# shuffle safe and unsafe like usual
# #2 
# same as 1 but without the 'items outside `unsafe extern` ...'

Rust Version

$ rustc --version --verbose
rustc 1.83.0 (90b35a623 2024-11-26)
binary: rustc
commit-hash: 90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf
commit-date: 2024-11-26
host: x86_64-unknown-linux-gnu
release: 1.83.0
LLVM version: 19.1.1

Anything else?

This is a follow up to #133586 and #133631

#133586 has the PR #133618
(all of the above are closed)

[compiler-errors]

@ionicmc-rs: E-* issue tags are used as call for participation, not priority. It's probably not wise to add them unless you can gauge the difficulty level from experience with the compiler. I would consider this to be difficult, but also E-hard is reserved for large changes that require mentorship, so it's probably best if you leave the tags alone in this issue.

This is just a diagnostic issue. As alluded to in the PR that attempted to fix a subset of this issue, this requires some delicate treatment around contextual/"soft" keywords because a bad implementation is likely to regress legitimate code if it's not aware of the grammatical implications of the recovery path.

Thanks for raising this issue, but it's also not going to get the issue fixed faster by trying to push on things.

[ionicmc-rs]

@ionicmc-rs: E-* issue tags are used as call for participation, not priority. It's probably not wise to add them unless you can gauge the difficulty level from experience with the compiler. I would consider this to be difficult, but also E-hard is reserved for large changes that require mentorship, so it's probably best if you leave the tags alone in this issue.

This is just a diagnostic issue. As alluded to in the PR that attempted to fix a subset of this issue, this requires some delicate treatment around contextual/"soft" keywords because a bad implementation is likely to regress legitimate code if it's not aware of the grammatical implications of the recovery path.

Thanks for raising this issue, but it's also not going to get the issue fixed faster by trying to push on things.

Ok sorry for my mistake

[ionicmc-rs]

Ok so it seems the issue is just with unsafe safe and safe unsafe and not the actual item

[spastorino]

I'm going to address this at some point when I'm back from vacations and if nobody beats me.
I have a question for @compiler-errors and/or @jieyouxu ... when you say that this is not properly handling safe that is a contextual keyword, I meant the phrase is right ... but should I interpret this also as ... we have infrastructure to handle contextual keywords in the parser and I'm misusing them.
If that's the case please let me know as it seems to me that handling a contextual keyword in the parser it's just a bunch of scattered if statements across the parser.

[jieyouxu]

when you say that this is not properly handling safe that is a contextual keyword, I meant the phrase is right

I specifically mean that the recovery paths seems a bit wonky, because where the safe recovery happens, it's inside parse_fn_front_matter. But, AFAICT, parse_fn_front_matter does not know whether it's within an extern block or not, so it gives the safe/unsafe suggestion sequences even outside of extern blocks.

but should I interpret this also as ... we have infrastructure to handle contextual keywords in the parser and I'm misusing them.

I don't think we have specific contextual keywords infra because... (see below)

If that's the case please let me know as it seems to me that handling a contextual keyword in the parser it's just a bunch of scattered if statements across the parser.

@spastorino I briefly looked at it, it seems like the contextual keyword handling is indeed quite scattered, because "contextual" obviously depends on... context. For safe in particular, it can appear in fn-like frontmatters which makes the recoveries very tricky, because the fn-like frontmatters can have multiple optional modifiers (visibility, ABI string, among others). The implication of that is that you don't have easy fixed n lookaheads to work with.