Skip to content

Make $crate a keyword #42902

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 2 commits into from
Jun 30, 2017
Merged

Make $crate a keyword #42902

merged 2 commits into from
Jun 30, 2017

Conversation

petrochenkov
Copy link
Contributor

Fixes #42898

r? @jseyfried or @nrc

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 26, 2017
@jseyfried
Copy link
Contributor

LGTM overall. I still don't really think of $crate as a keyword though -- instead I consider macro_rules! to expand it into the hygienic crate root; I think how we represent this is an implementation detail.

If we're making it a keyword though, could we put it next to its counterpart CrateRoot. Putting it with the "real" keywords seems weird to me.

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jun 28, 2017

@jseyfried

I still don't really think of $crate as a keyword though -- instead I consider macro_rules! to expand it into the hygienic crate root; I think how we represent this is an implementation detail.

That's true, it just needs to "quack like a keyword" (path segment keyword, to be precise), i.e. answer "yes" to questions "is it reserved?" and "can it be used in a path segment?". So, making it a keyword is the simplest solution.

If we're making it a keyword though, could we put it next to its counterpart CrateRoot.

I suspect $crate can be lowered directly into CrateRoot with correct hygiene, but I can't check right now.
Or maybe both can be replaced with keywords::Invalid which is usually used for other things similar to crate root (elided lifetimes, unnamed parameters, etc), but that's the opposite to what I'd like to see, I'd rather replace the remaining uses of keywords::Invalid with more specialized "reserved identifier placeholders" instead.

(I'll return to this PR in a few days.)

@jseyfried
Copy link
Contributor

So, making it a keyword is the simplest solution.

Makes sense.

more specialized "reserved identifier placeholders" instead [of keywords::Invalid]

Agreed, I was just saying we should declare the reserved identifier placeholders next to each other, not interspersed with the traditional keywords (or we could also separate/refactor out "reserved identifier placeholders" further).

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jun 29, 2017

@jseyfried

we should declare the reserved identifier placeholders next to each other, not interspersed with the traditional keywords (or we could also separate/refactor out "reserved identifier placeholders" further).

Updated.

@jseyfried
Copy link
Contributor

Thanks! @bors r+

@bors
Copy link
Collaborator

bors commented Jun 29, 2017

📌 Commit b33fd6d has been approved by jseyfried

@bors
Copy link
Collaborator

bors commented Jun 29, 2017

⌛ Testing commit b33fd6d with merge 5eef7c7...

bors added a commit that referenced this pull request Jun 29, 2017
@bors
Copy link
Collaborator

bors commented Jun 30, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: jseyfried
Pushing 5eef7c7 to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants