Nest tests/codegen/sanitizer*.rs tests in sanitizer dir#114229
Merged
bors merged 2 commits intorust-lang:masterfrom Aug 7, 2023
Merged
Nest tests/codegen/sanitizer*.rs tests in sanitizer dir#114229bors merged 2 commits intorust-lang:masterfrom
bors merged 2 commits intorust-lang:masterfrom
Conversation
In the basic case, simply do the string substitution. For one case with many instances, capture the Itanium- mangled filename and assert its reuse instead.
Collaborator
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
Member
|
@bors r+ rollup |
Collaborator
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 7, 2023
…nest, r=Mark-Simulacrum Nest other codegen test topics This PR is like rust-lang#114229 in that it mostly pushes codegen tests around, shoving them into their own directories, but because all of the changes are very simple cleanups I pulled them into a separate PR. The other PR might involve actually evaluating the correctness of the test after changes, but here it is mostly a matter of taste. The only "functional" change is deleting a few tests that... hinge on a version of LLVM that we don't support (as of rust-lang#114148 anyways). I considered a few different ways to group other topics but I feel the question of whether `tests/codegen/{vec,array,slice}` should exist is more subtle than these choices, as it might be better to group such related tests by other topics like bounds check elision, thus I avoided making it.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 7, 2023
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#98935 (Implement `Option::take_if`) - rust-lang#114093 (Add regression test for `echo 'mod unknown;' | rustc -`) - rust-lang#114229 (Nest tests/codegen/sanitizer*.rs tests in sanitizer dir) - rust-lang#114230 (Nest other codegen test topics) - rust-lang#114362 (string.rs: remove "Basic usage" text) - rust-lang#114365 (str.rs: remove "Basic usage" text) - rust-lang#114382 (Add a new `compare_bytes` intrinsic instead of calling `memcmp` directly) - rust-lang#114549 (Style fix and refactor on resolve diagnostics) r? `@ghost` `@rustbot` modify labels: rollup
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Aug 8, 2023
…ark-Simulacrum Nest other codegen test topics This PR is like rust-lang/rust#114229 in that it mostly pushes codegen tests around, shoving them into their own directories, but because all of the changes are very simple cleanups I pulled them into a separate PR. The other PR might involve actually evaluating the correctness of the test after changes, but here it is mostly a matter of taste. The only "functional" change is deleting a few tests that... hinge on a version of LLVM that we don't support (as of rust-lang/rust#114148 anyways). I considered a few different ways to group other topics but I feel the question of whether `tests/codegen/{vec,array,slice}` should exist is more subtle than these choices, as it might be better to group such related tests by other topics like bounds check elision, thus I avoided making it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The sanitizer tests are the largest and most meticulously tested set of tests in tests/codegen. That's good! They all clearly belong to a subject and thus could go in a directory, but are not, instead being placed simply in tests/codegen. That's bad! Fix this by placing them in their own directory and renaming them to be less repetitive after that move.
A few tests are brittle, and embed their filename in the test's checks. This is acceptable for the ones where it is used only two times, but one test embeds the test's mangled filename in the test over 50 times! This may have been one of the things discouraging anyone from moving it, and thus from moving the set. Fortunately, I have some knowledge of Itanium mangling (involuntarily), regex, and the FileCheck syntax. With a capturing variable, FileCheck allows us to now move this test around again without diffing it on ~50 lines, while still guaranteeing that the mangled substring is the same each time.
This also clarifies why the substring is repeated a zillion times, instead of being cryptic. They don't call it mangling because the result is pretty and easy to understand, but now it is slightly easier! Yay descriptive variables!