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

tests: use minicore more #137599

Merged
merged 1 commit into from
Mar 1, 2025
Merged

tests: use minicore more #137599

merged 1 commit into from
Mar 1, 2025

Conversation

davidtwco
Copy link
Member

minicore makes it much easier to add new language items to all of the existing no_core tests.

Most of the remaining tests that could use minicore either fail because..

  1. LLVM IR output changes and doesn't pass the test as written. I didn't look into these further.
  2. The test has revisions w/ different compilation flags, expecting some to fail, and when using minicore, minicore is compiled with those flags and fails in the expected way because of the flags rather than the test, and that's considered a failure.

But these tests can be changed and make adding new language items a lot easier.

r? @jieyouxu

minicore makes it much easier to add new language items to all of the
existing `no_core` tests.
@rustbot rustbot added 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. labels Feb 25, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 25, 2025

Some changes occurred in tests/codegen/stack-probes-inline.rs

cc @rust-lang/project-exploit-mitigations, @rcvalle

Some changes occurred in tests/codegen/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

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.

Thanks for working on this! Most of the tests look good to me, however I noticed a few tests that has minor problems, should not / cannot use minicore, and a few unexpected changes. With the changes in this PR, I think I can see a better guideline for when to use minicore. In general, I think minicore...

  • Should be used when one wants a no_core ui/assembly/codegen test for ABI/codegen/layout purposes, and maybe some other use cases
  • Should not be used if the test is exercising diagnostics or lang_item-handling itself, or if using minicore can complicate debugging (such as when a resolve test fails)

We should add that to rustc-dev-guide. Maybe you have suggestions for refinements?

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 25, 2025
@davidtwco
Copy link
Member Author

  • Should be used when one wants a no_core ui/assembly/codegen test for ABI/codegen/layout purposes, and maybe some other use cases
  • Should not be used if the test is exercising diagnostics or lang_item-handling itself, or if using minicore can complicate debugging (such as when a resolve test fails)

This makes sense to me, but I think diagnostics changing by itself doesn't mean that minicore shouldn't be used, it depends on the specific change - some tests will have a diagnostic change for irrelevant reasons (as in tests/ui/associated-types/associated-types-ICE-when-projecting-out-of-err.stderr in this PR) and that's okay.

@davidtwco davidtwco 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 Feb 25, 2025
@jieyouxu
Copy link
Member

Yeah, the classic "It Depends" situation 😆

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.

Thanks!

@jieyouxu
Copy link
Member

@bors r+ rollup=maybe

@bors
Copy link
Contributor

bors commented Feb 25, 2025

📌 Commit 92eb445 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 Feb 25, 2025
davidtwco added a commit to davidtwco/rust that referenced this pull request Feb 26, 2025
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 28, 2025
…youxu

tests: use minicore more

minicore makes it much easier to add new language items to all of the existing `no_core` tests.

Most of the remaining tests that *could* use minicore either fail because..

1. LLVM IR output changes and doesn't pass the test as written. I didn't look into these further.
2. The test has revisions w/ different compilation flags, expecting some to fail, and when using minicore, minicore is compiled with those flags and fails in the expected way because of the flags rather than the test, and that's considered a failure.

But these tests can be changed and make adding new language items a lot easier.

r? `@jieyouxu`
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 28, 2025
…youxu

tests: use minicore more

minicore makes it much easier to add new language items to all of the existing `no_core` tests.

Most of the remaining tests that *could* use minicore either fail because..

1. LLVM IR output changes and doesn't pass the test as written. I didn't look into these further.
2. The test has revisions w/ different compilation flags, expecting some to fail, and when using minicore, minicore is compiled with those flags and fails in the expected way because of the flags rather than the test, and that's considered a failure.

But these tests can be changed and make adding new language items a lot easier.

r? ``@jieyouxu``
davidtwco added a commit to davidtwco/rust that referenced this pull request Feb 28, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#134943 (Add FileCheck annotations to mir-opt/issues)
 - rust-lang#137017 (Don't error when adding a staticlib with bitcode files compiled by newer LLVM)
 - rust-lang#137197 (Update some comparison codegen tests now that they pass in LLVM20)
 - rust-lang#137540 (Fix (more) test directives that were accidentally ignored)
 - rust-lang#137551 (import `simd_` intrinsics)
 - rust-lang#137599 (tests: use minicore more)
 - rust-lang#137673 (Fix Windows `Command` search path bug)
 - rust-lang#137676 (linker: Fix escaping style for response files on Windows)
 - rust-lang#137693 (Re-enable `--generate-link-to-defintion` for tools internal rustdoc)
 - rust-lang#137770 (Fix sized constraint for unsafe binder)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 50ed7f9 into rust-lang:master Mar 1, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 1, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2025
Rollup merge of rust-lang#137599 - davidtwco:use-minicore-more, r=jieyouxu

tests: use minicore more

minicore makes it much easier to add new language items to all of the existing `no_core` tests.

Most of the remaining tests that *could* use minicore either fail because..

1. LLVM IR output changes and doesn't pass the test as written. I didn't look into these further.
2. The test has revisions w/ different compilation flags, expecting some to fail, and when using minicore, minicore is compiled with those flags and fails in the expected way because of the flags rather than the test, and that's considered a failure.

But these tests can be changed and make adding new language items a lot easier.

r? ```@jieyouxu```
@davidtwco davidtwco deleted the use-minicore-more branch March 3, 2025 12:12
@davidtwco davidtwco mentioned this pull request Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants