run-make: explaing why fmt-write-bloat is ignore-windows by ChrisDenton · Pull Request #128807 · rust-lang/rust (original) (raw)

Rollup merge of #148393 - jieyouxu:remove-fmt-write-bloat, r=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 #143669.

r? @ChrisDenton (as you reviewed #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.