Skip to content

Expand docs on rustc_abi::Variants - #163073

Open
ada4a wants to merge 1 commit into
rust-lang:mainfrom
ada4a:push-sqkvvqyooqky
Open

ada4a wants to merge 1 commit into
rust-lang:mainfrom
ada4a:push-sqkvvqyooqky

Conversation

@ada4a

@ada4a ada4a commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

This information is scattered throughout the comments at the use sites. I thought I'd consolidate it.

cc @RalfJung since you seem to be the one who wrote most of these comments

@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 Sep 20, 2026
@rustbot

rustbot commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

r? @oli-obk

rustbot has assigned @oli-obk.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: codegen, compiler
  • codegen, compiler expanded to 77 candidates
  • Random selection from 20 candidates

@RalfJung RalfJung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sounds mostly right. :)

View changes since this review

Comment thread compiler/rustc_abi/src/lib.rs Outdated
Comment thread compiler/rustc_abi/src/lib.rs Outdated
@ada4a ada4a changed the title Expand docs on rustc_abi::Variants::Single Expand docs on rustc_abi::Variants Sep 20, 2026
@ada4a

ada4a commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

Pulled out the enum/coroutine explanation to the top, to avoid repeating it in each variant, with a FIXME for coroutines

Comment thread compiler/rustc_abi/src/lib.rs Outdated
@RalfJung

Copy link
Copy Markdown
Member

Pulled out the enum/coroutine explanation to the top, to avoid repeating it in each variant, with a FIXME for coroutines

I have to admit I'm not a fan of that approach.

I would simply define Single as all inhabited types that are not Multiple, that also avoids the redundancy. ;)

This information is scattered throughout the comments at the use sites.
I thought I'd consolidate it.
@ada4a

ada4a commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

Pulled out the enum/coroutine explanation to the top, to avoid repeating it in each variant, with a FIXME for coroutines

I have to admit I'm not a fan of that approach.

I would simply define Single as all inhabited types that are not Multiple, that also avoids the redundancy. ;)

Hm, let me try that

@ada4a

ada4a commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

Hm, one downside of that is that the explanation of Single.index becomes a bit awkward: currently, I "attach" it to the two described cases, but with the change you propose, those would no longer be present

@oli-obk oli-obk assigned RalfJung and unassigned oli-obk Sep 21, 2026
Comment on lines +2009 to +2010
// FIXME: unclear if coroutines actually obey the same rules as enums, or just always have
// [`Self::Multiple`]

@RalfJung RalfJung Sep 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@oli-obk you might know about this?

AFAIK they are always Multiple.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried looking for the code that does uninhabited variant pruning for enums (to see if it applies to coroutines, too), but wasn't able to find it... So I guess I'll go with coroutines always being Multiple

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it is this function

pub fn layout_of_struct_or_enum<

That is only called for ADTs.

@RalfJung

Copy link
Copy Markdown
Member

@rustbot author

@rustbot rustbot 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 Sep 21, 2026
@rustbot

rustbot commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

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. 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.

4 participants