Should use `extern "system"` as much as possible

[Berrysoft]

Suggestion

extern "system" is designed for Windows API, which uses "stdcall" on x86 and "C" on other archs. windows_targets::link now distinguishes them again. It becomes very annoying because previously I use unsafe extern "system" fn function pointers to store some windows APIs but now they become "C" on non-x86. I don't get the point why you distinguish them.

[kennykerr]

I'm not really sure what you're alluding to. Is this in reference to #3622 or something else?

[Berrysoft]

I think so. #3622 modifies crates/libs/targets/src/lib.rs.

[RalfJung]

I think is is related to my question here: for all non-variadic functions, windows-sys should be able to use extern "system" on all targets: Windows or not, x86-32 or not. For variadic functions it should be able to use extern "C" everywhere. (I think we should allow variadic "system" functions and make them behave like "C", which is already implemented but still pending stabilization: rust-lang/rust#136946.)

now they become "C" on non-x86

#3622 didn't change this though... already before that PR, it was "system" on i686-windows and "C" on x86_64-windows. That PR changed some "cdecl" to "C", but this does not seem to be your problem. It also changed the ABI on non-Windows from "system" to "C", but I assume you are building for a Windows target here.

[ChrisDenton]

Hm, the link macro could be made to convert "cdecl" to "C" and leave other abis as-is.

That would avoid the future incompatibilities while allowing "system" to pass through unchanged.

[ChrisDenton]

But yes, I'm confused by your PR because it changes something that existed before my PR.

[Berrysoft]

#3622 didn't change this though... already before that PR, it was "system" on i686-windows and "C" on x86_64-windows.

Before that PR, only when raw-dylib enabled, it was "C" on x86_64-windows. That PR changed the behavior.

[Berrysoft]

But yes, I'm confused by your PR because it changes something that existed before my PR.

The first commit reverts some changes in your PR. The second & thrid commits just want to unify the behaviors between whether raw-dylib is enabled.

[RalfJung]

There was only one commit in your PR when I looked at it. ;)

[Berrysoft]

Basically I'm against to this change: https://github.com/microsoft/windows-rs/pull/3622/files#diff-6fd35ee92167be2217aca197f703a29f6835760ee209be67a60d7e08e879bbc5

[kennykerr]

Chris Denton (@ChrisDenton) made the change #3622 intentionally and I was under the impression that it was required for future compatibility. Is that not so?

This issue also does not have a minimal repro that illustrates an actual problem that's being solved by #3627 - that would be helpful in discussing and validating a solution. 😊