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

Allow extern types in #[repr(transparent)] structs #55624

Closed
wants to merge 2 commits into from

Conversation

mjbshaw
Copy link
Contributor

@mjbshaw mjbshaw commented Nov 2, 2018

Fixes #55541

This should be okay given that #[repr(transparent)] requires a single non-ZST field, and all other fields must be ZSTs with alignment 1.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 2, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Nov 2, 2018

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Nov 2, 2018

📌 Commit c3964b3a29e591e4d068c1780c7270ff045c3eb6 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 2, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 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.
travis_time:end:0d9ee38e:start=1541194990057349859,finish=1541195061059559613,duration=71002209754
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---
[00:48:56] .................................................................................................... 3700/4983
[00:48:57] ......................i............................................................................. 3800/4983
[00:49:00] .................................................................................................... 3900/4983
[00:49:03] .................................................................................................... 4000/4983
[00:49:06] ...........................................................................................F........ 4100/4983
[00:49:14] .................................................................................................... 4300/4983
[00:49:18] .................................................................................................... 4400/4983
[00:49:20] .................................................................................................... 4500/4983
[00:49:24] ......................................................i............................................. 4600/4983
[00:49:24] ......................................................i............................................. 4600/4983
[00:49:28] .................................................................................................... 4700/4983
[00:49:31] .................................................................................................... 4800/4983
[00:49:33] .................................................................................................... 4900/4983
35] +    |                                 ^^^^^^^^
[00:49:35] + 
[00:49:35] + error[E0691]: zero-sized field in transparent struct has alignment larger than 1
[00:49:35] +   --> $DIR/repr-transparent.rs:60:30
[00:49:35] +    |
[00:49:35] + LL | struct GenericAlignExtern<T>(ZstAlign32<T>, ExternType); //~ ERROR alignment larger than 1
[00:49:35] + 
[00:49:35] + error: aborting due to 10 previous errors
[00:49:35] 70 
[00:49:35] 71 Some errors occurred: E0690, E0691.
[00:49:35] 71 Some errors occurred: E0690, E0691.
[00:49:35] 72 For more information about an error, try `rustc --explain E0690`.
[00:49:35] 
[00:49:35] 
[00:49:35] The actual stderr differed from the expected stderr.
[00:49:35] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/repr/repr-transparent/repr-transparent.stderr
[00:49:35] To update references, rerun the tests and pass the `--bless` flag
[00:49:35] To only update this specific test, also pass `--test-args repr/repr-transparent.rs`
[00:49:35] error: 1 errors occurred comparing output.
[00:49:35] status: exit code: 1
[00:49:35] status: exit code: 1
[00:49:35] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/repr/repr-transparent.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/repr/repr-transparent/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/repr/repr-transparent/auxiliary" "-A" "eld, but has 2\n    unit: U,\n}\n```\n\nBecause transparent structs are represented exactly like one of their fields at\nrun time, said field must be uniquely determined. If there is no field, or if\nthere are multiple fields, it is not clear how the struct should be represented.\nNote that fields of zero-typed types (e.g., `PhantomData`) can also exist\nalongside the field that contains the actual data, they do not count for this\nerror. When generic types are involved (as in the above example), an error is\nreported because the type parameter could be non-zero-sized.\n\nTo combine `repr(transparent)` with type parameters, `PhantomData` may be\nuseful:\n\n```\nuse std::marker::PhantomData;\n\n#[repr(transparent)]\nstruct LengthWithUnit<U> {\n    value: f32,\n    unit: PhantomData<U>,\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/repr/repr-transparent.rs","byte_start":981,"byte_end":1020,"line_start":27,"line_end":27,"column_start":1,"column_end":40,"is_primary":true,"text":[{"text":"struct ContainsOnlyZstArray([bool; 0]); //~ ERROR needs exactly one non-zero-sized field","highlight_start":1,"highlight_end":40}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"non-zero-sized field","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0690]: transparent struct needs exactly one non-zero-sized field, but has 0\n  --> /checkout/src/test/ui/repr/repr-transparent.rs:27:1\n   |\nLL | struct ContainsOnlyZstArray([bool; 0]); //~ ERROR needs exactly one non-zero-sized field\n   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n   |\n   = note: non-zero-sized field\n\n"}
[00:49:35] {"message":"transparent struct needs exactly one non-zero-sized field, but has 0","code":{"code":"E0690","explanation":"\nA struct with the representation hint `repr(transparent)` had zero or more than\non fields that were not guaranteed to be zero-sized.\n\nErroneous code example:\n\n```compile_fail,E0690\n#[repr(transparent)]\nstruct LengthWithUnit<U> { // error: transparent struct needs exactly one\n    value: f32,            //        non-zero-sized field, but has 2\n    unit: U,\n}\n```\n\nBecause transparent structs are represented exactly like one of their fields at\nrun time, said field must be uniquely determined. If there is no field, or if\nthere are multiple fields, it is not clear how the struct should be represented.\nNote that fields of zero-typed types (e.g., `PhantomData`) can also exist\nalongside the field that contains the actual data, they do not count for this\nerror. When generic types are involved (as in the above example), an error is\nreported because the type parameter could be non-zero-sized.\n\nTo combine `repr(transparent)` with type parameters, `PhantomData` may be\nuseful:\n\n```\nuse std::marker::PhantomData;\n\n#[repr(transparent)]\nstruct LengthWithUnit<U> {\n    value: f32,\n    unit: PhantomData<U>,\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/repr/repr-transparent.rs","byte_start":1092,"byte_end":1154,"line_start":30,"line_end":30,"column_start":1,"column_end":63,"is_primary":true,"text":[{"text":"struct ContainsMultipleZst(PhantomData<*const i32>, NoFields);","highlight_start":1,"highlight_ parameters, `PhantomData` may be\nuseful:\n\n```\nuse std::marker::PhantomData;\n\n#[repr(transparent)]\nstruct LengthWithUnit<U> {\n    value: f32,\n    unit: PhantomData<U>,\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/repr/repr-transparent.rs","byte_start":1227,"byte_end":1257,"line_start":34,"line_end":34,"column_start":1,"column_end":31,"is_primary":true,"text":[{"text":"struct MultipleNonZst(u8, u8); //~ ERROR needs exactly one non-zero-sized field","highlight_start":1,"highlight_end":31}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"non-zero-sized field","code":null,"level":"note","spans":[{"file_name":"/checkout/src/test/ui/repr/repr-transparent.rs","byte_start":1249,"byte_end":1251,"line_start":34,"line_end":34,"column_start":23,"column_end":25,"is_primary":true,"text":[{"text":"struct MultipleNonZst(u8, u8); //~ ERROR needs exactly one non-zero-sized field","highlight_start":23,"highlight_end":25}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null},{"file_name":"/checkout/src/test/ui/repr/repr-transparent.rs","byte_start":1253,"byte_end":1255,"line_start":34,"line_end":34,"column_start":27,"column_end":29,"is_primary":true,"text":[{"text":"struct MultipleNonZst(u8, u8); //~ ERROR needs exactly one non-zero-sized field","highlight_start":27,"highlight_end":29}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":null}],"rendered":"error[E0690]: transparent struct needs exactly one non-zero-sized field, but has 2\n  --> /checkout/src/test/ui/repr/repr-transparent.rs:34:1\n   |\nLL | struct MultipleNonZst(u8, u8); //~ ERROR needs exactly one non-zero-sized field\n   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n   |\nnote: non-zero-sized field\n  --> /checkout/src/test/ui/repr/repr-transparent.rs:34:23\n   |\nLL | struct MultipleNonZst(u8, u8); //~ ERROR needs exactly one non-zero-sized field\n   |                       ^^  ^^\n\n"}
[00:49:35] {"message":"transparent struct needs exactly one non-zero-sized field, but has 2","code":{"code":"E0690","explanation":"\nA struct with the representation hint `repr(transparent)` had zero or more than\non fields that were not guaranteed to be zero-sized.\n\nErroneous code example:\n\n```compile_fail,E0690\n#[repr(transparent)]\nstruct LengthWithUnit<U> { // error: transparent struct needs exactly one\n    value: f32,            //        non-zero-sized field, but has 2\n    unit: U,\n}\n```\n\nBecause transparent structs are represented exactly like one of their fields at\nrun time, said field must be uniquely determined. If there is no field, or if\nthere are multiple fields, it is not clear how the struct should be represented.\nNote that fields of zero-typed types (e.g., `PhantomData`) can also exist\nalongside the field that contains the actual data, they do not count for this\nerror. When generic types are involved (as in the above example), an error is\nreported because the type parameter could be non-zero-sized.\n\nTo combine `repr(transparent)` with type parameters, `PhantomData` may be\nuseful:\n\n```\nuse std::marker::PhantomData;\n\n#[repr(transparent)]\nstruct LengthWithUnit<U> {\n    value: f3n-zero-sized field\n  --> /checkout/src/test/ui/repr/repr-transparent.rs:40:33\n   |\nLL | pub struct StructWithProjection(f32, <f32 as Mirror>::It);\n   |                                 ^^^  ^^^^^^^^^^^^^^^^^^^\n\n"}
[00:49:35] {"message":"zero-sized field in transparent struct has alignment larger than 1","code":{"code":"E0691","explanation":"\nA struct with the `repr(transparent)` representation hint contains a zero-sized\nfield that requires non-trivial alignment.\n\nErroneous code example:\n\n```compile_fail,E0691\n#![feature(repr_align)]\n\n#[repr(align(32))]\nstruct ForceAlign32;\n\n#[repr(transparent)]\nstruct Wrapper(f32, ForceAlign32); // error: zero-sized field in transparent\n                                   //        struct has alignment larger than 1\n```\n\nA transparent struct is supposed to be represented exactly like the piece of\ndata it contains. Zero-sized fields with different alignment requirements\npotentially conflict with this property. In the example above, `Wrapper` would\nhave to be aligned to 32 bytes even though `f32` has a smaller alignment\nrequirement.\n\nConsider removing the over-aligned zero-sized field:\n\n```\n#[repr(transparent)]\nstruct Wrapper(f32);\n```\n\nAlternatively, `PhantomData<T>` has alignment 1 for all `T`, so you can use it\nif you need to keep the field for some reason:\n\n```\n#![feature(repr_align)]\n\nuse std::marker::PhantomData;\n\n#[repr(align(32))]\nstruct ForceAlign32;\n\n#[repr(transparent)]\nstruct Wrapper(f32, PhantomData<ForceAlign32>);\n```\n\nNote that empty arrays `[T; 0]` have the same alignment requirement as the\nelement type `T`. Also note that the e above, `Wrapper` would\nhave to be aligned to 32 bytes even though `f32` has a smaller alignment\nrequirement.\n\nConsider removing the over-aligned zero-sized field:\n\n```\n#[repr(transparent)]\nstruct Wrapper(f32);\n```\n\nAlternatively, `PhantomData<T>` has alignment 1 for all `T`, so you can use it\nif you need to keep the field for some reason:\n\n```\n#![feature(repr_align)]\n\nuse std::marker::PhantomData;\n\n#[repr(align(32))]\nstruct ForceAlign32;\n\n#[repr(transparent)]\nstruct Wrapper(f32, PhantomData<ForceAlign32>);\n```\n\nNote that empty arrays `[T; 0]` have the same alignment requirement as the\nelement type `T`. Also note that the error is conservatively reported even when\nthe alignment of the zero-sized type is less than or equal to the data field's\nalignment.\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/repr/repr-transparent.rs","byte_start":1723,"byte_end":1736,"line_start":50,"line_end":50,"column_start":24,"column_end":37,"is_primary":true,"text":[{"text":"struct GenericAlign<T>(ZstAlign32<T>, u32); //~ ERROR alignment larger than 1","highlight_start":24,"highlight_end":37}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error[E0691]: zero-sized field in transparent struct has alignment larger than 1\n  --> /checkout/src/test/ui/repr/repr-transparent.rs:50:24\n   |\nLL | struct GenericAlign<T>(ZstAlign32<T>, u32); //~ ERROR alignment larger than 1\n   |                        ^^^^^^^^^^^^^\n\n"}
[00:49:35] {"message":"zero-sized field in transparent struct has alignment larger than 1","code":{"code":"E0691","e try `rustc --explain E0690`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about an error, try `rustc --explain E0690`.\n"}
[00:49:35] ------------------------------------------
[00:49:35] 
[00:49:35] thread '[ui] ui/repr/repr-transparent.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3284:9
[00:49:35] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
[00:49:35] 
[00:49:35] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:503:22
[00:49:35] 
[00:49:35] 
[00:49:35] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:49:35] 
[00:49:35] 
[00:49:35] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:49:35] Build completed unsuccessfully in 0:03:37
[00:49:35] Build completed unsuccessfully in 0:03:37
[00:49:35] make: *** [check] Error 1
[00:49:35] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0d47fac0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:02789484:start=1541198047972089228,finish=1541198047977509434,duration=5420206
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:092d79d6
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!check

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)

@mjbshaw
Copy link
Contributor Author

mjbshaw commented Nov 2, 2018

Oops, my last commit forgot to update the test's .stderr. I've revised the commit and re-pushed the fix.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 2, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Nov 2, 2018

📌 Commit ffdd4b9 has been approved by oli-obk

@rust-highfive
Copy link
Collaborator

The job mingw-check 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.
travis_time:end:21889ed0:start=1541198584863713828,finish=1541198645628076899,duration=60764363071
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=mingw-check
---
##################################################                        69.9%
######################################################################## 100.0%
[00:02:01] extracting /checkout/obj/build/cache/2018-10-30/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:02:03]     Updating crates.io index
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)

@mjbshaw
Copy link
Contributor Author

mjbshaw commented Nov 3, 2018

That failure looks like some transient network error. Is there a way to rerun the tests? Or does that require an authorized user to submit that command?

@kennytm
Copy link
Member

kennytm commented Nov 3, 2018

Restarted the check. But the Pull Request CI failing won't block it from being merged anyway.

@eddyb
Copy link
Member

eddyb commented Nov 6, 2018

@bors r-

@oli-obk This conflicts with #55672, which has the right fix IMO.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 6, 2018
@RalfJung
Copy link
Member

RalfJung commented Nov 6, 2018

oops I had no idea this PR exists.

@mjbshaw
Copy link
Contributor Author

mjbshaw commented Nov 6, 2018

Sorry, I'm still figuring out who/when to cc people on pull requests. I'm fine with closing this pull request in favor of @RalfJung's.

@mjbshaw mjbshaw closed this Nov 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants