Rustfmt: Allow arbitrary extern ABI

[dtolnay]

Rustfmt fails to format the following file, even though rustc happily compiles it.

I am using cfg here to repro, but the more important case in practice would be to allow non-standard ABI in the input of procedural macro attributes like https://github.com/dtolnay/cxx.

#[cfg(any())]
extern "C++" {}

error[E0703]: invalid ABI: found `C++`
 --> /git/testing/src/main.rs:2:8
  |
2 | extern "C++" {}
  |        ^^^^^ invalid ABI
  |
  = help: valid ABIs: cdecl, stdcall, fastcall, vectorcall, thiscall, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, Rust, C, system, rust-intrinsic, rust-call, platform-intrinsic, unadjusted

[rchaser53]

I cannot reproduce this. Could you tell me the version you use?

[dtolnay]

I guess the error message was from the rustfmt that ships with stable 1.42, but after trying it with today's rustfmt master branch (9124dd88d6ef14d0ae77dc52ff5e9598f24a75a0) the current behavior is even worse; it silently replaces the ABI with "Rust".

$ echo '#[cfg(any())] extern "C++" {}' | rustup run 1.42.0 rustfmt
error[E0703]: invalid ABI: found `C++`
 --> <stdin>:1:22
  |
1 | #[cfg(any())] extern "C++" {}
  |                      ^^^^^ invalid ABI
  |
  = help: valid ABIs: cdecl, stdcall, fastcall, vectorcall, thiscall, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, Rust, C, system, rust-intrinsic, rust-call, platform-intrinsic, unadjusted

$ echo '#[cfg(any())] extern "C++" {}' | rustup run nightly-2020-03-19 rustfmt
#[cfg(any())]
extern "Rust" {}

$ git rev-parse HEAD
9124dd88d6ef14d0ae77dc52ff5e9598f24a75a0

$ echo '#[cfg(any())] extern "C++" {}' | cargo run --bin rustfmt
#[cfg(any())]
extern "Rust" {}

[calebcartwright]

Rust is the fallback in the event of an abi lookup failure, which I believe would happen here since C++ isn't an Abi variant

https://github.com/rust-lang/rustfmt/blob/9124dd88d6ef14d0ae77dc52ff5e9598f24a75a0/rustfmt-core/rustfmt-lib/src/utils.rs#L137-L148

Updating that logic to keep the original string should be doable, though I'm not sure if that'd have any other side effects

[dtolnay]

Updating that logic to keep the original string should be doable, though I'm not sure if that'd have any other side effects

I think keeping the original string would be the obvious correct behavior for rustfmt. It's definitely what I would want in https://github.com/dtolnay/cxx.

[Mark-Simulacrum]

Upstream, I believe the validation of the string was moved from parsing to a later stage recently as well so it's plausible a bump of the syntax version used here would be reasonable as well.

[calebcartwright]

I think keeping the original string would be the obvious correct behavior for rustfmt.

Agreed, just needs some refactoring to support doing so while still honoring the force_explicit_abi option in the other cases

[calebcartwright]

it's plausible a bump of the syntax version used here would be reasonable as well.

Also agreed, though we're going to have some challenges bumping the version of the syntax/ast crate and friends in rustfmt given some of those usptream changes in parsing, especially https://github.com/rust-lang/rust/pull/69838

[calebcartwright]

@rchaser53 are you working on this one already? if not i'll take a look in the next day or so

[Centril]

Upstream, I believe the validation of the string was moved from parsing to a later stage recently as well so it's plausible a bump of the syntax version used here would be reasonable as well.

Yep; the grammar of the language now only cares about string literals, deferring translation actual ABIs (in the sense of rustc_target) to HIR lowering.

(Aside: very nice, and unexpected, to see @dtolnay taking advantage of this =P.)

[rchaser53]

@calebcartwright No, I haven't yet. I cannot have the enough time until this Sunday. So I will leave it to you. Thank you.

[calebcartwright]

We're still a good ways out from being ready to release v2.x of rustfmt, so I've also opened #4089 against the latest 1.x branch in case there's a desire to have the fix available sooner.