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

Rename Piece::String to Piece::Lit #135943

Closed
wants to merge 1 commit into from
Closed

Rename Piece::String to Piece::Lit #135943

wants to merge 1 commit into from

Conversation

hkBst
Copy link
Contributor

@hkBst hkBst commented Jan 23, 2025 •
edited
Loading

This renames Piece::String to Piece::Lit to avoid shadowing std::string::String and removes "pub use Piece::*;".

@rustbot
Copy link
Collaborator

rustbot commented Jan 23, 2025

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 23, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

Please in the future indicate clearly when you're stacking changes from another PR, since this contains commits from #135882.

@hkBst
Copy link
Contributor Author

hkBst commented Jan 23, 2025

@compiler-errors, sorry Michael, still figuring out how to best organize my workflow locally :)

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@estebank estebank changed the title simplify and cleanup Rename Piece::String to Piece::Lit and make it easier to check "similar tokens" Jan 24, 2025
estebank
estebank approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Ping me after #135882 lands and this PR gets rebased. r=me.

@hkBst hkBst changed the title Rename Piece::String to Piece::Lit and make it easier to check "similar tokens" Rename Piece::String to Piece::Lit Jan 28, 2025
@hkBst
Copy link
Contributor Author

hkBst commented Jan 28, 2025

@estebank I figured out how to rebase this, so I think this is now reviewable.

estebank
estebank reviewed Jan 28, 2025
View reviewed changes
compiler/rustc_parse_format/src/lib.rs Outdated
S2: Into<string::String>,
S3: Into<string::String>,
>(
fn err_with_note<S1: Into<String>, S2: Into<String>, S3: Into<String>>(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not because of you, it's just a fmt change, but nowadays we'd likely have written these using impl Into<String>.

Copy link
Contributor Author

@hkBst hkBst Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed both here and just above for fn err. And it was already merged so I probably messed up this PR now...

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 28, 2025

📌 Commit 3026545 has been approved by estebank

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 Jan 28, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Jan 29, 2025
@Zalathar
Rollup merge of rust-lang#135943 - hkBst:opt_imports, r=estebank
9567909 
Rename `Piece::String` to `Piece::Lit`

This renames Piece::String to Piece::Lit to avoid shadowing std::string::String and removes "pub use Piece::*;".
@Zalathar Zalathar mentioned this pull request Jan 29, 2025
Closed
fmease added a commit to fmease/rust that referenced this pull request Jan 29, 2025
@fmease
Rollup merge of rust-lang#135943 - hkBst:opt_imports, r=estebank
dbb092b 
Rename `Piece::String` to `Piece::Lit`

This renames Piece::String to Piece::Lit to avoid shadowing std::string::String and removes "pub use Piece::*;".
@fmease fmease mentioned this pull request Jan 29, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2025
@bors
Auto merge of rust-lang#136225 - fmease:rollup-fm7m744, r=fmease
ccc9ba5 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#135625 ([cfg_match] Document the use of expressions.)
 - rust-lang#135902 (Do not consider child bound assumptions for rigid alias)
 - rust-lang#135943 (Rename `Piece::String` to `Piece::Lit`)
 - rust-lang#136104 (Add mermaid graphs of NLL regions and SCCs to polonius MIR dump)
 - rust-lang#136143 (Update books)
 - rust-lang#136147 (ABI-required target features: warn when they are missing in base CPU)
 - rust-lang#136164 (Refactor FnKind variant to hold &Fn)

r? `@ghost`
`@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2025
@rust-timer
Unrolled build for rust-lang#135943
3d3557d 
Rollup merge of rust-lang#135943 - hkBst:opt_imports, r=estebank

Rename `Piece::String` to `Piece::Lit`

This renames Piece::String to Piece::Lit to avoid shadowing std::string::String and removes "pub use Piece::*;".
@bors
Copy link
Contributor

bors commented Jan 29, 2025

☔ The latest upstream changes (presumably #136225) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 29, 2025
@klensy
Copy link
Contributor

klensy commented Jan 29, 2025 •
edited
Loading

Do not push after r+ (and when pr in rollup), this results in probably wrong commit state merged, and now merged in #136225.

@fmease
Copy link
Member

fmease commented Jan 29, 2025 •
edited
Loading

The main changes have effectively been merged by the rollup. Namely commit 3026545.

The following post-r+ force-push was not included on master: https://github.com/rust-lang/rust/compare/3026545ab50e65fcd1c888b77272032000e36147..faa086152b7658f4ed1394b8d82b7055470b61c9.

As [@]klensy wrote, please don't push after an r+ (unless overwritten by a subsequent r-). Closing as effectively merged. There's nothing else we could do (well, we could re-r+ this but that wouldn't be any less confusing for future GitHub archeologists; it'd better to open a new PR for any further modifications unless they're trivial)

@fmease fmease closed this Jan 29, 2025
@hkBst
Copy link
Contributor Author

hkBst commented Jan 29, 2025 •
edited
Loading

Sorry, this was a dumb mistake. New PR at #136251.

I don't suppose there is anything I can do to fix things?

@fmease
Copy link
Member

fmease commented Jan 29, 2025 •
edited
Loading

Hmm, I haven't tried to recover from such a situation before (it's not strictly necessary to do so).

I was about to say, please don't force-push to this branch and don't delete it and I'll see what I can do but I can actually no longer reopen this PR because you reused the branch for your new PR #136251 >.< GH won't let me ("There is already an open pull request from hkBst:opt_imports to rust-lang:master"). I would've probably tried to reset the branch to the old master used originally and cherry-picked the original commit but that could've made this even worse maybe (with GH possibly showing all commits in old_master..current_master in this PR or showing an "empty" diff, idk).

In any case, don't let it trouble you. It's unfortunate but my hands are tied.

@hkBst hkBst deleted the opt_imports branch February 2, 2025 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank estebank approved these changes

Assignees

@estebank estebank

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@hkBst @rustbot @rust-log-analyzer @compiler-errors @estebank @bors @klensy @fmease