Skip to content

Rewrite procedural macro based on the enum discriminant trick #123

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

Merged
merged 19 commits into from
Feb 28, 2017

Conversation

SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Feb 28, 2017

#122 introduced a procedural component to some macro_rules macros by abusing derive on a dummy type declaration. (Custom derive is available on stable Rust 1.15, but functional procedural macros are not.) It used an attribute to pass data to the procedural macro, but this data is limited to string literals in Rust 1.15.

We can improve on this with an idea from @dtolnay:

#[derive(Something)]
enum Dummy {
    Input = (0, stringify!( ... )).0
}

Instead of a custom attribute, we use an enum with an explicit discriminant (because that’s the only case where a type definition can contain a (const) expression), together with accessing the first field of a literal tuple to ignore other fields.

Instead of just string literals, we can accept arbitrary tokens that stringify! will later be expanded into something parseable in a const expression context. (stringify! doesn’t help in attributes because it is not yet expanded by the time anything other than string literals is rejected.)

The details of this hack are hidden away into a new crate, called procedural-masquerade.


This enables concrete benefits for users of cssparser:

  • match_ignore_ascii_case! now supports the full syntax of match expressions, including alternatives pattern | other_pattern, bindings name @ pattern, and guards pattern if condition.
  • Map values in asci_case_insensitive_phf_map! are now given literally instead of as string literals containing Rust syntax.

This change is Reviewable

@SimonSapin
Copy link
Member Author

r? @nox

Do not merge yet, I’ll port Servo and figure out how to land it without breaking Firefox too much…

@nox
Copy link
Contributor

nox commented Feb 28, 2017

r=me

SimonSapin added a commit to servo/servo that referenced this pull request Feb 28, 2017
@SimonSapin SimonSapin mentioned this pull request Feb 28, 2017
5 tasks
bors-servo pushed a commit to servo/servo that referenced this pull request Feb 28, 2017
Update cssparser

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15766)
<!-- Reviewable:end -->
@SimonSapin
Copy link
Member Author

@bors-servo r=nox

@bors-servo
Copy link
Contributor

📌 Commit 4a46a54 has been approved by nox

@bors-servo
Copy link
Contributor

⌛ Testing commit 4a46a54 with merge 291d79c...

bors-servo pushed a commit that referenced this pull request Feb 28, 2017
Rewrite procedural macro based on the enum discriminant trick

We can improve on this with an idea from @dtolnay:

```rust
enum Dummy {
    Input = (0, stringify!( ... )).0
}
```

Instead of a custom attribute, we use an enum with an explicit discriminant (because that’s the only case where a type definition can contain a (const) expression), together with accessing the first field of a literal tuple to ignore other fields.

Instead of just string literals, we can accept arbitrary tokens that `stringify!` will later be expanded into something parseable in a const expression context. (`stringify!` doesn’t help in attributes because it is not yet expanded by the time anything other than string literals is rejected.)

The details of this hack are hidden away into a new crate, called [`procedural-masquerade`](https://docs.rs/procedural-masquerade/).

----

This enables concrete benefits for users of cssparser:

* `match_ignore_ascii_case!` now supports the full syntax of `match` expressions, including alternatives `pattern | other_pattern`, bindings `name @ pattern`, and guards `pattern if condition`.
* Map values in `asci_case_insensitive_phf_map!` are now given literally instead of as string literals containing Rust syntax.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-cssparser/123)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: nox
Pushing 291d79c to master...

@bors-servo bors-servo merged commit 4a46a54 into master Feb 28, 2017
@SimonSapin SimonSapin deleted the proc-macro-hack branch February 28, 2017 16:02
SimonSapin added a commit to servo/servo that referenced this pull request Feb 28, 2017
bors-servo pushed a commit to servo/servo that referenced this pull request Feb 28, 2017
Update cssparser

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15766)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 1, 2017
…nishearth

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: fbfcfc2dbe838ef011f14a863003a575078f455f

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 0be96fc61d6e7a157d88ce68c85a94344dc46d8e
Manishearth pushed a commit to Manishearth/gecko-dev that referenced this pull request Mar 9, 2017
…nishearth

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: fbfcfc2dbe838ef011f14a863003a575078f455f
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request Mar 13, 2017
…nishearth

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: fbfcfc2dbe838ef011f14a863003a575078f455f
clementmiao pushed a commit to clementmiao/servo that referenced this pull request Apr 7, 2017
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
…nishearth

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: fbfcfc2dbe838ef011f14a863003a575078f455f

UltraBlame original commit: 7b1716fa9983b0662773fea92eb8847edb65557c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
…nishearth

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: fbfcfc2dbe838ef011f14a863003a575078f455f

UltraBlame original commit: 7b1716fa9983b0662773fea92eb8847edb65557c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
…nishearth

<!-- Please describe your changes on the following line: -->

servo/rust-cssparser#123

In particular, `match_ignore_ascii_case` now supports the full `match` syntax.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: fbfcfc2dbe838ef011f14a863003a575078f455f

UltraBlame original commit: 7b1716fa9983b0662773fea92eb8847edb65557c
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.

3 participants