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.

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

Hir attributes #131808

Merged
merged 3 commits into from
Dec 16, 2024
Merged

Hir attributes #131808

merged 3 commits into from
Dec 16, 2024

Conversation

jdonszelmann
Copy link
Contributor

@jdonszelmann jdonszelmann commented Oct 17, 2024

This PR needs some explanation, it's somewhat large.

  • This is step one as described in Attribute handling reworks compiler-team#796. I've added a new hir::Attribute which is a lowered version of ast::Attribute. Right now, this has few concrete effects, however every place that after this PR parses a hir::Attribute should later get a pre-parsed attribute as described in Attribute handling reworks compiler-team#796 and transitively Tracking issue: Attribute refactor #131229.
  • an extension trait AttributeExt is added, which is implemented for both ast::Attribute and hir::Atribute. This makes hir::Attributes mostly compatible with code that used to parse ast::Attribute. All its methods are also added as inherent methods to avoid having to import the trait everywhere in the compiler.
    • Incremental can not not hash ast::Attribute at all.

@rustbot

This comment was marked as resolved.

@rustbot

This comment was marked as outdated.

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-rustdoc-json Area: Rustdoc JSON backend PG-exploit-mitigations Project group: Exploit mitigations 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Oct 17, 2024
@rust-log-analyzer

This comment has been minimized.

compiler/rustc_ast/src/ast.rs Outdated Show resolved Hide resolved
compiler/rustc_ast/src/ast.rs Outdated Show resolved Hide resolved
compiler/rustc_ast/src/ast.rs Show resolved Hide resolved
compiler/rustc_hir/src/hir.rs Outdated Show resolved Hide resolved
@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-release Relevant to the release subteam, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Oct 17, 2024
@jdonszelmann jdonszelmann force-pushed the hir-attributes branch 4 times, most recently from cf3179b to e3133bb Compare October 17, 2024 06:52
@petrochenkov petrochenkov self-assigned this Oct 17, 2024
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@xFrednet xFrednet left a comment

Choose a reason for hiding this comment

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

The Clippy changes look good to me :D

=^.^=

@jdonszelmann jdonszelmann marked this pull request as ready for review October 17, 2024 18:44
@rustbot

This comment was marked as resolved.

@jdonszelmann jdonszelmann force-pushed the hir-attributes branch 2 times, most recently from 1920abd to cefa0ea Compare October 17, 2024 22:02
@cjgillot cjgillot self-assigned this Oct 17, 2024
@nnethercote

This comment was marked as resolved.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 15, 2024

@bors delegate+

r=me,petrochenkov once CI is happy

@bors
Copy link
Contributor

bors commented Dec 15, 2024

✌️ @jdonszelmann, you can now approve this pull request!

If @oli-obk told you to "r=me" after making some further change, please make that change, then do @bors r=@oli-obk

@jieyouxu
Copy link
Member

@bors p=10 (very very conflict-prone)
@bors rollup=never (significant attr handling changes)

@jieyouxu jieyouxu removed T-release Relevant to the release subteam, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 15, 2024
@jdonszelmann
Copy link
Contributor Author

@bors r=oli-obk,petrochenkov

@bors
Copy link
Contributor

bors commented Dec 15, 2024

📌 Commit 1d5ec2c has been approved by oli-obk,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 Dec 15, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2024
…bk,petrochenkov

Hir attributes

This PR needs some explanation, it's somewhat large.

- This is step one as described in rust-lang/compiler-team#796. I've added a new `hir::Attribute` which is a lowered version of `ast::Attribute`. Right now, this has few concrete effects, however every place that after this PR parses a `hir::Attribute` should later get a pre-parsed attribute as described in rust-lang/compiler-team#796 and transitively rust-lang#131229.
- an extension trait `AttributeExt` is added, which is implemented for both `ast::Attribute` and `hir::Atribute`. This makes `hir::Attributes` mostly compatible with code that used to parse `ast::Attribute`. All its methods are also added as inherent methods to avoid having to import the trait everywhere in the compiler.
  - Incremental can not not hash `ast::Attribute` at all.
@bors
Copy link
Contributor

bors commented Dec 15, 2024

⌛ Testing commit 1d5ec2c with merge 6add46f4e832c1c8ef2d88fdce74739f0a5041c4...

@bors
Copy link
Contributor

bors commented Dec 15, 2024

⌛ Testing commit 1d5ec2c with merge f2b91cc...

@bors
Copy link
Contributor

bors commented Dec 16, 2024

☀️ Test successful - checks-actions
Approved by: oli-obk,petrochenkov
Pushing f2b91cc to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 16, 2024
@bors bors merged commit f2b91cc into rust-lang:master Dec 16, 2024
7 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 16, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f2b91cc): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

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

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
0.4% [0.2%, 1.1%] 9
Regressions ❌
(secondary)
0.4% [0.1%, 0.6%] 20
Improvements ✅
(primary)
-0.2% [-0.4%, -0.2%] 3
Improvements ✅
(secondary)
-0.3% [-0.7%, -0.2%] 5
All ❌✅ (primary) 0.3% [-0.4%, 1.1%] 12

Max RSS (memory usage)

Results (primary -0.9%, secondary -0.9%)

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)
1.1% [0.7%, 1.6%] 2
Regressions ❌
(secondary)
2.6% [1.8%, 3.3%] 2
Improvements ✅
(primary)
-1.1% [-2.4%, -0.5%] 20
Improvements ✅
(secondary)
-2.1% [-3.0%, -1.6%] 6
All ❌✅ (primary) -0.9% [-2.4%, 1.6%] 22

Cycles

Results (primary 1.4%, secondary 1.9%)

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)
1.4% [0.6%, 2.8%] 23
Regressions ❌
(secondary)
1.9% [1.1%, 2.3%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.4% [0.6%, 2.8%] 23

Binary size

Results (primary -0.2%, secondary -0.4%)

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)
- - 0
Improvements ✅
(primary)
-0.2% [-1.0%, -0.0%] 75
Improvements ✅
(secondary)
-0.4% [-1.0%, -0.0%] 21
All ❌✅ (primary) -0.2% [-1.0%, -0.0%] 75

Bootstrap: 770.348s -> 773.825s (0.45%)
Artifact size: 333.25 MiB -> 332.94 MiB (-0.09%)

@rustbot rustbot added the perf-regression Performance regression. label Dec 16, 2024
@jieyouxu
Copy link
Member

(Perf regression is real as we are doing more work, not sure how to claw perf back personally.)

@oli-obk
Copy link
Contributor

oli-obk commented Dec 16, 2024

Interestingly it's only a rustdoc regression. Maybe run cachegrind on one of them and see if it's sth obvious?

@jdonszelmann
Copy link
Contributor Author

Zzzzz good morning, oh that's interesting, didn't expect that. I'll take a look today

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 16, 2024
Try to fix perf regression in rustdoc after hir attributes

Slight performance regression introduced in rust-lang#131808

r? `@jieyouxu`
@jdonszelmann jdonszelmann deleted the hir-attributes branch December 16, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-rustdoc-json Area: Rustdoc JSON backend merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. PG-exploit-mitigations Project group: Exploit mitigations 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.