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

Move from {{closure}}#0 syntax to {closure#0} for (def) path components #76176

Merged
merged 11 commits into from
Sep 26, 2020
Merged

Move from {{closure}}#0 syntax to {closure#0} for (def) path components #76176

merged 11 commits into from
Sep 26, 2020

Conversation

marmeladema
Copy link
Contributor

@marmeladema marmeladema commented Aug 31, 2020 •
edited
Loading

Part of #70334

I followed the approach described by @eddyb and introduced a DefPathDataName enum.
To preserve compatibility, in various places, I had to rely on formatting manually by calling format!("{{{{{}}}}}", namespace).

My questions are:

  • Do we want to convert for places to use the new naming scheme? Or shall I re-add DefPathData::as_symbol but renamed as DefPathData::as_legacy_symbol to avoid manually allocating the legacy symbols?
  • Do we want to impl Display for DisambiguatedDefPathData to avoid manually calling write!(s, "{{{}#{}}}", namespace, component.disambiguator)?
  • We might also want to improve naming for DefPathDataName and DefPathData::get_name

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2020
@marmeladema marmeladema force-pushed the fix-closure-path-printing branch 2 times, most recently from 277e65a to 93ce1e4 Compare August 31, 2020 23:43
@marmeladema marmeladema force-pushed the fix-closure-path-printing branch 3 times, most recently from 7588122 to ee81a4e Compare September 1, 2020 18:35
@bors

This comment has been minimized.

Sign in to view
@eddyb
Copy link
Member

eddyb commented Sep 25, 2020

@bors r+ Thanks!

@bors
Copy link
Contributor

bors commented Sep 25, 2020

📌 Commit 6785568870cac091755900f1623795535f41591b has been approved by eddyb

@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 Sep 25, 2020
@eddyb
Copy link
Member

eddyb commented Sep 25, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Sep 25, 2020

📌 Commit 5946c12 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Sep 26, 2020

⌛ Testing commit 5946c12 with merge c6622d1...

@bors
Copy link
Contributor

bors commented Sep 26, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: eddyb
Pushing c6622d1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 26, 2020
@bors bors merged commit c6622d1 into rust-lang:master Sep 26, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 26, 2020
This was referenced Sep 26, 2020
Closed
Closed
Merged
Merged
@marmeladema marmeladema deleted the fix-closure-path-printing branch September 26, 2020 08:15
@arora-aman arora-aman mentioned this pull request Nov 6, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyb eddyb eddyb approved these changes

Assignees

@eddyb eddyb

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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.
6 participants
@marmeladema @bors @eddyb @rust-highfive @jyn514 @rustbot