The macro chapter needs hygiene :P #683
Conversation
|
Sitll WIP, but a note to reviewers: the diff for |
|
Ok, I'm sure there are a bunch of broken links and things, but I think this is ready for some review, @rust-lang/wg-rustc-dev-guide |
There was a problem hiding this comment.
@mark-i-m Tremendous amount of work here. This looks great! I am at the Producing Macro Output section and will get to the rest in the next few days. I figured I would submit what I have right now.
One general comment so far. This is a really lengthy text that goes into great detail. That is good IMO. But I think that a bit of organization would help to guide the reader. I indicated a few areas where section headers might be helpful to separate large text passages. I also suggest moving the lazy vs. eager discussion out of the early part of the text and into a separate linked section at the bottom of the document (or maybe an Appendix?). This dives into a detailed review against an edge case situation and distracts from the message about how default expansion happens.
I think that something like:
"With few exceptions, we use this method on the whole crate (see Lazy vs. Eager Expansion section below for more detailed discussion of edge case expansion issues)"
There are a large number of heirarchy -> hierarchy spelling corrections. I used the GH interface to indicate where they are all located. Please don't accept all of those commits :)
This really looks great Mark. Many thanks for the time and effort that you invested here. I am learning a great deal from this chapter.
src/macro-expansion.md
Outdated
| ### Hygiene | ||
| A lazy expansion would expand `foo!` first. An eager expansion would expand | ||
| `bar!` first. Implementing eager expansion more generally would be challenging, | ||
| but we implement it for a few special built-in macros for the sake of user |
There was a problem hiding this comment.
The rationale indicated here duplicates a line in the paragraph before the code block, but provides different rationale for why there is min support:
This is implemented only for a few special built-in macros that expect literals (it's not a generally available feature of Rust).
Suggest removal of one of them.
There was a problem hiding this comment.
My understanding is that because they only take literals, expanding early produces better UX (but tbh, I don't know how off the top of my head).
I tried to reword to make that a bit clearer.
Co-authored-by: Chris Simpkins <git.simpkins@gmail.com>
|
Thanks @chrissimpkins! I like your suggestions on organization. I think that there are a lot of random details jammed into different places, so if you feel they should be moved, that's helpful feedback. |
|
Is this one ready to merge?. |
|
I was under the impression it was still under review? |
There was a problem hiding this comment.
I read the rest of your changes here @mark-i-m. One very minor typo edit. LGTM!
Does Vadim know that this is available for review?
|
I read this a couple of days ago and it looked good. |
The first commit does some cleanup and resolves #645
I am planning to finally finish turning the discussion on hygiene into a proper section, which resolves #398.
cc #15
EDIT: I think we can consider this to reasonable address #15 too.
closes #15
closes #176