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.

Sign up for GitHub

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

Jump to bottom

Clean up LitKind #100018

Merged
merged 3 commits into from
Aug 17, 2022
Merged

Clean up LitKind #100018

merged 3 commits into from
Aug 17, 2022

Conversation

nnethercote
Copy link
Contributor

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 1, 2022
@rustbot
Copy link
Collaborator

rustbot commented Aug 1, 2022

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 1, 2022
@nnethercote
Copy link
Contributor Author

nnethercote commented Aug 1, 2022 •
edited
Loading

Today I was looking into removing ast::Lit::kind, as per this comment. It's a reasonably large change and I didn't get close to finishing it, but I spotted some places where things could be improved.

Best reviewed one commit at a time.

@petrochenkov
Copy link
Contributor

Today I was looking into removing ast::Lit::kind, as per this comment.

If we are talking about this comment

    /// FIXME: Remove this and only create the semantic representation during lowering to HIR.

then I'm not exactly sure it's a right thing to do, it's more like an invitation to trying and making this experiment.
Maybe the semantic representation is used often enough even before lowering to HIR so it's reasonable to keep it, or maybe not, I don't know.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 1, 2022
@nnethercote
Copy link
Contributor Author

I'm not exactly sure it's a right thing to do, it's more like an invitation to trying and making this experiment. Maybe the semantic representation is used often enough even before lowering to HIR so it's reasonable to keep it, or maybe not, I don't know.

Right, and this experiment is what I'm doing.

@nnethercote
Avoid code duplication in {MetaItem,MetaItemKind}::value_str.
7d1e5a4 
The two methods are almost identical.
@nnethercote
Make ExtCtxt::expr_lit non-pub.
d7a041f 
By using `expr_str` more and adding `expr_{char,byte_str}`.
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nnethercote
Rename some things related to literals.
5d3cc17 
- Rename `ast::Lit::token` as `ast::Lit::token_lit`, because its type is
  `token::Lit`, which is not a token. (This has been confusing me for a
  long time.)
  reasonable because we have an `ast::token::Lit` inside an `ast::Lit`.
- Rename `LitKind::{from,to}_lit_token` as
  `LitKind::{from,to}_token_lit`, to match the above change and
  `token::Lit`.
@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 16, 2022
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 16, 2022

📌 Commit 5d3cc17 has been approved by petrochenkov

It is now in the queue for this repository.

@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 Aug 16, 2022
TaKO8Ki added a commit to TaKO8Ki/rust that referenced this pull request Aug 16, 2022
@TaKO8Ki
Rollup merge of rust-lang#100018 - nnethercote:clean-up-LitKind, r=pe…
7617437 
…trochenkov

Clean up `LitKind`

r? `@petrochenkov`
@TaKO8Ki TaKO8Ki mentioned this pull request Aug 16, 2022
Closed
@nnethercote
Copy link
Contributor Author

@bors rollup=always

TaKO8Ki added a commit to TaKO8Ki/rust that referenced this pull request Aug 16, 2022
@TaKO8Ki
Rollup merge of rust-lang#100018 - nnethercote:clean-up-LitKind, r=pe…
3b0215a 
…trochenkov

Clean up `LitKind`

r? `@petrochenkov`
@TaKO8Ki TaKO8Ki mentioned this pull request Aug 16, 2022
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 17, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 17, 2022
@bors
Auto merge of rust-lang#100677 - matthiaskrgr:rollup-au41ho1, r=matth…
9c20b2a 
…iaskrgr

Rollup of 15 pull requests

Successful merges:

 - rust-lang#99474 (Rustdoc json tests: New `@hasexact` test command)
 - rust-lang#99972 (interpret: only consider 1-ZST when searching for receiver)
 - rust-lang#100018 (Clean up `LitKind`)
 - rust-lang#100379 (triagebot: add translation-related mention groups)
 - rust-lang#100389 (Do not report cycle error when inferring return type for suggestion)
 - rust-lang#100489 (`is_knowable` use `Result` instead of `Option`)
 - rust-lang#100532 (unwind: don't build dependency when building for Miri)
 - rust-lang#100608 (needless separation of impl blocks)
 - rust-lang#100621 (Pass +atomics-32 feature for {arm,thumb}v4t-none-eabi)
 - rust-lang#100646 (Migrate emoji identifier diagnostics to `SessionDiagnostic` in rustc_interface)
 - rust-lang#100652 (Remove deferred sized checks (make them eager))
 - rust-lang#100655 (Update books)
 - rust-lang#100656 (Update cargo)
 - rust-lang#100660 (Fixed a few documentation errors)
 - rust-lang#100661 (Fixed a few documentation errors)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d5dca26 into rust-lang:master Aug 17, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 17, 2022
@nnethercote nnethercote deleted the clean-up-LitKind branch August 17, 2022 21:11
Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 29, 2022
@matthiaskrgr
Rollup merge of rust-lang#100018 - nnethercote:clean-up-LitKind, r=pe…
37eeed7 
…trochenkov

Clean up `LitKind`

r? ``@petrochenkov``
@rust-timer rust-timer mentioned this pull request Sep 1, 2022
Merged
calebcartwright pushed a commit to calebcartwright/rust that referenced this pull request Jan 24, 2023
@matthiaskrgr
Rollup merge of rust-lang#100018 - nnethercote:clean-up-LitKind, r=pe…
870ed72 
…trochenkov

Clean up `LitKind`

r? ``@petrochenkov``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@calebcartwright calebcartwright calebcartwright left review comments

Assignees

@petrochenkov petrochenkov

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.65.0
Development

Successfully merging this pull request may close these issues.
7 participants
@nnethercote @rustbot @petrochenkov @rust-log-analyzer @bors @calebcartwright @rust-highfive