Skip to content

Macro cleanups#4959

Open
nnethercote wants to merge 3 commits intorust-lang:mainfrom
nnethercote:s
Open

Macro cleanups#4959
nnethercote wants to merge 3 commits intorust-lang:mainfrom
nnethercote:s

Conversation

@nnethercote
Copy link

I was building rustc with -Zmacro-stats, which drew my attention to the s! macro, and I saw some improvements could be made.

@rustbot

This comment has been minimized.

It's unused. `c_enum!` is now used instead.
It doesn't seem necessary. It dates back to a large commit "Let's just
juggle everything around!" from the very old mega-PR rust-lang#21.
- Fix comments that incorrectly say that `Debug` is part of
  `extra_traits`.
- Use a consistent and sensible ordering for the traits: Clone, Copy,
  Debug; then PartialEq, Eq, Hash.
- Explain `repr` behaviour for `s_paren!`; it's different to `s!` and
  `s_no_extra_traits!`.
- Use `$pub:vis` (instead of a hard-wired `pub`) in `s_paren!` for
  consistency with the other macros.
@nnethercote nnethercote force-pushed the s branch 2 times, most recently from 6d82762 to fc62e18 Compare February 5, 2026 00:56
@tgross35
Copy link
Contributor

tgross35 commented Feb 5, 2026

Most of this looks good but the deprecated change seems to be causing the CI failure. Not sure if there is a good solution there.

@tgross35
Copy link
Contributor

tgross35 commented Feb 5, 2026

Mind posting the before & after macro-stats if you have them handy? I'm curious about the wins.

@nnethercote
Copy link
Author

I removed the commit removing the allow(deprecated).

I don't have the -Zmacro-stats measurements... these changes will have no effect on them. I just mentioned it in case anyone was wondering why on earth I was looking at these macros :)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants