Skip to content

Conversation

@paradoxicalguy
Copy link

@paradoxicalguy paradoxicalguy commented Dec 25, 2025

this adds the Ordering enum to minicore.rs.

consequently, this updates tests/assembly-llvm/rust-abi-arg-attr.rs to import minicore directly. previously, this test file contained traits like Copy Clone PointeeSized, which were giving a duplicate lang item error, so replace those by importing minicore completely.

@rustbot
Copy link
Collaborator

rustbot commented Dec 25, 2025

This PR modifies tests/auxiliary/minicore.rs.

cc @jieyouxu

@rustbot rustbot added A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` 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 25, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 25, 2025

Mark-Simulacrum is not on the review rotation at the moment.
They may take a while to respond.

@rustbot
Copy link
Collaborator

rustbot commented Dec 25, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

This doesn't actually remove any diverging definitions from tests?

View changes since this review


#[lang = "c_void"]
#[repr(u8)]
#[repr(C)]
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 not how c_void is defined in core::ffi. Please revert this unrelated change.

Copy link
Author

Choose a reason for hiding this comment

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

sure!

#[lang = "Ordering"]
#[repr(i8)]
pub enum Ordering {
Less = 0xFFu8 as i8,
Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry?

You can just write -1, can't you?

Copy link
Author

Choose a reason for hiding this comment

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

it was giving an error before, reverted it back

Comment on lines 308 to 310
#[rustc_nounwind]
#[rustc_intrinsic]
pub const fn three_way_compare<T: Copy>(lhs: T, rhs: T) -> Ordering;
Copy link
Member

Choose a reason for hiding this comment

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

Why are you also adding this?

Copy link
Author

Choose a reason for hiding this comment

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

tests/assembly-llvm/rust-abi-arg-attr.rs - uses Ordering and three_way_compare

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 26, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 26, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 26, 2025
@paradoxicalguy
Copy link
Author

fixed minicore issues, aligned ordering and usize behavior, ready for re-review
@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 26, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. label Dec 26, 2025
@rust-log-analyzer

This comment has been minimized.

@paradoxicalguy
Copy link
Author

@rustbot ready

@paradoxicalguy
Copy link
Author

@workingjubilee
i used 0xFFu8 as i8 instead of -1 because in #![no_core] context, the unary - operator requires the Neg lang item and an impl Neg for i8, which aren't currently in minicore.

adding impl Neg for i8 just to support this discriminant would introduce operator semantics (arithmetic, overflow handling, MIR lowering) which minicore intentionally avoids. the 0xFFu8 as i8 approach uses pure constant evaluation with no operator traits or lang items required

if you'd prefer -1 for readability, i can add the Neg implementation

@workingjubilee
Copy link
Member

don't quote the deep magic to me. I was there when it was written.

minicore is minimal for the sake of not reimplementing all of core, nothing more.

@workingjubilee workingjubilee self-assigned this Dec 26, 2025
@paradoxicalguy
Copy link
Author

#150423
new pr generated

@paradoxicalguy
Copy link
Author

@workingjubilee i have added the Ordering enum in minicore.rs file
just pinging you for a design choice as i am hitting a duplicating lang error when i import minicore in rust-abi-arg-attr.rs file for other traits like Copy, Clone etc,
is there a specific way you prefer we handle these trait collisions?

i found 2 ways to fix it which passes locally:

  • option a: removing the duplicating traits from rust-abi-arg-attr.rs test file and just importing minicore
  • option b: keeping both files separate and independent
    or any different way which you think might be the best.

@rustbot
Copy link
Collaborator

rustbot commented Dec 31, 2025

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.

@rust-log-analyzer

This comment has been minimized.

@paradoxicalguy
Copy link
Author

@workingjubilee i have closed the duplicate PR and pushed the minicore changes here,

CI failure is the duplicate lang item issue i asked in my previous comment, when importing minicore, it conflicts with the local trait definitions in the test file.
just need your preference on which direction to take.

@paradoxicalguy
Copy link
Author

@workingjubilee just following up, waiting for whichever solution you prefer for the duplicate lang item error. i'll update the PR

@workingjubilee
Copy link
Member

@paradoxicalguy It seems that this test was the only one that actually used lang = "Ordering". Correct the PR description, please.

It's arguably fine to make the change since it makes it easier to write certain tests and verify they cross-compile. But "some" is awfully vague when it's just 1.

@paradoxicalguy paradoxicalguy changed the title tests: add Ordering to minicore for no_core codegen tests adding Ordering enum to minicore.rs, importing minicore in test file Jan 9, 2026
@paradoxicalguy paradoxicalguy changed the title adding Ordering enum to minicore.rs, importing minicore in test file adding Ordering enum to minicore.rs, importing minicore in "tests/assembly-llvm/rust-abi-arg-attr.rs" test file Jan 9, 2026
@workingjubilee
Copy link
Member

@bors r+

@rust-bors rust-bors bot 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 Jan 9, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 9, 2026

📌 Commit 484ea76 has been approved by workingjubilee

It is now in the queue for this repository.

@paradoxicalguy
Copy link
Author

yay :-D

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 9, 2026
adding Ordering enum to minicore.rs, importing minicore in "tests/assembly-llvm/rust-abi-arg-attr.rs" test file

this adds the `Ordering` enum to `minicore.rs`.

consequently, this updates `tests/assembly-llvm/rust-abi-arg-attr.rs` to import `minicore` directly. previously, this test file contained traits like `Copy` `Clone` `PointeeSized`, which were giving a duplicate lang item error, so replace those by importing `minicore` completely.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 9, 2026
adding Ordering enum to minicore.rs, importing minicore in "tests/assembly-llvm/rust-abi-arg-attr.rs" test file

this adds the `Ordering` enum to `minicore.rs`.

consequently, this updates `tests/assembly-llvm/rust-abi-arg-attr.rs` to import `minicore` directly. previously, this test file contained traits like `Copy` `Clone` `PointeeSized`, which were giving a duplicate lang item error, so replace those by importing `minicore` completely.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 9, 2026
adding Ordering enum to minicore.rs, importing minicore in "tests/assembly-llvm/rust-abi-arg-attr.rs" test file

this adds the `Ordering` enum to `minicore.rs`.

consequently, this updates `tests/assembly-llvm/rust-abi-arg-attr.rs` to import `minicore` directly. previously, this test file contained traits like `Copy` `Clone` `PointeeSized`, which were giving a duplicate lang item error, so replace those by importing `minicore` completely.
tgross35 added a commit to tgross35/rust that referenced this pull request Jan 9, 2026
adding Ordering enum to minicore.rs, importing minicore in "tests/assembly-llvm/rust-abi-arg-attr.rs" test file

this adds the `Ordering` enum to `minicore.rs`.

consequently, this updates `tests/assembly-llvm/rust-abi-arg-attr.rs` to import `minicore` directly. previously, this test file contained traits like `Copy` `Clone` `PointeeSized`, which were giving a duplicate lang item error, so replace those by importing `minicore` completely.
Zalathar added a commit to Zalathar/rust that referenced this pull request Jan 10, 2026
adding Ordering enum to minicore.rs, importing minicore in "tests/assembly-llvm/rust-abi-arg-attr.rs" test file

this adds the `Ordering` enum to `minicore.rs`.

consequently, this updates `tests/assembly-llvm/rust-abi-arg-attr.rs` to import `minicore` directly. previously, this test file contained traits like `Copy` `Clone` `PointeeSized`, which were giving a duplicate lang item error, so replace those by importing `minicore` completely.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jan 10, 2026
adding Ordering enum to minicore.rs, importing minicore in "tests/assembly-llvm/rust-abi-arg-attr.rs" test file

this adds the `Ordering` enum to `minicore.rs`.

consequently, this updates `tests/assembly-llvm/rust-abi-arg-attr.rs` to import `minicore` directly. previously, this test file contained traits like `Copy` `Clone` `PointeeSized`, which were giving a duplicate lang item error, so replace those by importing `minicore` completely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` 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-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants