Skip to content

stricter alignment enforcement for ScalarPair and Vector #105006

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Nov 28, 2022

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 28, 2022

@eddyb indicated that alignment violating this check might be a bug. So let's see what the test suite says.

(Only the 2nd commit actually changes behavior... but I couldn't not do that other cleanup.^^)

Does the PR CI runner even enable debug assertions though...?

@rustbot
Copy link
Collaborator

rustbot commented Nov 28, 2022

r? @TaKO8Ki

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

@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 Nov 28, 2022
@RalfJung
Copy link
Member Author

r? @oli-obk
(but not ready to be merged yet)

@RalfJung
Copy link
Member Author

Does the PR CI runner even enable debug assertions though...?

Looks like it does. :)

@RalfJung RalfJung force-pushed the scalar-pair-alignment branch 3 times, most recently from 38ff1c3 to 6289910 Compare November 28, 2022 10:27
@eddyb
Copy link
Member

eddyb commented Nov 28, 2022

Oh, I see what's going on, one has to hide whitespace to review this PR.

@RalfJung
Copy link
Member Author

Or just look at the 2nd commit. The first is a NOp but I couldn't resist doing the de-indentation when I noticed this...

@RalfJung RalfJung force-pushed the scalar-pair-alignment branch 2 times, most recently from f8a03e0 to 6115a39 Compare November 28, 2022 10:37
@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member Author

RalfJung commented Nov 28, 2022

Oh funny, we allow vectors with non-power-of-2 length? Interesting.

@RalfJung RalfJung force-pushed the scalar-pair-alignment branch from 6115a39 to ee7a266 Compare November 28, 2022 12:48
@RalfJung
Copy link
Member Author

Ah this actually passes, nice.

@RalfJung RalfJung force-pushed the scalar-pair-alignment branch from ee7a266 to 406a44b Compare November 28, 2022 14:07
@RalfJung
Copy link
Member Author

@eddyb @oli-obk All right, this is ready for review

@RalfJung RalfJung changed the title experiment: stricter alignment enforcement for ScalarPair stricter alignment enforcement for ScalarPair Nov 28, 2022
@RalfJung RalfJung force-pushed the scalar-pair-alignment branch from 406a44b to 891a4da Compare November 28, 2022 14:10
@eddyb
Copy link
Member

eddyb commented Nov 28, 2022

@bors r+ Thanks!

@bors
Copy link
Collaborator

bors commented Nov 28, 2022

📌 Commit 891a4da has been approved by eddyb

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 Nov 28, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 28, 2022
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#104804 (Rename `ast::Lit` as `ast::MetaItemLit`.)
 - rust-lang#104891 (Add documentation for `has_escaping_bound_vars`)
 - rust-lang#104933 (interpret: remove PartialOrd from a bunch of types that do not have or need a sensible order)
 - rust-lang#104936 (Ignore bivariant parameters in test_type_match.)
 - rust-lang#104954 (make simple check of prinf function)
 - rust-lang#104956 (Avoid ICE if the Clone trait is not found while building error suggestions)
 - rust-lang#104982 (interpret: get rid of run() function)
 - rust-lang#104998 (Update my mailmap)
 - rust-lang#105006 (stricter alignment enforcement for ScalarPair)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@RalfJung RalfJung changed the title stricter alignment enforcement for ScalarPair stricter alignment enforcement for ScalarPair and Vector Nov 28, 2022
@bors bors merged commit 3dfb6ca into rust-lang:master Nov 28, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 28, 2022
@RalfJung RalfJung deleted the scalar-pair-alignment branch December 2, 2022 11:11
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…eddyb

stricter alignment enforcement for ScalarPair

`@eddyb` [indicated](rust-lang#103926 (comment)) that alignment violating this check might be a bug. So let's see what the test suite says.

(Only the 2nd commit actually changes behavior... but I couldn't not do that other cleanup.^^)

Does the PR CI runner even enable debug assertions though...?
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#104804 (Rename `ast::Lit` as `ast::MetaItemLit`.)
 - rust-lang#104891 (Add documentation for `has_escaping_bound_vars`)
 - rust-lang#104933 (interpret: remove PartialOrd from a bunch of types that do not have or need a sensible order)
 - rust-lang#104936 (Ignore bivariant parameters in test_type_match.)
 - rust-lang#104954 (make simple check of prinf function)
 - rust-lang#104956 (Avoid ICE if the Clone trait is not found while building error suggestions)
 - rust-lang#104982 (interpret: get rid of run() function)
 - rust-lang#104998 (Update my mailmap)
 - rust-lang#105006 (stricter alignment enforcement for ScalarPair)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # 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.

7 participants