latest nightly won't compile old versions of winapi

[dimbleby]

As reported at tokio-rs/mio#832 (where I proposed having mio just sidestep this): old versions of winapi fail to build with the nightly compiler.

But @carllerche points out that this seems like a compiler regression, so let's report it here.

Various failures, different in different winapi releases:

2.4:

   Compiling winapi v0.2.4
error[E0391]: cyclic dependency detected
    --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.4\src\sapi.rs:1653:9
     |
1653 | /         SPGO_SRGS as i32 | SPGO_SRGS_MS_SCRIPT as i32 | SPGO_SRGS_W3C_SCRIPT as i32 |
1654 | |         SPGO_SRGS_STG_SCRIPT as i32,
     | |___________________________________^ cyclic reference
     |
note: the cycle begins when const-evaluating `sapi::SPGRAMMAROPTIONS::SPGO_SRGS_SCRIPT::{{initializer}}`...
    --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.4\src\sapi.rs:1653:9
     |
1653 | /         SPGO_SRGS as i32 | SPGO_SRGS_MS_SCRIPT as i32 | SPGO_SRGS_W3C_SCRIPT as i32 |
1654 | |         SPGO_SRGS_STG_SCRIPT as i32,
     | |___________________________________^
note: ...which then requires computing layout of `sapi::SPGRAMMAROPTIONS`...
    --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.4\src\sapi.rs:1653:9
     |
1653 |         SPGO_SRGS as i32 | SPGO_SRGS_MS_SCRIPT as i32 | SPGO_SRGS_W3C_SCRIPT as i32 |
     |         ^^^^^^^^^
     = note: ...which then again requires const-evaluating `sapi::SPGRAMMAROPTIONS::SPGO_SRGS_SCRIPT::{{initializer}}`, completing the cycle.

error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
error: Could not compile `winapi`.

To learn more, run the command again with --verbose.

2.5:

   Compiling winapi v0.2.5
error[E0592]: duplicate definitions with name `item`
    --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.5\src\macros.rs:148:13
     |
148  |                pub unsafe fn $variant(&self) -> &$fieldtype {
     |   _____________-
     |  |_____________|
     | ||
149  | ||                 ::std::mem::transmute(&self.$field)
150  | ||             }
     | ||             -
     | ||_____________|
     | |______________duplicate definitions for `item`
     |                other definition for `item`
     |
    ::: C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.5\src\commctrl.rs:2496:1
     |
2496 |    UNION!(TVINSERTSTRUCTA, itemex, item, item_mut, TV_ITEMA);
     |    ---------------------------------------------------------- in this macro invocation

error[E0592]: duplicate definitions with name `item_mut`
    --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.5\src\macros.rs:152:13
     |
152  |                pub unsafe fn $variantmut(&mut self) -> &mut $fieldtype {
     |   _____________-
     |  |_____________|
     | ||
153  | ||                 ::std::mem::transmute(&mut self.$field)
154  | ||             }
     | ||             -
     | ||_____________|
     | |______________duplicate definitions for `item_mut`
     |                other definition for `item_mut`
     |
    ::: C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.5\src\commctrl.rs:2496:1
     |
2496 |    UNION!(TVINSERTSTRUCTA, itemex, item, item_mut, TV_ITEMA);
     |    ---------------------------------------------------------- in this macro invocation

error: aborting due to 2 previous errors

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

2.6:

   Compiling winapi v0.2.6
error[E0080]: constant evaluation error
   --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.6\src\commctrl.rs:162:31
    |
162 | pub const BCN_FIRST: ::UINT = 0 - 1250;
    |                               ^^^^^^^^ attempt to subtract with overflow

error[E0080]: constant evaluation error
   --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.6\src\commctrl.rs:130:31
    |
130 | pub const HDN_FIRST: ::UINT = 0 - 300;
    |                               ^^^^^^^ attempt to subtract with overflow

error[E0080]: constant evaluation error
   --> C:\Users\dch\.cargo\registry\src\github.com-1ecc6299db9ec823\winapi-0.2.6\src\commctrl.rs:140:31
    |
140 | pub const TBN_FIRST: ::UINT = 0 - 700;
    |                               ^^^^^^^ attempt to subtract with overflow

... etc.

Compiler is rustc 1.27.0-nightly (eeea94c11 2018-04-06)

[oli-obk]

2.6 is expected, how did that ever work?
2.4 is an accepted regression, because the old behaviour didn't make sense. Also noone on crater depended on this version, otherwise I would've caught it and opened a PR.

Not sure what 2.5 is about, probably macro hygiene?

[retep998]

Also noone on crater depended on this version, otherwise I would've caught it and opened a PR.

Crater would never catch it because crater does not test Windows.

Also this is a pretty hilarious amount of breakage from Rust in a single crate. All those promises of stability and backwards compatibility were apparently hollow promises without anything to back them up.

[dimbleby]

Checking further, the middle breakage has made it into the stable compiler but the others have not (yet):

• winapi 2.4 and 2.5 fail to compile with Rust 1.25, both for that macro-y reason
• winapi 2.6 compiles fine

So for two of these three regressions there's still opportunity to decide - or not - that stability is more important than whatever introduced the breakage.

[oli-obk]

The two things I can talk about

2.6 "breakage"

Ah, so 2.6 never actually worked for using the constant. It was possible to create broken constants (which still caused a warning!) but using them also doesn't work on stable:

http://play.integer32.com/?gist=156d69400581c9bea41081be12b703b9&version=stable

We can turn this into a future compat lint if we want to. Opinions?

2.4 breakage

cc @eddyb for the 2.4 breakage.

We can probably reallow this if we want to. A minimal example of this is

enum Foo {
    Bar = Foo::Baa as isize + 1,
    Baa = 42,
}

The issue is that computing Foo::Bar during the layout computation of Foo, requires computing Foo::Baa, which also tries to compute the layout of Foo. We could probably fix this by staying inside the isize layout the entire time.

All those promises of stability and backwards compatibility were apparently hollow promises without anything to back them up.

Miri had to be an all or nothing change, a dual-rail option (switching between both with a runtime flag) would have been unmanageable (trust me, I tried). I am doing my best in fixing up those changes, and we will get these issues addressed in time before the next stable release. Also please note that there has been a warning about the 0usize - 1 inside a constant for a very long time.

[retep998]

For the record that 2.5 breakage is #32247

[pnkfelix]

visited during compiler team triage meeting.

Some thoughts:

• I'd personally like to see a check-only crater run that applies to windows source code.
• Should we consider replacing this issue with three, where each is specifically talking about the particular regression (since three were listed in the description, and they sound like they have pretty different conversations associated with each)

[dimbleby]

re splitting this up:

• per @retep998 "the 0.2.5 breakage" was 'duplicate definitions' regression with winapi 0.2.5 #32247. That was fully two years ago. I see little value in re-opening discussion on that one at this point.
• Looks as though @oli-obk has an open pull request Don't report compile-time errors for promoteds #49779 for "the 0.2.6 breakage". So since that's where we've seen activity so far I suggest that we say that this issue covers that, at least.
• I don't yet see any movement on "the 0.2.4 breakage". So if it would be helpful for you to split that off into a separate issue - fine by me.

[oli-obk]

#49779 does not address this, but I have opened #49791 to discuss it

[dimbleby]

OK. then I guess the opposite of what I said! You already have split out "the 0.2.6 breakage", so what's left to track here is only "the 0.2.4 breakage".

[durka]

The "0.2.4 breakage" hit my (unpublished) crate which had been working since 1.0: https://github.com/haptics-nri/nri/blob/f6347ff03feb51157558d1620078beb2a2862c88/sys/bluefox-sys/src/lib.rs#L234-L245

A warning cycle would have been nice.

This kind of enum comes up all the time in native bindings (e.g. the source of mine), so how are we supposed to work around this breaking change? @oli-obk dismissed this as "the old behavior didn't make sense", but I'm not sure what is wrong, in theory, with my enum? Maybe there should be an exception for C-style (or just #[repr(C)]) enums?

[eddyb]

@durka The cyclical enum definitions only worked by accident / hackily (under quite specific conditions), if only @nikomatsakis had gotten the chance to remove them before 1.0...
But @oli-obk has come up with a slightly more general hack to keep it working (#50072).

[nikomatsakis]

It looks like there have been a number of fixes -- can someone update with current status?

[nikomatsakis]

Closing, remaining regression is covered in #32247 (according to @oli-obk)

[oli-obk]

For future reference:

• The 2.4 breakage is fixed in Allow variant discriminant initializers to refer to other initializer… #50072
• The 2.6 breakage is fixed in Don't report compile-time errors for promoteds #49779