Skip to content

Conversation

@chenyukang
Copy link
Member

@chenyukang chenyukang commented Nov 7, 2025

Fixes #148634

The ICE happened because

attr::ReprSimd => ReprFlags::IS_SIMD,

will always set IS_SIMD according to get_all_attrs, and since we already report error attribute should be applied to a struct, it's OK to bypass here.

@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 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 7, 2025

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@Kivooeo
Copy link
Member

Kivooeo commented Nov 7, 2025

r=me when ci is green

@rust-log-analyzer

This comment has been minimized.

@lcnr
Copy link
Contributor

lcnr commented Nov 7, 2025

hmm, would it be possible to instead filter the ReprOptions so that we don't end up with unsupported ones? It feels brittle to simply handle incorrect info by ignoring it, e.g. it adds an implicit assumption that repr(simd) is only ever used for structs, and if we were to idk, extend support to unions at some point (even if unlikely) this code would suddenly be incorrect

alternatively, accept all Adts, but then match on the kind and skip + span_delay_bug if it's not a struct

@Kivooeo
Copy link
Member

Kivooeo commented Nov 7, 2025

it adds an implicit assumption that repr(simd) is only ever used for structs

according to the rfc it seems (at least for me) that this is only for structs https://github.com/rust-lang/rfcs/blob/master/text/1199-simd-infrastructure.md#types

@lcnr
Copy link
Contributor

lcnr commented Nov 7, 2025

Yes, but is this a guarantee that we'll never extend it to unions? My point is not that the code is wrong, it's that this code adds an implicit dependency which may result in bugs in the future.

Most bugs in large projects happen by:

  • somebody writes code which has an unchecked assumption which holds right now
  • somebody else modifies another part of the project, causing this assumption to no longer hold, resulting in a bug in entirely untouched code

This implies that a good way to avoid bugs in large projects it to avoid unchecked assumptions. It's why exhaustive matches are so powerful. It's also why we do stuff like

// This causes a compiler error if any new float kinds are added.
let (ty::FloatTy::F16 | ty::FloatTy::F32 | ty::FloatTy::F64 | ty::FloatTy::F128);
let possible_floats = [
ty::SimplifiedType::Float(ty::FloatTy::F16),
ty::SimplifiedType::Float(ty::FloatTy::F32),
ty::SimplifiedType::Float(ty::FloatTy::F64),
ty::SimplifiedType::Float(ty::FloatTy::F128),
];

@Kivooeo Kivooeo 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 7, 2025
@chenyukang chenyukang force-pushed the yukang-fix-148634-repr-simd-enum-ice branch from 995c118 to 112d833 Compare November 8, 2025 01:41
@chenyukang
Copy link
Member Author

@lcnr
I updated the code in commit:
https://github.com/rust-lang/rust/compare/995c11894fdabe1c630694254de756f82389c6cf..112d833ea5c72f89078069ffc5d10da5b1adf2ca

I'm thinking do we need to delay_as_bug, since if we already returned Err(NonAsmTypeReason::Invalid(ty)), there will always an error reported error[E0084]: unsupported representation for ...

@chenyukang
Copy link
Member Author

Ok, span_delay_bug seems checking the whether there is already an error before this point.

@chenyukang
Copy link
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 12, 2025
@lcnr
Copy link
Contributor

lcnr commented Nov 13, 2025

@bors delegate+

@bors
Copy link
Collaborator

bors commented Nov 13, 2025

✌️ @chenyukang, you can now approve this pull request!

If @lcnr told you to "r=me" after making some further change, please make that change, then do @bors r=@lcnr

@chenyukang chenyukang force-pushed the yukang-fix-148634-repr-simd-enum-ice branch from a21210d to df57c32 Compare November 13, 2025 11:32
@chenyukang
Copy link
Member Author

@bors r=Kivooeo,lcnr

@bors
Copy link
Collaborator

bors commented Nov 13, 2025

📌 Commit df57c32 has been approved by Kivooeo,lcnr

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 13, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 13, 2025
…simd-enum-ice, r=Kivooeo,lcnr

Fix ICE for repr simd on non struct

Fixes rust-lang#148634

The ICE happened because
https://github.com/rust-lang/rust/blob/995c11894fdabe1c630694254de756f82389c6cf/compiler/rustc_middle/src/ty/mod.rs#L1531

will always set `IS_SIMD` according to `get_all_attrs`, and since we already report error `attribute should be applied to a struct`, it's OK to bypass here.
@chenyukang
Copy link
Member Author

@bors try jobs=test-various

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 14, 2025
…ce, r=<try>

Fix ICE for repr simd on non struct

try-job: test-various
@rust-log-analyzer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Nov 14, 2025

💔 Test for 2c4f26b failed: CI. Failed jobs:

@chenyukang chenyukang force-pushed the yukang-fix-148634-repr-simd-enum-ice branch from 53cfe97 to f9cf0c4 Compare November 14, 2025 03:10
@chenyukang
Copy link
Member Author

@bors try jobs=test-various

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Nov 14, 2025
…ce, r=<try>

Fix ICE for repr simd on non struct

try-job: test-various
@rust-log-analyzer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Nov 14, 2025

💔 Test for 5ec32cc failed: CI. Failed jobs:

@chenyukang chenyukang force-pushed the yukang-fix-148634-repr-simd-enum-ice branch from f9cf0c4 to 1610851 Compare November 14, 2025 04:43
@chenyukang
Copy link
Member Author

@bors try jobs=test-various

rust-bors bot added a commit that referenced this pull request Nov 14, 2025
…ce, r=<try>

Fix ICE for repr simd on non struct

try-job: test-various
@rust-bors

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Nov 14, 2025

☀️ Try build successful (CI)
Build commit: 9d98235 (9d98235200f597b8afa69e59002efc894f14891b, parent: 7a72c5459dd58f81b0e1a0e5436d145485889375)

@chenyukang
Copy link
Member Author

finally it's passed, I added //@ needs-asm-support in the testcase.
@bors r=Kivooeo,lcnr

@bors
Copy link
Collaborator

bors commented Nov 14, 2025

📌 Commit 1610851 has been approved by Kivooeo,lcnr

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 Nov 14, 2025
@bors
Copy link
Collaborator

bors commented Nov 14, 2025

⌛ Testing commit 1610851 with merge 5d9fe54...

bors added a commit that referenced this pull request Nov 14, 2025
…ce, r=Kivooeo,lcnr

Fix ICE for repr simd on non struct

Fixes #148634

The ICE happened because
https://github.com/rust-lang/rust/blob/995c11894fdabe1c630694254de756f82389c6cf/compiler/rustc_middle/src/ty/mod.rs#L1531

will always set `IS_SIMD` according to `get_all_attrs`, and since we already report error `attribute should be applied to a struct`, it's OK to bypass here.
@Zalathar
Copy link
Member

Yielding to enclosing rollup.

@bors retry

bors added a commit that referenced this pull request Nov 14, 2025
Rollup of 4 pull requests

Successful merges:

 - #148638 (Fix ICE for repr simd on non struct)
 - #148725 (Implement the alternative `try` block desugaring)
 - #148777 (Lock shards while emitting depth limit error.)
 - #148933 (Add note for option llvm.download-ci-llvm)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bf7d553 into rust-lang:main Nov 14, 2025
12 of 13 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 14, 2025
rust-timer added a commit that referenced this pull request Nov 14, 2025
Rollup merge of #148638 - chenyukang:yukang-fix-148634-repr-simd-enum-ice, r=Kivooeo,lcnr

Fix ICE for repr simd on non struct

Fixes #148634

The ICE happened because
https://github.com/rust-lang/rust/blob/995c11894fdabe1c630694254de756f82389c6cf/compiler/rustc_middle/src/ty/mod.rs#L1531

will always set `IS_SIMD` according to `get_all_attrs`, and since we already report error `attribute should be applied to a struct`, it's OK to bypass here.
@bors
Copy link
Collaborator

bors commented Nov 14, 2025

⌛ Testing commit 1610851 with merge c8551d3...

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.

ICE: assertion failed: self.is_struct() || self.is_union()

8 participants