Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize #![feature(repr_align_enum)] in Rust 1.37.0 #61229

Merged
merged 2 commits into from Jun 10, 2019

Conversation

Projects
None yet
5 participants
@Centril
Copy link
Member

commented May 27, 2019

On an enum item, you may now write:

#[repr(align(X))]
enum Foo {
    // ...
}

This has equivalent effects to first defining:

#[repr(align(X))]
struct AlignX<T>(T);

and then using AlignX<Foo> in Foo's stead.

r? @nagisa

@Centril

This comment has been minimized.

Copy link
Member Author

commented May 27, 2019

Stabilization proposal

I propose that we stabilize #![feature(repr_align_enum)].

@rfcbot merge

Tracking issue: #57996
Version target: 1.37 (2019-07-04 => beta, 2019-08-15 => stable).

What is stabilized

On an enum item, you may now write:

#[repr(align(X))]
enum Foo {
    // ...
}

This has equivalent effects to first defining:

#[repr(align(X))]
struct AlignX<T>(T);

and then using AlignX<Foo> in Foo's stead.

The documentation on #[repr(align(X))] may be found in the reference.

What is not stabilized

There's not much to say here. #[repr(align(X))] is already allowed on struct and union items. As this extends align(X) to enum items, the story for align(X) with respect to sorts of type definitions should be complete.

Divergences from RFC

There was no RFC. Rather, we decided in a language team meeting to accept this experimentally since we felt that it was a minor consistent extension of an already existing concept.

Tests

The tests can be primarily seen in the PR itself. Here are some of them:

Motivation

As aforementioned, the effects of this proposal with respect to dynamic semantics can be achieved through a generic wrapper struct AlignX<T>. However, it is not particularly convenient to achieve the desired effect through AlignX<T> particularly when the enum in question is part of a public API. This disconnects said enum from the alignment and infects pattern matching, construction, and denoting the type-with-alignment. By permitting #[repr(align(X))] on enum items, both ease of writing and reading can be improved.

History

  • Originally hinted at in RFC 1358 as a backwards compatible extension to align(X).

  • Proposed and implemented in #57998 by @niklasf.

    The PR was merged on 2019-02-07. Today is 2019-05-07. This means that the PR was merged over 16 weeks ago, well within acceptable bake-times.

    The PR was reviewed by @nagisa.

@rfcbot

This comment has been minimized.

Copy link

commented May 27, 2019

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@Centril

This comment has been minimized.

Copy link
Member Author

commented May 27, 2019

N.B. Aaron is on leave so I've checked his box.

@Centril

This comment has been minimized.

Copy link
Member Author

commented May 30, 2019

@rfcbot review

@rfcbot

This comment has been minimized.

Copy link

commented May 30, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@nagisa

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

r=me on the implementation.

@rfcbot

This comment has been minimized.

Copy link

commented Jun 9, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@Centril

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2019

@bors r=nagisa rollup

@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

📌 Commit c25e3d2 has been approved by nagisa

@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

⌛️ Testing commit c25e3d2 with merge 0fd591e...

bors added a commit that referenced this pull request Jun 9, 2019

Auto merge of #61229 - Centril:stabilize-repr_align_enum, r=nagisa
Stabilize #![feature(repr_align_enum)] in Rust 1.37.0

On an `enum` item, you may now write:

```rust
#[repr(align(X))]
enum Foo {
    // ...
}
```

This has equivalent effects to first defining:

```rust
#[repr(align(X))]
struct AlignX<T>(T);
```

and then using `AlignX<Foo>` in `Foo`'s stead.

r? @nagisa
@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

💔 Test failed - checks-travis

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Jun 9, 2019

The job dist-i686-apple of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
wry
xmoto
zxing-cpp
==> Downloading https://homebrew.bintray.com/bottles/xz-5.2.4.high_sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/e7/e7be50f4ee00e35887f3957263334eb3baba59e8c061919060f9259351be6880?__gda__=exp=1560122410~hmac=ff3b1fd66b475b6357fb85564f91f7d0e95a25d42fb11c788a3b5fcc5039ac25&response-content-disposition=attachment%3Bfilename%3D%22xz-5.2.4.high_sierra.bottle.tar.gz%22&response-content-type=application%2Fgzip&requestInfo=U2FsdGVkX1-ppjJMwH4RuT0t2QPM7jLUyxDjIHi9ORa_cKJcchX2SkhU6ocu6CXQFBgA41Ayr6X7gryyOLHixWdWbRHI2xHU63l6-PlEYCoix26ap16DqYzRrUVu2e_A6ksWJQUCoxcbWBShi-aBTQ&response-X-Checksum-Sha1=32dc0b28e61f32b40c20e2993418aa8cb6e746d5&response-X-Checksum-Sha2=e7be50f4ee00e35887f3957263334eb3baba59e8c061919060f9259351be6880
🍺  /usr/local/Cellar/xz/5.2.4: 92 files, 1MB
==> `brew cleanup` has not been run in 30 days, running now...
Removing: /Users/travis/Library/Caches/Homebrew/boost-1.66.0.high_sierra.bottle.tar.gz... (84.6MB)
Removing: /Users/travis/Library/Caches/Homebrew/carthage-0.28.0.high_sierra.bottle.tar.gz... (8.3MB)
---
Pruned 0 symbolic links and 5 directories from /usr/local
==> Installing dependencies for swig: pcre
==> Installing swig dependency: pcre
==> Downloading https://homebrew.bintray.com/bottles/pcre-8.43.high_sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/03/0389911a93a88efd4a69b52dea8ecb872fdb55bcfff45d2f7313be5f79730861?__gda__=exp=1560122422~hmac=4910980d19e42b0c47c538ae0e50dbc88998de365a8b867635c1ce5346e5435d&response-content-disposition=attachment%3Bfilename%3D%22pcre-8.43.high_sierra.bottle.tar.gz%22&response-content-type=application%2Fgzip&requestInfo=U2FsdGVkX19gO6txci2x5E6RRpnZanHZzxUuAuiv9JIgpd2xiXC1DLoNpJGAkLe6qw7kPoW2ytxqjrUxeUeRcyKZ7i9DsynDPo8veBnmlb8X2KuxdooiCAo1319vzfn9vL9iPLZiS91sDdRKuFrHrA&response-X-Checksum-Sha1=c67d4b99bb245f0ea56b34118dd6325b06a7250c&response-X-Checksum-Sha2=0389911a93a88efd4a69b52dea8ecb872fdb55bcfff45d2f7313be5f79730861
🍺  /usr/local/Cellar/pcre/8.43: 204 files, 5.5MB
==> Installing swig
==> Downloading https://homebrew.bintray.com/bottles/swig-4.0.0.high_sierra.bottle.tar.gz
==> Downloading https://homebrew.bintray.com/bottles/swig-4.0.0.high_sierra.bottle.tar.gz
==> Downloading from https://akamai.bintray.com/ae/aed79cb436b3a0ac5812c4085e3121ffd62866397b8c7eaa06815ed8ec1e22b7?__gda__=exp=1560122425~hmac=a61ffb58af1e6ab0b66819e8d7392115e7ab0823371a75bb2614f738484cd3db&response-content-disposition=attachment%3Bfilename%3D%22swig-4.0.0.high_sierra.bottle.tar.gz%22&response-content-type=application%2Fgzip&requestInfo=U2FsdGVkX1-N90OjFFvEWwgzdOVOuKc5HjVYYEPZcI0oRGXd2X3-mWE4A5Sl0nYelONALRnJ7WkZz7_Q-b6qIQsyIefbJAd_-x-Fqik_E8FOZzu1WO_82jbYXlWlz3R6gBoUNANDQTvTVXKNjahwoQ&response-X-Checksum-Sha1=a9c428aee4337d91061a69c02d7ae508b627d03f&response-X-Checksum-Sha2=aed79cb436b3a0ac5812c4085e3121ffd62866397b8c7eaa06815ed8ec1e22b7
🍺  /usr/local/Cellar/swig/4.0.0: 722 files, 5.4MB
travis_time:end:0288dc8c:start=1560121361975314000,finish=1560121742640534000,duration=380665220000
travis_fold:end:install
travis_fold:start:before_script.1
---
[00:03:20]       Memory: 8 GB
[00:03:20]       Boot ROM Version: VMW71.00V.7581552.B64.1801142334
[00:03:20]       Apple ROM Info: [MS_VM_CERT/SHA1/27d66596a61c48dd3dc7216fd715126e33f59ae7]Welcome to the Virtual Machine
[00:03:20]       SMC Version (system): 2.8f0
[00:03:20]       Serial Number (system): VMMnv4xSQhlq
[00:03:20] 
[00:03:20] hw.ncpu: 4
[00:03:20] hw.byteorder: 1234
[00:03:20] hw.memsize: 8589934592
---
[00:07:57] [RUSTC-TIMING] syntax_pos test:false 5.630
[00:07:57]    Compiling rustc_errors v0.0.0 (/Users/travis/build/rust-lang/rust/src/librustc_errors)
[00:08:08] [RUSTC-TIMING] rustc_errors test:false 11.135
[00:08:18] [RUSTC-TIMING] rustc_target test:false 20.884
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Centril

This comment has been minimized.

Copy link
Member Author

commented Jun 9, 2019

@bors retry spurious 30 min

@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

⌛️ Testing commit c25e3d2 with merge 61a60ce...

bors added a commit that referenced this pull request Jun 9, 2019

Auto merge of #61229 - Centril:stabilize-repr_align_enum, r=nagisa
Stabilize #![feature(repr_align_enum)] in Rust 1.37.0

On an `enum` item, you may now write:

```rust
#[repr(align(X))]
enum Foo {
    // ...
}
```

This has equivalent effects to first defining:

```rust
#[repr(align(X))]
struct AlignX<T>(T);
```

and then using `AlignX<Foo>` in `Foo`'s stead.

r? @nagisa
@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: nagisa
Pushing 61a60ce to master...

@bors bors added the merged-by-bors label Jun 10, 2019

@bors bors merged commit c25e3d2 into rust-lang:master Jun 10, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@Centril Centril deleted the Centril:stabilize-repr_align_enum branch Jun 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.