run-make: explaing why fmt-write-bloat is ignore-windows by ChrisDenton · Pull Request #128807 · rust-lang/rust
bors added a commit to rust-lang-ci/rust that referenced this pull request
Aug 8, 2024run-make: run fmt-write-bloat on Windows The trouble here is that libc doesn't exist on Windows. Well it kinda does but it isn't called that so we substitute a name that works. Ideally finding necessary libs for the platform would be done at a higher level but until then this should work. try-job: x86_64-msvc try-job: i686-msvc
bors
added
S-waiting-on-author
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.labels
Aug 8, 2024bors added a commit to rust-lang-ci/rust that referenced this pull request
Aug 8, 2024run-make: run fmt-write-bloat on Windows The trouble here is that libc doesn't exist on Windows. Well it kinda does but it isn't called that so we substitute a name that works. Ideally finding necessary libs for the platform would be done at a higher level but until then this should work. try-job: x86_64-msvc try-job: i686-msvc
bors
added
S-waiting-on-bors
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.labels
Aug 8, 2024matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
Aug 8, 2024run-make: run fmt-write-bloat on Windows The trouble here is that libc doesn't exist on Windows. Well it kinda does but it isn't called that so we substitute a name that works. Ideally finding necessary libs for the platform would be done at a higher level but until then this should work. try-job: x86_64-msvc try-job: i686-msvc
bors added a commit to rust-lang-ci/rust that referenced this pull request
Aug 8, 2024…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#128795 (Update E0517 message to reflect RFC 2195.) - rust-lang#128804 (run-make: enable msvc for redundant-libs) - rust-lang#128807 (run-make: run fmt-write-bloat on Windows) r? `@ghost` `@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request
Aug 8, 2024run-make: run fmt-write-bloat on Windows The trouble here is that libc doesn't exist on Windows. Well it kinda does but it isn't called that so we substitute a name that works. Ideally finding necessary libs for the platform would be done at a higher level but until then this should work. try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw
bors added a commit to rust-lang-ci/rust that referenced this pull request
Aug 8, 2024run-make: run fmt-write-bloat on Windows The trouble here is that libc doesn't exist on Windows. Well it kinda does but it isn't called that so we substitute a name that works. Ideally finding necessary libs for the platform would be done at a higher level but until then this should work. try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw
ChrisDenton
changed the title
run-make: run fmt-write-bloat on Windows
run-make: explaing why fmt-write-bloat is ignore-windows
bors
added
S-waiting-on-bors
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.labels
Aug 10, 2024bors added a commit to rust-lang-ci/rust that referenced this pull request
Aug 10, 2024…llaumeGomez Rollup of 8 pull requests Successful merges: - rust-lang#128273 (Improve `Ord` violation help) - rust-lang#128807 (run-make: explaing why fmt-write-bloat is ignore-windows) - rust-lang#128903 (rustdoc-json-types `Discriminant`: fix typo) - rust-lang#128905 (gitignore: Add Zed and Helix editors) - rust-lang#128908 (diagnostics: do not warn when a lifetime bound infers itself) - rust-lang#128909 (Fix dump-ice-to-disk for RUSTC_ICE=0 users) - rust-lang#128910 (Differentiate between methods and associated functions in diagnostics) - rust-lang#128923 ([rustdoc] Stop showing impl items for negative impls) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Aug 10, 2024Rollup merge of rust-lang#128807 - ChrisDenton:bloat, r=jieyouxu run-make: explaing why fmt-write-bloat is ignore-windows The trouble here is that libc doesn't exist on Windows. Well it kinda does but it isn't called that so we substitute a name that works. Ideally finding necessary libs for the platform would be done at a higher level but until then this should work. try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw
jieyouxu added a commit to jieyouxu/rust that referenced this pull request
Nov 2, 2025This 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, 2025Rollup 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
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