Skip to content

Remove some dead or leftover code related to rustc-intrinsic abi removal #139530

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

Merged
merged 1 commit into from
Apr 10, 2025

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Apr 8, 2025

r? @RalfJung

PR that removed the ABI: #139455

tracking issue: #132735

@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 Apr 8, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 8, 2025

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

@rust-log-analyzer

This comment has been minimized.

@@ -397,7 +397,6 @@ E0618: 0618,
E0619: 0619,
E0620: 0620,
E0621: 0621,
E0622: 0622,
Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove it but instead add a comment after with // REMOVED: explanation

Copy link
Member

@RalfJung RalfJung Apr 8, 2025

Choose a reason for hiding this comment

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

Dealing with no-longer emitted error codes is a pain. Some time ago, I added some comments at the top of this file to explain what one has to do (which I had to figure out by trial and error as nothing seemed documented). Not sure how to make this more visible.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed but --explain is still supposed to work. ^^'

Copy link
Contributor

@Skgland Skgland Apr 10, 2025

Choose a reason for hiding this comment

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

Wouldn't it make sense to keep this error for misused #[rustc_intrinsic]?
Currently that annotation appears to be silently ignored (or ICE) when misapplied playground

For this reason I hadn't removed the check motioned in #139530 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

This is an internal attribute, having it ICE is entirely fine.

Having it silently ignored is a bit unfortunate; not sure if there's an ways way to avoid that -- probably only after the attribute refactor (Cc @jdonszelmann )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, rustc_* attributes are excluded from the "must have logic to check whether they are used in the correct place" rule. Some have impls regardless, but most do not.

@oli-obk oli-obk force-pushed the rustc-intrinsic-cleanup branch from 0574242 to f0684f9 Compare April 8, 2025 14:08
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Makes sense, r=me when CI is happy.

Comment on lines -790 to -798
if tcx.has_attr(def_id, sym::rustc_intrinsic) {
intrinsic::check_intrinsic_type(
tcx,
item.id.owner_id.def_id,
item.span,
item.ident.name,
abi,
);
}
Copy link
Member

Choose a reason for hiding this comment

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

So this was for extern blocks with functions marked as #[rustc_intrinsic]? That was never even a thing, was it?

So yeah I agree with removing this.^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was just something I overlooked in the transition PR where an ABI check was turned into an attr check. Not explicitly added for the attr

@rust-log-analyzer

This comment has been minimized.

@oli-obk oli-obk force-pushed the rustc-intrinsic-cleanup branch from f0684f9 to 5ca462d Compare April 8, 2025 16:06
@rust-log-analyzer

This comment has been minimized.

@oli-obk oli-obk force-pushed the rustc-intrinsic-cleanup branch from 5ca462d to deed461 Compare April 9, 2025 06:46
@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 9, 2025

@bors r=RalfJung rollup

@bors
Copy link
Collaborator

bors commented Apr 9, 2025

📌 Commit deed461 has been approved by RalfJung

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 Apr 9, 2025
@oli-obk oli-obk force-pushed the rustc-intrinsic-cleanup branch from deed461 to b2aa9d0 Compare April 9, 2025 07:57
@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 9, 2025

@bors r=RalfJung rollup

@bors
Copy link
Collaborator

bors commented Apr 9, 2025

📌 Commit b2aa9d0 has been approved by RalfJung

It is now in the queue for this repository.

Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 10, 2025
…=RalfJung

Remove some dead or leftover code related to rustc-intrinsic abi removal

r? `@RalfJung`

PR that removed the ABI: rust-lang#139455

tracking issue: rust-lang#132735
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2025
Rollup of 18 pull requests

Successful merges:

 - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped)
 - rust-lang#138167 (Small code improvement in rustdoc hidden stripper)
 - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated)
 - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern)
 - rust-lang#138904 (Test linking and running `no_std` binaries)
 - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer)
 - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call)
 - rust-lang#139507 (compiletest: Trim whitespace from environment variable names)
 - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal)
 - rust-lang#139560 (fix title of offset_of_enum feature)
 - rust-lang#139563 (emit a better error message for using the macro incorrectly)
 - rust-lang#139568 (Don't use empty trait names)
 - rust-lang#139580 (Temporarily leave the review rotation)
 - rust-lang#139589 (saethlin is back from vacation)
 - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests)
 - rust-lang#139599 (Tracking issue template: fine-grained information on style update status)
 - rust-lang#139600 (Update `compiler-builtins` to 0.1.153)
 - rust-lang#139606 (Update compiletest to Edition 2024)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 10, 2025
…=RalfJung

Remove some dead or leftover code related to rustc-intrinsic abi removal

r? ``@RalfJung``

PR that removed the ABI: rust-lang#139455

tracking issue: rust-lang#132735
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2025
Rollup of 17 pull requests

Successful merges:

 - rust-lang#137412 (Ensure `swap_nonoverlapping` is really always untyped)
 - rust-lang#138167 (Small code improvement in rustdoc hidden stripper)
 - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated)
 - rust-lang#138682 (Allow drivers to supply a list of extra symbols to intern)
 - rust-lang#138904 (Test linking and running `no_std` binaries)
 - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer)
 - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call)
 - rust-lang#139507 (compiletest: Trim whitespace from environment variable names)
 - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal)
 - rust-lang#139560 (fix title of offset_of_enum feature)
 - rust-lang#139563 (emit a better error message for using the macro incorrectly)
 - rust-lang#139568 (Don't use empty trait names)
 - rust-lang#139580 (Temporarily leave the review rotation)
 - rust-lang#139589 (saethlin is back from vacation)
 - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests)
 - rust-lang#139599 (Tracking issue template: fine-grained information on style update status)
 - rust-lang#139600 (Update `compiler-builtins` to 0.1.153)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2025
…iaskrgr

Rollup of 13 pull requests

Successful merges:

 - rust-lang#138167 (Small code improvement in rustdoc hidden stripper)
 - rust-lang#138605 (Clean up librustdoc::html::render to be better encapsulated)
 - rust-lang#139423 (Suppress missing field error when autoderef bottoms out in infer)
 - rust-lang#139449 (match ergonomics: replace `peel_off_references` with a recursive call)
 - rust-lang#139507 (compiletest: Trim whitespace from environment variable names)
 - rust-lang#139530 (Remove some dead or leftover code related to rustc-intrinsic abi removal)
 - rust-lang#139560 (fix title of offset_of_enum feature)
 - rust-lang#139563 (emit a better error message for using the macro incorrectly)
 - rust-lang#139568 (Don't use empty trait names)
 - rust-lang#139580 (Temporarily leave the review rotation)
 - rust-lang#139589 (saethlin is back from vacation)
 - rust-lang#139592 (rustdoc: Enable Markdown extensions when looking for doctests)
 - rust-lang#139599 (Tracking issue template: fine-grained information on style update status)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c808b20 into rust-lang:master Apr 10, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 10, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2025
Rollup merge of rust-lang#139530 - oli-obk:rustc-intrinsic-cleanup, r=RalfJung

Remove some dead or leftover code related to rustc-intrinsic abi removal

r? ```@RalfJung```

PR that removed the ABI: rust-lang#139455

tracking issue: rust-lang#132735
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

7 participants