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

syntax: Treat error literals in more principled way #61615

Merged
merged 1 commit into from
Jun 8, 2019

Conversation

petrochenkov
Copy link
Contributor

Free them from their character literal origins.

I actually tried to remove LitKind::Err entirely (by converting it into ExprKind::Err immediately), and it caused no diagnostic regressions in the test suite.
However, I'd still want to use error literals as general purpose error tokens some day, so I kept them.

The downside of having LitKind::Err in addition to ExprKind::Err is that every time you want to do something with ExprKind::Err you need to make sure that ExprKind::Lit(LitKind::Err) is treated in the same way.
Fortunately, this usually happens automatically because both literals and errors are "leaf" expressions, however this PR does fix a couple of inconsistencies between them.

Addresses #60679 (comment) in a way

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 7, 2019
@petrochenkov
Copy link
Contributor Author

r? @matklad
@bors delegate=matklad

@bors
Copy link
Contributor

bors commented Jun 7, 2019

✌️ @matklad can now approve this pull request

@matklad
Copy link
Member

matklad commented Jun 7, 2019

However, I'd still want to use error literals as general purpose error tokens some day, so I kept them.

Than maybe there shouldn't be an error literal token, and just an error token?

@petrochenkov
Copy link
Contributor Author

Than maybe there shouldn't be an error literal token, and just an error token?

Well, maybe yes in theory, but it may be more convenient to make that token a literal due to various circumstances.

  • The set of tokens understood by proc macros is closed and any extensions have to go through TokenTree::Literal.
  • The error token needs a payload Symbol which makes it very similar to a literal token.
  • If the error token needs to survive pretty-printing then some ill-formed literal may be the best choice for that.

@matklad
Copy link
Member

matklad commented Jun 7, 2019

not sure about #61615 (comment), but r=me

@petrochenkov
Copy link
Contributor Author

Added a test for #61615 (comment).
@bors r=matklad rollup

@bors
Copy link
Contributor

bors commented Jun 7, 2019

📌 Commit 2af47fa has been approved by matklad

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 7, 2019
Centril added a commit to Centril/rust that referenced this pull request Jun 7, 2019
syntax: Treat error literals in more principled way

Free them from their character literal origins.

I actually tried to remove `LitKind::Err` entirely (by converting it into `ExprKind::Err` immediately), and it caused no diagnostic regressions in the test suite.
However, I'd still want to use error literals as general purpose error tokens some day, so I kept them.

The downside of having `LitKind::Err` in addition to `ExprKind::Err` is that every time you want to do something with `ExprKind::Err` you need to make sure that `ExprKind::Lit(LitKind::Err)` is treated in the same way.
Fortunately, this usually happens automatically because both literals and errors are "leaf" expressions, however this PR does fix a couple of inconsistencies between them.

Addresses rust-lang#60679 (comment) in a way
Centril added a commit to Centril/rust that referenced this pull request Jun 7, 2019
syntax: Treat error literals in more principled way

Free them from their character literal origins.

I actually tried to remove `LitKind::Err` entirely (by converting it into `ExprKind::Err` immediately), and it caused no diagnostic regressions in the test suite.
However, I'd still want to use error literals as general purpose error tokens some day, so I kept them.

The downside of having `LitKind::Err` in addition to `ExprKind::Err` is that every time you want to do something with `ExprKind::Err` you need to make sure that `ExprKind::Lit(LitKind::Err)` is treated in the same way.
Fortunately, this usually happens automatically because both literals and errors are "leaf" expressions, however this PR does fix a couple of inconsistencies between them.

Addresses rust-lang#60679 (comment) in a way
Centril added a commit to Centril/rust that referenced this pull request Jun 7, 2019
syntax: Treat error literals in more principled way

Free them from their character literal origins.

I actually tried to remove `LitKind::Err` entirely (by converting it into `ExprKind::Err` immediately), and it caused no diagnostic regressions in the test suite.
However, I'd still want to use error literals as general purpose error tokens some day, so I kept them.

The downside of having `LitKind::Err` in addition to `ExprKind::Err` is that every time you want to do something with `ExprKind::Err` you need to make sure that `ExprKind::Lit(LitKind::Err)` is treated in the same way.
Fortunately, this usually happens automatically because both literals and errors are "leaf" expressions, however this PR does fix a couple of inconsistencies between them.

Addresses rust-lang#60679 (comment) in a way
Centril added a commit to Centril/rust that referenced this pull request Jun 8, 2019
syntax: Treat error literals in more principled way

Free them from their character literal origins.

I actually tried to remove `LitKind::Err` entirely (by converting it into `ExprKind::Err` immediately), and it caused no diagnostic regressions in the test suite.
However, I'd still want to use error literals as general purpose error tokens some day, so I kept them.

The downside of having `LitKind::Err` in addition to `ExprKind::Err` is that every time you want to do something with `ExprKind::Err` you need to make sure that `ExprKind::Lit(LitKind::Err)` is treated in the same way.
Fortunately, this usually happens automatically because both literals and errors are "leaf" expressions, however this PR does fix a couple of inconsistencies between them.

Addresses rust-lang#60679 (comment) in a way
bors added a commit that referenced this pull request Jun 8, 2019
Rollup of 7 pull requests

Successful merges:

 - #61223 (Document tuple's Ord behavior as sequential)
 - #61615 (syntax: Treat error literals in more principled way)
 - #61616 (parser: Remove `Deref` impl from `Parser`)
 - #61621 (Clarify when we run steps with ONLY_HOSTS)
 - #61627 (Add regression test for #61452.)
 - #61641 (Revert "Make LocalAnalizer visitor iterate instead of recurse")
 - #61647 (Use stable wrappers in f32/f64::signum)

Failed merges:

r? @ghost
@bors bors merged commit 2af47fa into rust-lang:master Jun 8, 2019
@petrochenkov petrochenkov deleted the errlit branch June 8, 2019 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants