Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

ast: Standardize visiting order for attributes and node IDs #125741

Merged
merged 2 commits into from
Jun 25, 2024
Merged

ast: Standardize visiting order for attributes and node IDs #125741

merged 2 commits into from
Jun 25, 2024

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented May 29, 2024 •
edited
Loading

This should only affect macro_rules scopes and order of diagnostics.

Also add a deprecation lint for macro_rules called outside of their scope, like in #124535.

@rustbot
Copy link
Collaborator

rustbot commented May 29, 2024

r? @davidtwco

rustbot has assigned @davidtwco.
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 May 29, 2024
@rustbot
Copy link
Collaborator

rustbot commented May 29, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 29, 2024
@petrochenkov petrochenkov mentioned this pull request May 29, 2024
Closed
@bors

This comment was marked as resolved.

# to view
@petrochenkov
Copy link
Contributor Author

@bors try

@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels May 30, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request May 30, 2024
@bors
Auto merge of rust-lang#125741 - petrochenkov:atvisord, r=<try>
0525276 
ast: Standardize visiting order for attributes and node IDs

This should only affect `macro_rules` scopes and order of diagnostics.

This is blocked on rust-lang#125734 and will also need a crater run.
@bors
Copy link
Contributor

bors commented May 30, 2024

⌛ Trying commit baccb16 with merge 0525276...

@bors
Copy link
Contributor

bors commented May 30, 2024

☀️ Try build successful - checks-actions
Build commit: 0525276 (0525276e4a85ae0a91f6f7447e501e2981e77908)

@petrochenkov
Copy link
Contributor Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-125741 created and queued.
🤖 Automatically detected try build 0525276
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🚧 Experiment pr-125741 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-125741 is completed!
📊 573 regressed and 2 fixed (467353 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jun 5, 2024
@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 Jun 5, 2024
@petrochenkov
Copy link
Contributor Author

The number of root regressions is not large (10 crates), I think we can fix this.

  • 491 cannot find macro self_test in this scope - in document-features (crate root)
  • 196 cannot find macro cargo_toml in this scope - in influxdb (crate root)
  • 136 cannot find macro blob_url_prefix in this scope - in chumsky (crate root)
  • 84 cannot find macro font_bmp in this scope - in bitmap-font (module)
  • 32 cannot find macro docs_self in this scope - in pacaptr (module)
  • 9 cannot find macro file_url in this scope - in html5tokenizer (crate root)
  • 4 cannot find macro doc_WallpaperBuilder_example in this scope - in more-wallpapers (crate root)
  • 4 cannot find macro alias_file in this scope - in attr_alias (crate root)
  • 3 cannot find macro stringified_module_code in this scope - safer-ffi (module)
  • 2 cannot find macro doctest_example in this scope - test_gen (crate root)

@rust-log-analyzer

This comment has been minimized.

# to view
@rust-log-analyzer

This comment has been minimized.

# to view
davidtwco
davidtwco approved these changes Jun 24, 2024
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

r=me once you've rebased

@petrochenkov
Copy link
Contributor Author

@bors r=davidtwco

@bors
Copy link
Contributor

bors commented Jun 24, 2024

📌 Commit c4c7859 has been approved by davidtwco

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 Jun 24, 2024
@bors
Copy link
Contributor

bors commented Jun 25, 2024

⌛ Testing commit c4c7859 with merge d929a42...

@bors
Copy link
Contributor

bors commented Jun 25, 2024

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing d929a42 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 25, 2024
@bors bors merged commit d929a42 into rust-lang:master Jun 25, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 25, 2024
This was referenced Jun 25, 2024
Merged
Merged
Open
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d929a42): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 0.3% [0.2%, 0.3%] 3
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.3% [-0.4%, -0.2%] 12
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary -0.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 4.6% [3.5%, 5.7%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -5.0% [-8.0%, -2.0%] 2
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 693.71s -> 692.703s (-0.15%)
Artifact size: 326.75 MiB -> 326.77 MiB (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jun 25, 2024
@tgross35 tgross35 mentioned this pull request Jun 26, 2024
Closed
@petrochenkov petrochenkov mentioned this pull request Jun 26, 2024
Merged
@nnethercote
Copy link
Contributor

What was the motivation for this change?

@petrochenkov
Copy link
Contributor Author

Fixing scoping for macro_rules and reporting diagnostics in source code order.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 27, 2024
@bors
Auto merge of rust-lang#126993 - petrochenkov:atvisord3, r=BoxyUwU
036b38c 
ast: Standardize visiting order

Order: ID, attributes, inner nodes in source order if possible, tokens, span.

Also always use exhaustive matching in visiting infra, and visit some discovered missing nodes.

Unlike rust-lang#125741 this shouldn't affect anything serious like `macro_rules` scopes.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 27, 2024
@bors
Auto merge of rust-lang#125741 - petrochenkov:atvisord, r=davidtwco
dfaa53f 
ast: Standardize visiting order for attributes and node IDs

This should only affect `macro_rules` scopes and order of diagnostics.

Also add a deprecation lint for `macro_rules` called outside of their scope, like in rust-lang#124535.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jun 28, 2024
@bors
Auto merge of #126993 - petrochenkov:atvisord3, r=BoxyUwU
60ccf03 
ast: Standardize visiting order

Order: ID, attributes, inner nodes in source order if possible, tokens, span.

Also always use exhaustive matching in visiting infra, and visit some discovered missing nodes.

Unlike rust-lang/rust#125741 this shouldn't affect anything serious like `macro_rules` scopes.
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Jun 28, 2024
@bors
Auto merge of #126993 - petrochenkov:atvisord3, r=BoxyUwU
89e87d7 
ast: Standardize visiting order

Order: ID, attributes, inner nodes in source order if possible, tokens, span.

Also always use exhaustive matching in visiting infra, and visit some discovered missing nodes.

Unlike rust-lang/rust#125741 this shouldn't affect anything serious like `macro_rules` scopes.
@pnkfelix
Copy link
Member

pnkfelix commented Jul 2, 2024

visiting for weekly rustc-perf triage

  • solely regressions to secondary benchmark: tt-muncher.
  • marking as triaged

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jul 2, 2024
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Jul 11, 2024
@bors
Auto merge of #126993 - petrochenkov:atvisord3, r=BoxyUwU
af0694f 
ast: Standardize visiting order

Order: ID, attributes, inner nodes in source order if possible, tokens, span.

Also always use exhaustive matching in visiting infra, and visit some discovered missing nodes.

Unlike rust-lang/rust#125741 this shouldn't affect anything serious like `macro_rules` scopes.
@petrochenkov petrochenkov deleted the atvisord branch February 22, 2025 18:28
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

Labels
merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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.81.0
Development

Successfully merging this pull request may close these issues.
9 participants
@petrochenkov @rustbot @bors @craterbot @rust-log-analyzer @rust-timer @nnethercote @pnkfelix @davidtwco