forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit 76066d7
committed
test: remove
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#143669tests/run-make/fmt-write-bloat/
1 parent bd3ac03 commit 76066d7Copy full SHA for 76066d7
Expand file treeCollapse file tree
2 files changed
+0
-67
lines changedOpen diff view settings
Collapse file
tests/run-make/fmt-write-bloat/main.rs
Copy file name to clipboardExpand all lines: tests/run-make/fmt-write-bloat/main.rs-32Lines changed: 0 additions & 32 deletions
This file was deleted.
Collapse file
tests/run-make/fmt-write-bloat/rmake.rs
Copy file name to clipboardExpand all lines: tests/run-make/fmt-write-bloat/rmake.rs-35Lines changed: 0 additions & 35 deletions
This file was deleted.
0 commit comments