-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Reimplement and complete spanless_eq
for the AST
#15599
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
base: master
Are you sure you want to change the base?
Conversation
r? @Alexendoo rustbot has assigned @Alexendoo. Use |
$( | ||
( | ||
$name::$vname { $($fname: $lname),*}, | ||
$name::$vname { $($fname: $rname),*} | ||
) => { | ||
$(on_unequal!(@$unequal return false);)? | ||
$(eq_enum_field!($(@$unordered)? cx $lname $rname) &&)* true | ||
}, | ||
)* |
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.
Could we use ${concat}
here to allow a similar syntax to the struct definitions?
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 ran into hygiene issues when I tried it the first time, but it might be a thing I can work around.
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.
Tried some other options. The feature is just not implemented enough currently to work.
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.
concat
doesn't work in nested repetitions and pushing the concat
into an inner macro means the generated identifier can only the inner macro can access the name due to hygiene rules. Basically neither of these work:
// with repetitions: ice
$(
(
$var { $($name: ${concat(l, $name)}),* },
$var { $($name: ${concat(r, $name)}),* },
) => $(eq!(${concat(l, $name)} ${concat(r, $name)}))&&*,
)*
// with inner: binding names can't be used outside the `name` macro
$(
(
$var { $($name: name!(l, $name)),* },
$var { $($name: name!(r, $name)),* },
) => $(eq!($name))&&*,
)*
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.
We could accept it as is then, but I'm worried the syntax will be confusing for rustc contributions
A temporary paste
dependency could also work, it's a pretty small dependency at least
c6e5e54
to
91b9b9c
Compare
☔ The latest upstream changes (possibly #15773) made this pull request unmergeable. Please resolve the merge conflicts. |
This replaces the many exported functions that used to exist with a single
spanless_eq
function. The macros implementing this would be derives, but that's not possible so nearly copying the struct/enum definitions is required.The new implementation has full exhaustiveness checking on both fields and variants. This way any changes made to the AST in rustc will require changes here.
changelog: none