-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert newtype_index
to a proc macro
#93878
Conversation
r? @jackh726 (rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
/// | ||
/// - The `From` impls are the preferred way. So you can do | ||
/// `S::from(v)` with a `usize` or `u32`. And you can convert back | ||
/// to an integer with `u32::from(s)`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this comment is pre-existing, but while we're here: is this advice correct? I personally prefer S::new(v)
and s.index()
, and I feel like I've seen those forms more often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure - I think this could be dealt with in a follow-up issue, since it's just a style reccomendation.
346e2a2
to
3f95400
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great simplification to the expansion!
3f95400
to
cb2e4ee
Compare
@cjgillot I've addressed your comments |
📌 Commit cb2e4ee has been approved by |
Convert `newtype_index` to a proc macro The `macro_rules!` implementation was becomng excessively complicated, and difficult to modify. The new proc macro implementation should make it much easier to add new features (e.g. skipping certain `#[derive]`s)
Convert `newtype_index` to a proc macro The `macro_rules!` implementation was becomng excessively complicated, and difficult to modify. The new proc macro implementation should make it much easier to add new features (e.g. skipping certain `#[derive]`s)
Convert `newtype_index` to a proc macro The `macro_rules!` implementation was becomng excessively complicated, and difficult to modify. The new proc macro implementation should make it much easier to add new features (e.g. skipping certain `#[derive]`s)
Convert `newtype_index` to a proc macro The `macro_rules!` implementation was becomng excessively complicated, and difficult to modify. The new proc macro implementation should make it much easier to add new features (e.g. skipping certain `#[derive]`s)
might be reponsible for these ci failures: #94167 (comment) |
⌛ Testing commit cb2e4ee with merge 0525b7fa548f72c821ac5997db8da88ca7b685e9... |
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
☔ The latest upstream changes (presumably #93984) made this pull request unmergeable. Please resolve the merge conflicts. |
The `macro_rules!` implementation was becomng excessively complicated, and difficult to modify. The new proc macro implementation should make it much easier to add new features (e.g. skipping certain `#[derive]`s)
These links never worked, but the lint was suppressed due to the fact that the span was pointing into the macro. With the new macro implementation, the span now points directly to the doc comment in the macro invocation, so it's no longer suppressed.
cb2e4ee
to
7b7b0f1
Compare
@bors r=cjgillot |
📌 Commit 7b7b0f1 has been approved by |
☀️ Test successful - checks-actions |
Finished benchmarking commit (f6a7993): comparison url. Summary: This benchmark run shows 4 relevant improvements 🎉 to instruction counts.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
The
macro_rules!
implementation was becomng excessively complicated,and difficult to modify. The new proc macro implementation should make
it much easier to add new features (e.g. skipping certain
#[derive]
s)