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

Extend BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE. #107713

Conversation

nnethercote
Copy link
Contributor

To temporarily allow a str field in a packed struct using derive, along with [u8].

r? @RalfJung

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 6, 2023
@RalfJung
Copy link
Member

RalfJung commented Feb 6, 2023

Thanks!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 6, 2023

📌 Commit f075d484589c0f53a474b0afd676c29c66c150d6 has been approved by RalfJung

It is now in the queue for this repository.

@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 Feb 6, 2023
Comment on lines 1562 to 1565
// be removable relatively quickly, because there are
// no known crates for which they make the difference
// between "the crate compiles" and "the crate does not
// compile".
Copy link
Member

@RalfJung RalfJung Feb 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be entirely true, can it? The compiler depends on icu4x and that was still building even before unicode-org/icu4x#3060 got fixed. The [u8] hack and unicode-org/icu4x#2834 were sufficient for that (in fact either one should be sufficient). Looks like only some configurations of icu4x are affected by unicode-org/icu4x#3060, and some other configurations are helped by this hack?

Cc @Manishearth

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ICU4X is many components, the compiler only uses one of them, which doesn't do extremely fancy zero-copy stuff (it only does normal zero-copy stuff).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so there are cases where these hacks actually help the build to succeed.

@nnethercote can you adjust the comment? Then r=me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification. That makes me feel better about the whole lint :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, some cases, though in some cases the last field is a ZeroSlice or something, so I don't think a full ICU4X build is fixed by this. However like I said we have a dep update that fixes this so it's not a huge deal.

@RalfJung
Copy link
Member

RalfJung commented Feb 6, 2023

@bors r-
see comment above

@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 Feb 6, 2023
To temporarily allow a `str` field in a packed struct using `derive`,
along with `[u8]`.
@nnethercote nnethercote force-pushed the extend-BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE branch from f075d48 to a70d03b Compare February 9, 2023 00:47
@nnethercote
Copy link
Contributor Author

I removed the erroneous comment.

@bors r=RalfJung

@bors
Copy link
Contributor

bors commented Feb 9, 2023

📌 Commit a70d03b has been approved by RalfJung

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 9, 2023
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Feb 9, 2023
…ACKED_STRUCT_WITH_DERIVE, r=RalfJung

Extend `BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE`.

To temporarily allow a `str` field in a packed struct using `derive`, along with `[u8]`.

r? `@RalfJung`
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 9, 2023
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#107317 (Implement `AsFd` and `AsRawFd` for `Rc`)
 - rust-lang#107429 (Stabilize feature `cstr_from_bytes_until_nul`)
 - rust-lang#107713 (Extend `BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE`.)
 - rust-lang#107761 (Replace a command line flag with an env var to allow tools to initialize the tracing loggers at their own discretion)
 - rust-lang#107790 ( x.py fails all downloads that use a tempdir with snap curl rust-lang#107722)
 - rust-lang#107799 (correctly update goals in the cache)
 - rust-lang#107813 (Do not eagerly recover for bad `impl Trait` types in macros)
 - rust-lang#107817 (rustdoc: use svgo to shrink `wheel.svg`)
 - rust-lang#107819 (Set `rust-analyzer.check.invocationLocation` to `root`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 04f7708 into rust-lang:master Feb 9, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants