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

tests/assembly/asm: Remove uses of rustc_attrs and lang_items features by using minicore #134436

Merged
merged 1 commit into from
Dec 18, 2024
Merged

tests/assembly/asm: Remove uses of rustc_attrs and lang_items features by using minicore #134436

merged 1 commit into from
Dec 18, 2024

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Dec 17, 2024

Similar to #134385 (for tests/ui/asm), but for tests/assembly/asm.

r? jieyouxu

@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 Dec 17, 2024
jieyouxu
jieyouxu approved these changes Dec 17, 2024
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Nice cleanup, thanks! r=me once PR CI is green.

@jieyouxu
Copy link
Member

@bors delegate+ rollup

@bors
Copy link
Contributor

bors commented Dec 17, 2024

✌️ @taiki-e, you can now approve this pull request!

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

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jieyouxu
Copy link
Member

I'll take a look at the failures tmrw unless u know how to fix them already

@taiki-e taiki-e force-pushed the assembly-asm-minicore branch from e20646c to bd7213f Compare December 17, 2024 18:02
@taiki-e
Copy link
Member Author

taiki-e commented Dec 17, 2024 •
edited
Loading

It seems that failures are due to --crate-type cdylib passed in these tests:

rust/tests/assembly/asm/nvptx-types.rs

Line 4 in e20646c

//@ compile-flags: --crate-type cdylib

rust/tests/assembly/asm/wasm-types.rs

Line 4 in e20646c

//@ compile-flags: --crate-type cdylib

Removed that flag and changed to use #![crate_type = "rlib"] as with other assembly/asm/*-types.rs tests. (The change does not seem to affect the part of the generated assembly that we check in assembly tests.)

rust/tests/assembly/asm/aarch64-types.rs

Line 11 in e20646c

#![crate_type = "rlib"]

@jieyouxu
Copy link
Member

I'm curious why cdylib was used previously, I'll take a closer look tmrw.

@jieyouxu
Copy link
Member

jieyouxu commented Dec 18, 2024 •
edited
Loading

My git archeology:

  • I do not see why the nvptx-types test need cdylib specifically. I checked the original PR NVPTX support for new asm! #72439 and there was no elaboration. To me, this indicates we can change the crate type. If there's some kind of cdylib-specific behavior that this test wanted to check, it's not doing that because the test doesn't seem to be failing here on PR CI.
  • I do not see why the wasm-types test need cdylib specifically. I checked Add wasm32 support to inline asm #78684, no mention of the cdylib crate-type being specifically required. If it was, this test isn't checking that either.

So, if these two test changes turn out to be undesirable, then we better be documenting the reason for cdylib being load-bearing specifically anyway. Based on the current form of the test themselves, I'm not expecting this to be problematic, but maybe target maintainers know of problems otherwise.

Thanks for the cleanup, this looks good to go for me.
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 18, 2024

📌 Commit bd7213f has been approved by jieyouxu

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 18, 2024
@jieyouxu jieyouxu added the A-testsuite Area: The testsuite used to check the correctness of rustc label Dec 18, 2024
@jieyouxu jieyouxu mentioned this pull request Dec 18, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2024
@bors
Auto merge of rust-lang#134470 - jieyouxu:rollup-kld7kmk, r=jieyouxu
a52085d 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#130786 ( mir-opt: a sub-BB of a cleanup BB must also be a cleanup BB in `EarlyOtherwiseBranch`)
 - rust-lang#133926 (Fix const conditions for RPITITs)
 - rust-lang#134161 (Overhaul token cursors)
 - rust-lang#134253 (Overhaul keyword handling)
 - rust-lang#134394 (Clarify the match ergonomics 2024 migration lint's output)
 - rust-lang#134399 (Do not do if ! else, use unnegated cond and swap the branches instead)
 - rust-lang#134420 (refactor: replace &PathBuf with &Path to enhance generality)
 - rust-lang#134436 (tests/assembly/asm: Remove uses of rustc_attrs and lang_items features by using minicore)
 - rust-lang#134444 (Fix `x build --stage 1 std` when using cg_cranelift as the default backend)
 - rust-lang#134452 (fix(LazyCell): documentation of get[_mut] was wrong)
 - rust-lang#134460 (Merge some patterns together)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit af96692 into rust-lang:master Dec 18, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 18, 2024
@rust-timer
Unrolled build for rust-lang#134436
be1dffa 
Rollup merge of rust-lang#134436 - taiki-e:assembly-asm-minicore, r=jieyouxu

tests/assembly/asm: Remove uses of rustc_attrs and lang_items features by using minicore

Similar to rust-lang#134385 (for tests/ui/asm), but for tests/assembly/asm.

r? jieyouxu
@taiki-e taiki-e deleted the assembly-asm-minicore branch December 18, 2024 19:19
@taiki-e taiki-e mentioned this pull request Dec 20, 2024
Merged
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Dec 20, 2024
@jieyouxu
Rollup merge of rust-lang#134562 - taiki-e:codegen-asm-minicore, r=ji…
fb85ff8 
…eyouxu

tests/codegen/asm: Remove uses of rustc_attrs and lang_items features by using minicore

Similar to rust-lang#134385 (for tests/ui/asm) and rust-lang#134436 (for tests/assembly/asm), but for tests/codegen/asm.

r? jieyouxu
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 20, 2024
@matthiaskrgr
Rollup merge of rust-lang#134562 - taiki-e:codegen-asm-minicore, r=ji…
17f6690 
…eyouxu

tests/codegen/asm: Remove uses of rustc_attrs and lang_items features by using minicore

Similar to rust-lang#134385 (for tests/ui/asm) and rust-lang#134436 (for tests/assembly/asm), but for tests/codegen/asm.

r? jieyouxu
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 20, 2024
@rust-timer
Unrolled build for rust-lang#134562
765d4fc 
Rollup merge of rust-lang#134562 - taiki-e:codegen-asm-minicore, r=jieyouxu

tests/codegen/asm: Remove uses of rustc_attrs and lang_items features by using minicore

Similar to rust-lang#134385 (for tests/ui/asm) and rust-lang#134436 (for tests/assembly/asm), but for tests/codegen/asm.

r? jieyouxu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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.85.0
Development

Successfully merging this pull request may close these issues.
5 participants
@taiki-e @jieyouxu @bors @rust-log-analyzer @rustbot