Skip to content

rustc_builtin_macros cleanup, part 6 - #163211

Open
cyrgani wants to merge 12 commits into
rust-lang:mainfrom
cyrgani:builtin-macros-6
Open

rustc_builtin_macros cleanup, part 6#163211
cyrgani wants to merge 12 commits into
rust-lang:mainfrom
cyrgani:builtin-macros-6

Conversation

@cyrgani

@cyrgani cyrgani commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

As the next step after #163080, this PR removes more unneeded complexity, including the "mini version of Ty" and the FieldInfo for discriminants.

r? @nnethercote

@rustbot

rustbot commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Changes to the code generated for builtin derived traits.

cc @nnethercote

@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 23, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
nnethercote
nnethercote reviewed Sep 23, 2026
View reviewed changes

@nnethercote nnethercote left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mostly looks good. Just one question below.

Also, you are allowed to write commit messages with more than one line :) Sometimes it's helpful to include an extra sentence or two explaining what is going on, or why it's being done.

View changes since this review

Comment thread compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
.collect::<Vec<String>>();
let prefixes: &[&str] = match selflike_args.len() {
1 => &["__self"],
2 => &["__self", "__arg1"],

@nnethercote nnethercote Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is good. The old code seemed designed to be very generic and handle a wide variety of possible method signatures, when in practice they have only a few forms.

Comment thread compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
.map(|prefix| {
let pieces_iter =
struct_def.fields().iter().enumerate().map(|(i, struct_field)| {
let sp = struct_field.span.with_ctxt(self.span.ctxt());

@nnethercote nnethercote Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why can the with_ctxt be removed?

@cyrgani cyrgani Sep 23, 2026

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.

make_pattern_ident creates an ident with span self.span, so it already has the correct context and with_span_pos doesn't look at sps context

Comment thread compiler/rustc_builtin_macros/src/deriving/generic/mod.rs

pub methods: SmallVec<[MethodDef<'a>; 1]>,

pub associated_types: SmallVec<[(Ident, Ty); 1]>,

@nnethercote nnethercote Sep 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wow

@nnethercote nnethercote 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 23, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nnethercote

nnethercote commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

r=me once tests are green.

@bors delegate=cyrgani

@rust-bors

rust-bors Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

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

If @nnethercote told you to "r=me" after making some further change, then please make that change and post @bors r=nnethercote.

View changes since this delegation.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nnethercote nnethercote nnethercote approved these changes

Assignees

@nnethercote nnethercote

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cyrgani @rustbot @rust-log-analyzer @nnethercote