migrate fmt-write-bloat to rmake by lolbinarycat · Pull Request #128147 · rust-lang/rust

@rustbot rustbot added A-testsuite

Area: The testsuite used to check the correctness of rustc

T-bootstrap

Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

labels

Jul 24, 2024

jieyouxu

@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

Jul 24, 2024

@rustbot rustbot 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

Jul 24, 2024

@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

Jul 24, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jul 24, 2024
…r=<try>

migrate fmt-write-bloat to rmake

try-job: aarch64-apple
try-job: x86_64-gnu-llvm-18
try-job: dist-x86_64-linux

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jul 24, 2024
…r=<try>

migrate fmt-write-bloat to rmake

try-job: aarch64-apple
try-job: x86_64-gnu-llvm-18
try-job: dist-x86_64-linux

@lolbinarycat

this commit cannot easily be squashed, since there is already a
PR based on the previous commit.

@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

Jul 30, 2024

tgross35 added a commit to tgross35/rust that referenced this pull request

Jul 31, 2024
…, r=jieyouxu

migrate fmt-write-bloat to rmake

try-job: aarch64-apple
try-job: x86_64-gnu-llvm-18
try-job: dist-x86_64-linux

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jul 31, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#125048 (PinCoerceUnsized trait into core)
 - rust-lang#127681 (derive(SmartPointer): rewrite bounds in where and generic bounds)
 - rust-lang#127830 (When an archive fails to build, print the path)
 - rust-lang#128147 (migrate fmt-write-bloat to rmake)
 - rust-lang#128356 (Migrate `cross-lang-lto-clang` and `cross-lang-lto-pgo-smoketest` `run-make` tests to rmake)
 - rust-lang#128387 (More detailed note to deprecate ONCE_INIT)
 - rust-lang#128388 (Match LLVM ABI in `extern "C"` functions for `f128` on Windows)
 - rust-lang#128412 (Remove `crate_level_only` from `ELIDED_LIFETIMES_IN_PATHS`)

r? `@ghost`
`@rustbot` modify labels: rollup

tgross35 added a commit to tgross35/rust that referenced this pull request

Aug 1, 2024
…, r=jieyouxu

migrate fmt-write-bloat to rmake

try-job: aarch64-apple
try-job: x86_64-gnu-llvm-18
try-job: dist-x86_64-linux

tgross35 added a commit to tgross35/rust that referenced this pull request

Aug 1, 2024
…, r=jieyouxu

migrate fmt-write-bloat to rmake

try-job: aarch64-apple
try-job: x86_64-gnu-llvm-18
try-job: dist-x86_64-linux

tgross35 added a commit to tgross35/rust that referenced this pull request

Aug 1, 2024
…, r=jieyouxu

migrate fmt-write-bloat to rmake

try-job: aarch64-apple
try-job: x86_64-gnu-llvm-18
try-job: dist-x86_64-linux

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 1, 2024

lolbinarycat added a commit to lolbinarycat/rust that referenced this pull request

Aug 2, 2024
uses helper functions added in rust-lang#128147, must not be merged before that
PR.

lolbinarycat added a commit to lolbinarycat/rust that referenced this pull request

Aug 6, 2024
uses helper functions added in rust-lang#128147, must not be merged before that
PR.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Aug 7, 2024
…ke, r=<try>

port tests/run-make/extern-fn-reachable to rmake

uses helper functions added in rust-lang#128147, must not be merged before that PR.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: test-various
try-job: x86_64-msvc
try-job: x86_64-mingw
try-job: i686-msvc
try-job: i686-mingw
try-job: x86_64-gnu-llvm-17

jieyouxu added a commit to jieyouxu/rust that referenced this pull request

Nov 2, 2025
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

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Nov 2, 2025
…=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

rust-timer added a commit that referenced this pull request

Nov 2, 2025
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.

[PR-78122]: #78122
[PR-128147]: #128147
[PR-128807]: #128807
[PR-142841]: #142841
[PR-143669]: #143669