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

Expand docs on Macros By Example. #511

Merged
merged 4 commits into from
Mar 14, 2019
Merged

Conversation

alercah
Copy link
Contributor

@alercah alercah commented Jan 15, 2019

The primary motivation here was to increase clarity and fully address the
scoping and naming details. The inclusion of RFC 550's formal specification is
to move it to the reference where it can be updated. I made several changes,
motivated by accommodating ? and new fragment specifiers, but there are some
other things which need highlighting so that they can be double-checked for
correctness.

  • Permit the empty string to follow on in the first invariant; this is a
    technical oversight in the definition I believe.
  • Added a requirement that repetitions obey the follow rules; this was an
    oversight in the original RFC and currently planned for fix.
  • Rewrote the definition of FIRST for complex NTs to be more clear.
  • Added a case to LAST for ? repetitions
  • Removed the last example of LAST, because it is wrong.
  • Rearranged the definition of FOLLOW to be more clear
  • Added Shl to FOLLOW(ty) and FOLLOW(path), as documented in the Reference
    already.
  • Added missing follow sets for newer fragment specifiers.

The scoping text is probably not completely accurate, but it's certainly much
better than what was there before (i.e. basically nothing).

@alercah
Copy link
Contributor Author

alercah commented Jan 15, 2019

@petrochenkov
Copy link
Contributor

Reviewed.
Great work.

@nikomatsakis
Copy link
Contributor

Not sure what they are up to these days, but @paulstansifer was the original author of the algorithm we use and I don't think the approach has changed much. There is at least some chance they have the time to review this PR =)

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, I have been looking forward to seeing this! Just yesterday I got a weird "import resolution is stuck, try simplifying macro imports" error, and I think this makes it clearer for me!

I've been wondering if it would be useful to make it clear somewhere that $crate is two separate tokens? There are a few places in the reference that abut tokens in such a way that makes it not clear that they are distinct. It's a minor issue, and can be inferred from the tokens chapter, just a thought.

src/macros-by-example.md Outdated Show resolved Hide resolved
src/macros-by-example.md Outdated Show resolved Hide resolved
src/macros-by-example.md Outdated Show resolved Hide resolved
src/macro-ambiguity.md Outdated Show resolved Hide resolved
@alexreg
Copy link
Contributor

alexreg commented Jan 16, 2019

It looks like the macro guru @petrochenkov has already reviewed, so my input probably isn't needed -- but in brief, good work. :-)

@alercah alercah force-pushed the macro-names branch 2 times, most recently from 0573e53 to def43e0 Compare January 19, 2019 17:36
correct because in order for M to accept ε in the complex NT case, both the
complex NT and α must accept it. If OP = `+`, meaning that the complex NT
cannot be empty, then by definition ε ∉ ALPHA\_SET(M). Otherwise, the complex NT
can accept zero repititions, and then ALPHA\_SET(M) = FOLLOW(`α`). So this
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: repititions -> repetitions here and in few other places below.

Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly proofreading...

src/macro-ambiguity.md Outdated Show resolved Hide resolved
src/macro-ambiguity.md Outdated Show resolved Hide resolved
src/macro-ambiguity.md Outdated Show resolved Hide resolved
src/macro-ambiguity.md Outdated Show resolved Hide resolved
src/macro-ambiguity.md Outdated Show resolved Hide resolved
src/macros-by-example.md Outdated Show resolved Hide resolved
src/macros-by-example.md Outdated Show resolved Hide resolved
src/macros-by-example.md Outdated Show resolved Hide resolved

// m!(); // Error: m is not in scope.
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd break into a section for #[macro_use] here.

src/macros-by-example.md Show resolved Hide resolved
@jethrogb
Copy link
Contributor

jethrogb commented Jan 28, 2019

Can you add something along these lines?

When forwarding a matched fragment to another macro-by-example, matchers in the second macro will see an opaque AST of the fragment type. The second macro can't use literal tokens to match this in the matcher, only a fragment specifier of the same type. The ident, lifetime, and tt fragment types are an exception, and can be matched by literal tokens.

This explains the difference between:

// compiles OK
macro_rules! foo {
    ($l:tt) => { bar!($l); }
}

macro_rules! bar {
    (3) => {}
}

fn main() {
    foo!(3);
}
macro_rules! foo {
    ($l:expr) => { bar!($l); }
// ERROR:               ^^ no rules expected this token in macro call
}

macro_rules! bar {
    (3) => {}
}

fn main() {
    foo!(3);
}

@Centril
Copy link
Contributor

Centril commented Mar 8, 2019

Ping @alercah, there are some unaddressed review comments but otherwise this looks real good :)

@alercah
Copy link
Contributor Author

alercah commented Mar 11, 2019

I'm unable to work on this at the moment, someone else is welcome to bring it home or else I will get to it when I can.

The primary motivation here was to increase clarity and fully address the
scoping and naming details. The inclusion of RFC 550's formal specification is
to move it to the reference where it can be updated. I made several changes,
motivated by accommodating `?` and new fragment specifiers, but there are some
other things which need highlighting so that they can be double-checked for
correctness.

  * Permit the empty string to follow on in the first invariant; this is a
    technical oversight in the definition I believe.
  * Added a requirement that repetitions obey the follow rules; this was an
    oversight in the original RFC and currently planned for fix.
  * Rewrote the definition of FIRST for complex NTs to be more clear.
  * Added a case to LAST for `?` repetitions
  * Removed the last example of LAST, because it is wrong.
  * Rearranged the definition of FOLLOW to be more clear
  * Added Shl to FOLLOW(ty) and FOLLOW(path), as documented in the Reference
    already.
  * Added missing follow sets for newer fragment specifiers.

The scoping text is probably not completely accurate, but it's certainly much
better than what was there before (i.e. basically nothing).
- Address review comments.
- Minor typo and formatting fixes.
- Link `macro_use` and `macro_export` to their new home.
@ehuss
Copy link
Contributor

ehuss commented Mar 11, 2019

I'd be happy to address the final bits of the review. I think this is great content, and would like to see it published. I have rebased and pushed a small update.

src/macros-by-example.md Outdated Show resolved Hide resolved
@paulstansifer
Copy link

@nikomatsakis Thanks for thinking of me! I'm too busy to do anything other than skim this and say that it looks like a great resource. I also couldn't vet the technical stuff too well; the stuff with FIRST and FOLLOW is from after my time; I was pleasantly surprised when the compiler sprung it on me.

Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alercah; @ehuss thanks also for the fixes; Merge when you want :)

@ehuss ehuss merged commit 3c10c95 into rust-lang:master Mar 14, 2019
@ehuss
Copy link
Contributor

ehuss commented Mar 14, 2019

Merging so I can use it, thanks everyone!

Centril added a commit to Centril/rust that referenced this pull request Mar 29, 2019
Update books

Update reference, book, rust-by-example, edition-guide, embedded-book

## reference

15 commits in 41493ff..27ad493
2019-03-05 12:32:22 +0100 to 2019-03-26 02:06:15 +0100
- Document wasm_import_module for #[link]. (rust-lang/reference#554)
- Fix tidy error. (rust-lang/reference#552)
- Some minor contributing updates. (rust-lang/reference#551)
- Document `type_length_limit`. (rust-lang/reference#546)
- Add some terms to the glossary. (rust-lang/reference#547)
- Document `target_feature` and `cfg_target_feature`. (rust-lang/reference#545)
- Remove undocumented page (rust-lang/reference#539)
- Reorg and update attributes (rust-lang/reference#537)
- Fix some minor link errors. (rust-lang/reference#538)
- Add linkchecker. (rust-lang/reference#521)
- Expand docs on Macros By Example. (rust-lang/reference#511)
- document #[panic_handler] (rust-lang/reference#362)
- document #[used] (rust-lang/reference#361)
- Note that UB is program-global (rust-lang/reference#490)
- Fix copy-paste error in procedural-macros.md (rust-lang/reference#533)

## book

16 commits in 9cffbeabec3bcec42d09432bfe7705125c848889..b93ec30bbc7b1b5c2f44223249ab359bed2ed5a6
2019-03-02 08:22:41 -0500 to 2019-03-26 16:54:10 -0400
- Unignore example that now compiles
- Fix code snippet (rust-lang/book#1863)
- Fix mdbook link text in readme (rust-lang/book#1881)
- Wrap to 80 cols
- Make sentence more complete (rust-lang/book#1885)
- consistenly use increment and decrement (rust-lang/book#1884)
- Fix link to Reference's conditional-compilation. (rust-lang/book#1878)
- Fix subject/verb agreement
- Remove nostarch snapshot files that have been incorporated and checked
- haha teach the dictionary steve's name
- Add authorship info to the front page
- fix accidental <ol>'s (rust-lang/book#1866)
- Edits to Macros (rust-lang/book#1848)
- Mention `lock` returns `MutexGuard` wrapped in a `LockResult`
- Add an example that illustrates NLL (rust-lang/book#1842)
- change the parameter name from `type` to `kind` (rust-lang/book#1845)

## rust-by-example

33 commits in 2ce92beabb912d417a7314d6da83ac9b50dc2afb..f68ef3d0f4959f6a7d92a08d9994b117f0f4d32d
2018-11-20 10:10:23 -0500 to 2019-03-12 15:32:12 -0300
- Fix some broken links. (rust-lang/rust-by-example#1161)
- Update links in README (rust-lang/rust-by-example#1167)
- Add score/lifetimes/trait.md (rust-lang/rust-by-example#1168)
- Fix rust-lang/rust-by-example#1147 - No more `open_mode` method (rust-lang/rust-by-example#1164)
- Fix for loop description in list print example (rust-lang/rust-by-example#1162)
- Add link to Cargo chapter in the index page (rust-lang/rust-by-example#1159)
- Fix grammar in sentence about integer notation (rust-lang/rust-by-example#1157)
- Do not use deprecated functions from `std::error::Error` trait (rust-lang/rust-by-example#1151)
- Update new_types.md to clarify conversion to base type (rust-lang/rust-by-example#1148)
- Fix compatibility with Rust 2018 (rust-lang/rust-by-example#1150)
- Hello: Fix hint link in `fmt` chapter. (rust-lang/rust-by-example#1146)
- Clarify pub(restricted) example a bit (rust-lang/rust-by-example#1133)
- Add "literal" to list of macro designators (rust-lang/rust-by-example#1153)
- Minor fixes for the macros chapter (rust-lang/rust-by-example#1113)
- Use new book links instead of the old second-edition ones (rust-lang/rust-by-example#1143)
- Recommend implementing Display over ToString (rust-lang/rust-by-example#1145)
- Remove unused import and format with `rustfmt` (rust-lang/rust-by-example#1144)
- fix typo (rust-lang/rust-by-example#1142)
- Update syntax for 2018 Edition (rust-lang/rust-by-example#1136)
- Added two missing full stops (rust-lang/rust-by-example#1138)
- Removed unnecessary spaces before macro designators in macros/dry (rust-lang/rust-by-example#1139)
- fix install mdbook command (rust-lang/rust-by-example#1128)
- Changed word `function` to `type` in comment of fn area (rust-lang/rust-by-example#1132)
- Added two missing backticks in generics/multi_bounds (rust-lang/rust-by-example#1129)
- Fixed small logic error in error/option_unwrap/and_then (rust-lang/rust-by-example#1127)
- Fix typo (rust-lang/rust-by-example#1125)
- The code of conduct link was dead. I fixed it. (rust-lang/rust-by-example#1122)
- I added a space in the Display fmt for Complex (rust-lang/rust-by-example#1123)
- Fix Rust install link in the index (rust-lang/rust-by-example#1124)
- Update cargo conventions section (rust-lang/rust-by-example#1121)
- Fixed curly braces in the `To and from Strings` chapter to be parentheses (rust-lang/rust-by-example#1120)
- Edit a typo (rust-lang/rust-by-example#1119)
- Fixes rust-lang/rust-by-example#1115 by correcting the typo from into_iterator to into_iter (rust-lang/rust-by-example#1118)

## edition-guide

1 commits in aa0022c875907886cae8f3ef8e9ebf6e2a5e728d..b56ddb11548450a6df4edd1ed571b2bc304eb9e6
2019-02-27 22:10:39 -0800 to 2019-03-10 10:23:16 +0100
- Links fixes (rust-lang/edition-guide#133)

## embedded-book

6 commits in 9e656ead82bfe869493dec82653a52e27fa6a05c..07fd3880ea0874d82b1d9ed30ad3427ec98b4e8a
2019-03-03 16:03:26 +0000 to 2019-03-27 15:40:52 +0000
- Fix test errors.  (rust-embedded/book#180)
- Update qemu.md  (rust-embedded/book#170)
- Update no-std.md to remove obsolete FAQ link  (rust-embedded/book#177)
- We've come a long way :)  (rust-embedded/book#176)
- Correct link to team  (rust-embedded/book#175)
- Update some book links to their new homes.  (rust-embedded/book#173)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants