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

Avoid more symbol interning #61035

Merged
merged 9 commits into from
May 27, 2019
Merged

Avoid more symbol interning #61035

merged 9 commits into from
May 27, 2019

Conversation

nnethercote
Copy link
Contributor

@nnethercote nnethercote commented May 22, 2019 •
edited
Loading

@nnethercote nnethercote force-pushed the avoid-more-symbol-interning branch from 6ecb042 to 899720f Compare May 22, 2019 09:31
@nnethercote
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented May 22, 2019

⌛ Trying commit 899720f with merge 87c7099...

bors added a commit that referenced this pull request May 22, 2019
@bors
Auto merge of #61035 - nnethercote:avoid-more-symbol-interning, r=<try>
87c7099 
Avoid more symbol interning

r? @ghost
oli-obk
oli-obk reviewed May 22, 2019
View reviewed changes
src/libsyntax/ext/build.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented May 22, 2019

☀️ Try build successful - checks-travis
Build commit: 87c7099

@nnethercote
Copy link
Contributor Author

@rust-timer build 87c7099

@rust-timer
Copy link
Collaborator

Success: Queued 87c7099 with parent 1cc822c, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 87c7099: comparison url

@nnethercote nnethercote force-pushed the avoid-more-symbol-interning branch from 899720f to 8871a5c Compare May 23, 2019 09:18
@nnethercote
Copy link
Contributor Author

r? @petrochenkov

@nnethercote nnethercote force-pushed the avoid-more-symbol-interning branch from 8871a5c to 39e5dea Compare May 23, 2019 09:21
@petrochenkov petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 23, 2019
@petrochenkov
Copy link
Contributor

Changes to libsyntax/parse/literal.rs are going to conflict with #60965, which makes most of them as well, and also refactors literal.rs heavily.

I'd recommend to remove commits modifying it and then reapply the few remaining changes once #60965 lands.

petrochenkov
petrochenkov reviewed May 23, 2019
View reviewed changes
src/libsyntax/ast.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented May 23, 2019

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

@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 May 23, 2019
@nnethercote nnethercote force-pushed the avoid-more-symbol-interning branch from 39e5dea to 71920d3 Compare May 24, 2019 01:42
@nnethercote
Copy link
Contributor Author

@petrochenkov: new code is up. I have addressed all your comments. One of the commits disappeared after rebasing on top of #60965.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 24, 2019

📌 Commit 71920d3 has been approved by petrochenkov

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 24, 2019
@bors
Copy link
Contributor

bors commented May 26, 2019

📌 Commit ad45cd54e1d9ed7098f8743f8298f26182709610 has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 26, 2019
@bors
Copy link
Contributor

bors commented May 27, 2019

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout avoid-more-symbol-interning (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self avoid-more-symbol-interning --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message 
warning: Cannot merge binary files: Cargo.lock (HEAD vs. heads/homu-tmp)
Auto-merging Cargo.lock
CONFLICT (content): Merge conflict in Cargo.lock
Automatic merge failed; fix conflicts and then commit the result.

@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 May 27, 2019
@nnethercote nnethercote force-pushed the avoid-more-symbol-interning branch from ad45cd5 to 33a3206 Compare May 27, 2019 04:06
@nnethercote
Copy link
Contributor Author

Rebased again.

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented May 27, 2019

📌 Commit 33a3206 has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 27, 2019
@bors
Copy link
Contributor

bors commented May 27, 2019

⌛ Testing commit 33a3206 with merge ab7cf71...

bors added a commit that referenced this pull request May 27, 2019
@bors
Auto merge of #61035 - nnethercote:avoid-more-symbol-interning, r=pet…
ab7cf71 
…rochenkov

Avoid more symbol interning

r? @petrochenkov
@bors
Copy link
Contributor

bors commented May 27, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing ab7cf71 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 27, 2019
@bors bors merged commit 33a3206 into rust-lang:master May 27, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #61035!

Tested on commit ab7cf71.
Direct link to PR: #61035

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 27, 2019
@rust-highfive
📣 Toolstate changed by rust-lang/rust#61035!
fe15dfd 
Tested on commit rust-lang/rust@ab7cf71.
Direct link to PR: <rust-lang/rust#61035>

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
@bors bors mentioned this pull request May 27, 2019
Merged
@nnethercote nnethercote deleted the avoid-more-symbol-interning branch May 27, 2019 10:51
@nnethercote nnethercote mentioned this pull request May 29, 2019
Closed
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

@Centril Centril Centril left review comments

@oli-obk oli-obk oli-obk left review comments

Assignees

@petrochenkov petrochenkov

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nnethercote @bors @rust-timer @petrochenkov @rust-highfive @oli-obk @Centril