Skip to content
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

Clarify eager macro explanation #1872

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Arthur-Milchior
Copy link
Contributor

This CL tries to improve the text around the order in which macro are expanded in various related way.

  1. Remove duplication of "smoother".

Its first occurrence is

a smoother user experience. As an example

It seems that we'll have an example of a smooth user experience. It is not the case, we have an example of macro expansion. Removing this occurrence of "smooth" help understand this won't be considered in the following example.

As the same "smooth" explanation is given below, nothing is lost.

  1. Use an actual example of where eager expansion is needed

This example comes, modified, from
https://doc.rust-lang.org/std/macro.compile_error.html . It uses cfg to cause a compile error if something is not as expected.

As far as my understanding of rust goes, this could not be done with call-by-name evaluation.

At the same time, this example shows the steps of
rewritting. (Approximatively. I still use code represented as strings instead of a list of token/an AST, for the sake of the readability)

  1. Not all macros are in the queue

According to the way step 1 of the algorithm is currently written, it seems that, in foo!(bar!(baz));, both foo!(bar!(baz)) and bar!(baz) should be in the queue. This is false, as bar! should not be evaluated until foo! is.

So the rewritting state that the traversal does not enter unevaluated macros. It also add a note about the exception of non-eager macros.

Technically, it should state that, for non-eager macro, the macro itself should not be evaluated until all of its argument containing macros are evaluated. But I guess it's already fit in the case "macro expansion is not resolved".

This CL tries to improve the text around the order in which macro are expanded
in various related way.

1. Remove duplication of "smoother".

Its first occurrence is
> a smoother user experience.  As an example

It seems that we'll have an example of a smooth user experience. It is not the
case, we have an example of macro expansion. Removing this occurrence of
"smooth" help understand this won't be considered in the following example.

As the same "smooth" explanation is given below, nothing is lost.

2. Use an actual example of where eager expansion is needed

This example comes, modified, from
https://doc.rust-lang.org/std/macro.compile_error.html . It uses cfg to cause a
compile error if something is not as expected.

As far as my understanding of rust goes, this could not be done with
call-by-name evaluation.

At the same time, this example shows the steps of
rewritting. (Approximatively. I still use  code represented as strings instead
of a list of token/an AST, for the sake of the readability)

3. Not all macros are in the queue

According to the way step 1 of the algorithm is currently written, it seems
that, in `foo!(bar!(baz));`, both `foo!(bar!(baz))` and `bar!(baz)` should be in
the queue. This is false, as `bar!` should not be evaluated until `foo!` is.

So the rewritting state that the traversal does not enter unevaluated macros. It
also add a note about the exception of non-eager macros.

Technically, it should state that, for non-eager macro, the macro itself should
not be evaluated until all of its argument containing macros are evaluated.
But I guess it's already fit in the case "macro expansion is not resolved".
@jieyouxu jieyouxu added the waiting-on-review This PR is waiting for a reviewer to verify its content label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-review This PR is waiting for a reviewer to verify its content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants