-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add support for stringify! builtin macro #2348
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
Conversation
let macro_call = loc.ast_id.to_node(db); | ||
|
||
let macro_content = { | ||
let arg = macro_call.token_tree().ok_or_else(|| mbe::ExpandError::UnexpectedToken)?; |
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.
Might be a stupid question, but is this the same as _tt
?
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.
There is no spacing information in _tt
but I agree it is more accurate to use _tt
directly here :
The builtin stringify!
macro do not preserve space too.
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.
Might be worth adding a test.
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 think either solution is fine. It's also a shame that we can't easily add a test here, because there's no testing infra in hir_expand
, but I think it makes sense to merge this without blocking on the said infra
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.
bors r+
let macro_call = loc.ast_id.to_node(db); | ||
|
||
let macro_content = { | ||
let arg = macro_call.token_tree().ok_or_else(|| mbe::ExpandError::UnexpectedToken)?; |
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 think either solution is fine. It's also a shame that we can't easily add a test here, because there's no testing infra in hir_expand
, but I think it makes sense to merge this without blocking on the said infra
2348: Add support for stringify! builtin macro r=matklad a=piotr-szpetkowski Refs #2212 First time ever contributing here, hopefully it's ok. 2352: Move TypeAlias to hir_def r=matklad a=matklad Co-authored-by: Piotr Szpetkowski <[email protected]> Co-authored-by: Aleksey Kladov <[email protected]>
Canceled (will resume) |
2348: Add support for stringify! builtin macro r=matklad a=piotr-szpetkowski Refs #2212 First time ever contributing here, hopefully it's ok. 2352: Move TypeAlias to hir_def r=matklad a=matklad Co-authored-by: Piotr Szpetkowski <[email protected]> Co-authored-by: Aleksey Kladov <[email protected]>
Build succeeded
|
Refs #2212
First time ever contributing here, hopefully it's ok.