Rollup of 9 pull requests #148404
Merged
Rollup of 9 pull requests #148404
Conversation
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
The tcx.parent tree appears to be correct now.
They need an extra branch for the newtype.
After bors merges a PR, a message may be posted from the CI onto the PR
comment thread directing the user to use the following commands to look
at the test differences:
```
cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard [SOMEID] --output-dir test-dashboard
open test-dashboard/index.html
```
This command creates the `.citool-cache` and `test-dashboard`
directories, whose contents should not enter the repository and can be
ignored by `git`.
"running analysis passes on this crate" -> "running analysis passes on crate `foo`" This message is displayed in cycle errors in particular, and in some cases without any spans or any other identifiable information to determine which dependency introduced the cycle.
This test suffers from multiple issues that make it very, very difficult to fix, and even if fixed, it would still be too fragile. For some background context, this test tries to check that the optimization introduced in [PR-78122] is not regressed. The optimization is for eliding `usize` formatting machinery and padding code from the final binary. Previously, writing any `fmt::Arguments` would cause the `usize` formatting and padding machinery to be included in the final binary since indexing used in `fmt::write` generates code using `panic_bounds_check` (that prints the index and length). Those bounds check are never hit, since `fmt::Arguments` never contain any out-of-bounds indicies. The `Makefile` version of `fmt-write-bloat` was ported to the present `rmake.rs` test infra in [PR-128147]. However, this PR just tries to maintain the original test logic. The original test, it turns out, already have multiple limitations: - It only runs on non-Windows, since the `no_std` test of the original version tries to link against a `libc`. [PR-128807] worked around this by using a substitute name. We re-enabled this test in [PR-142841], but it turns out the assertions are too weak, it will even vacuously pass for no symbols at all. - In [PR-143669], we tried to make this test more robust by comparing the set of expected versus unexpected panic-related symbols, subject to if std was built with debug assertions. However, in working on [PR-143669], we've come to realize that this test is fundamentally very fragile: - The set of panic symbols depend on whether the standard library was built with or without debug assertions. - Different platforms often have different sets of panic machinery modules, functions and paths, and thus different sets of panic symbols. For instance, x86_64 msvc and i686 msvc have different panic codepaths. - This comes back to the way the test is trying to gauge the absence of panic symbols -- it tries to look for symbol substring matches for "known" panic symbols. This is fundamentally fragile, because the test is trying to peek into the symbols of the resultant binary post-linking, based on fuzzy matches (the symbols are mangled as well). Based on this assessment, we determined that we should remove this test. This is not intended to exclude the possibility of reintroducing a more robust version of this test. For instance, we could consider some kind of more controllable post-link "end product" integration codegen test suite. [PR-78122]: rust-lang#78122 [PR-128147]: rust-lang#128147 [PR-128807]: rust-lang#128807 [PR-142841]: rust-lang#142841 [PR-143669]: rust-lang#143669
Mention crate being analyzed in query description "running analysis passes on this crate" -> "running analysis passes on crate `foo`" This message is displayed in cycle errors in particular, and in some cases without any spans or any other identifiable information to determine which dependency introduced the cycle. Address rust-lang#74380, but we still need a test for that case in particular.
arm-linux.md: various fixes/improvements
Miscellaneous const-generics-related fixes Fixes rust-lang#129209. Fixes rust-lang#131295. Fixes rust-lang#139738.
…t-dashboard, r=jieyouxu,kobzol
Ignore test-dashboard related files
After bors merge a PR, a message may be posted from the CI onto the PR comment thread directing the user to use the following commands to look at the test differences:
```
cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard [SOMEID] --output-dir test-dashboard
open test-dashboard/index.html
```
This command creates the `.citool-cache` and `test-dashboard` directories, whose contents should not enter the repository and can be ignored by `git`.
…r=joboet Implement `strip_circumfix` lib feature Tracking issue: rust-lang#147946
…thanBrouwer Change cfg_trace, cfg_attr_trace symbol values For debugging - I ran into this working on `cfg` stuff in Clippy where I didn't notice the value was wrapped in `<>`, adding `_trace` makes it easier to spot
dangling ptr lint cleanup This cleans up the dangling_pointers_from_locals lint.
…=ChrisDenton Remove `tests/run-make/fmt-write-bloat/` This test suffers from multiple issues that make it very, very difficult to fix, and even if fixed, it would still be too fragile. So this PR removes `tests/run-make/fmt-write-bloat/`. This PR supersedes rust-lang#143669. r? `@ChrisDenton` (as you reviewed rust-lang#143669 and have context) ### Background context For some background context, this test tries to check that the optimization introduced in [PR-78122] is not regressed. The optimization is for eliding `usize` formatting machinery and padding code from the final binary. Previously, writing any `fmt::Arguments` would cause the `usize` formatting and padding machinery to be included in the final binary since indexing used in `fmt::write` generates code using `panic_bounds_check` (that prints the index and length). Those bounds check are never hit, since `fmt::Arguments` never contain any out-of-bounds indicies. The `Makefile` version of `fmt-write-bloat` was ported to the present `rmake.rs` test infra in [PR-128147]. However, that PR just tries to maintain the original test logic. ### Limitations and problems The original test, it turns out, already have multiple limitations: - It only runs on non-Windows, since the `no_std` test of the original version tries to link against a `libc`. [PR-128807] worked around this by using a substitute name. We re-enabled this test in [PR-142841], but it turns out the assertions are too weak, it will even vacuously pass for no symbols at all. - In [PR-143669], we tried to make this test more robust by comparing the set of expected versus unexpected panic-related symbols, subject to if std was built with debug assertions. However, in working on [PR-143669], we've come to realize that this test is fundamentally very fragile: - The set of panic symbols depend on whether the standard library was built with or without debug assertions. - Different platforms often have different sets of panic machinery modules, functions and paths, and thus different sets of panic symbols. For instance, x86_64 msvc and i686 msvc have different panic code paths. - This comes back to the way the test is trying to gauge the absence of panic symbols -- it tries to look for symbol substring matches for "known" panic symbols. This is fundamentally fragile, because the test is trying to peek into the symbols of the resultant binary post-linking, based on fuzzy matches (the symbols are mangled as well). Based on this assessment, we determined that we should remove this test. This is not intended to exclude the possibility of reintroducing a more robust version of this test. For instance, we could consider some kind of more controllable post-link "end product" integration codegen test suite. [PR-78122]: rust-lang#78122 [PR-128147]: rust-lang#128147 [PR-128807]: rust-lang#128807 [PR-142841]: rust-lang#142841 [PR-143669]: rust-lang#143669
Better warning message for crate type unsupported by codegen backend Turns out cargo doesn't hard code the "for target" part of the message after all.
rustbot
added
A-run-make
Member
Author
|
@bors r+ rollup=never p=5 |
Collaborator
bors
added
S-waiting-on-bors
Member
Author
bors
added
S-waiting-on-author
Member
Author
|
@bors r+ |
Collaborator
bors
added
S-waiting-on-bors
Collaborator
Collaborator
|
☀️ Test successful - checks-actions |
Collaborator
|
📌 Perf builds for each rolled up PR:
previous master: 6a884ad1b5 In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
Contributor
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 6a884ad (parent) -> 8483293 (this PR) Test differencesShow 659 test diffsStage 0
Stage 1
Stage 2
Additionally, 598 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 8483293b3b4eb209e8e8bd0d069e61de790018a8 --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Collaborator
|
Finished benchmarking commit (8483293): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (primary -3.2%, secondary -5.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 474.95s -> 473.503s (-0.30%) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 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.
Successful merges:
strip_circumfixlib feature #147947 (Implementstrip_circumfixlib feature)tests/run-make/fmt-write-bloat/#148393 (Removetests/run-make/fmt-write-bloat/)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup