-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Move some checks from check_doc_attrs directly into rustc_attr_parsing
#150934
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
Conversation
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs Some changes occurred in compiler/rustc_attr_parsing |
| pub(crate) target_span: Span, | ||
| /// The id ([`NodeId`] if `S` is `Early`, [`HirId`] if `S` is `Late`) of the syntactical component this attribute was applied to | ||
| pub(crate) target_id: S::Id, | ||
| pub(crate) target: Option<rustc_hir::Target>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels a little weird to me that target can be None, but at the same time I see getting parse_single_args to take a Target is just really annoying...
I think this is acceptable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, not sure how to deal with that. I have some ideas, gonna give it a try later on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it more but I really don't see a nice way of fixing this, but if you do feel free to still take a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a hunch, I'll see how far it leads me.
533e109 to
077c9b2
Compare
|
Applied suggestion for macro naming. |
|
☔ The latest upstream changes (presumably #150957) made this pull request unmergeable. Please resolve the merge conflicts. |
|
r=me after rebasing |
077c9b2 to
3343f73
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
3343f73 to
ef1e4e6
Compare
|
@bors r=JonathanBrouwer orllup |
|
@bors rollup |
…ouwer Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing` Part of rust-lang#149865. I also used this opportunity to remove the `id_is_crate_root` method. Once this is merged, I think I'll try to see if we can remove `target_id` as well. r? @JonathanBrouwer
…uwer Rollup of 13 pull requests Successful merges: - #145343 (Dogfood `-Zno-embed-metadata` in the standard library) - #150151 (Destabilise `target-spec-json`) - #150723 (std: move `errno` and related functions into `sys::io`) - #150771 (Remove legacy homu `try` and `auto` branch mentions) - #150826 (Add `f16` inline ASM support for s390x) - #150934 (Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing`) - #150943 (Port `#[must_not_suspend]` to attribute parser) - #150990 (std: sys: net: uefi: Make TcpStream Send) - #150995 (core: ptr: split_at_mut: fix typo in safety doc) - #150998 (Relax test expectation for @__llvm_profile_runtime_user) - #151002 (Remove a workaround for a bug (take 2)) - #151005 (Fix typo in `MaybeUninit` docs) - #151011 (Update books) r? @ghost
…ouwer Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing` Part of rust-lang#149865. I also used this opportunity to remove the `id_is_crate_root` method. Once this is merged, I think I'll try to see if we can remove `target_id` as well. r? @JonathanBrouwer
Rollup of 12 pull requests Successful merges: - #145343 (Dogfood `-Zno-embed-metadata` in the standard library) - #150151 (Destabilise `target-spec-json`) - #150723 (std: move `errno` and related functions into `sys::io`) - #150826 (Add `f16` inline ASM support for s390x) - #150934 (Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing`) - #150943 (Port `#[must_not_suspend]` to attribute parser) - #150990 (std: sys: net: uefi: Make TcpStream Send) - #150995 (core: ptr: split_at_mut: fix typo in safety doc) - #150998 (Relax test expectation for @__llvm_profile_runtime_user) - #151002 (Remove a workaround for a bug (take 2)) - #151005 (Fix typo in `MaybeUninit` docs) - #151011 (Update books) r? @ghost
…uwer Rollup of 14 pull requests Successful merges: - #150151 (Destabilise `target-spec-json`) - #150826 (Add `f16` inline ASM support for s390x) - #150883 (Improve span for "unresolved intra doc link" on `deprecated` attribute) - #150934 (Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing`) - #150943 (Port `#[must_not_suspend]` to attribute parser) - #150990 (std: sys: net: uefi: Make TcpStream Send) - #150995 (core: ptr: split_at_mut: fix typo in safety doc) - #150998 (Relax test expectation for @__llvm_profile_runtime_user) - #151002 (Remove a workaround for a bug (take 2)) - #151005 (Fix typo in `MaybeUninit` docs) - #151011 (Update books) - #151029 (rustc-dev-guide subtree update) - #151032 (fix: added missing backtick in triagebot.toml) - #151035 (Don't suggest replacing closure parameter with type name) r? @ghost
Rollup merge of #150934 - move-doc-attr-checks, r=JonathanBrouwer Move some checks from `check_doc_attrs` directly into `rustc_attr_parsing` Part of #149865. I also used this opportunity to remove the `id_is_crate_root` method. Once this is merged, I think I'll try to see if we can remove `target_id` as well. r? @JonathanBrouwer
|
@rust-timer build 01517e8 |
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
|
@rust-timer build 01517e8 |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (01517e8): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 2.0%, secondary -3.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.9%, secondary 3.2%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 473.958s -> 474.412s (0.10%) |
Part of #149865.
I also used this opportunity to remove the
id_is_crate_rootmethod.Once this is merged, I think I'll try to see if we can remove
target_idas well.r? @JonathanBrouwer