-
Notifications
You must be signed in to change notification settings - Fork 5.5k
-
|
Hello! I have a PR (#127295) intended to remove unused Goals & Benefits
ApproachThe initial pass was generated by an automated tool without cross-platform context, then iterated to fix issues across all target platforms. The final result was validated through binary-level comparison of compiled output across all CI platforms and configurations. Validation MethodologyTwo CI builds were run, publishing
Artifacts were downloaded (~15.7 GB across 40 platform/config combinations) and compared using platform-appropriate tools: ResultsDisassemblable objects (x64, ARM64, ARM, RISC-V, WASM): 11,301 files compared. 5,868 were byte-identical. The remaining 3,919 diffs were exclusively LTCG/WPO objects (Windows Release): All 1,255 objects had diffs fully explained by removed header path strings, changed anonymous namespace hashes ( LoongArch64 native objects: 87 identical code sections, 132 with immediate-field-only diffs ( x86_64 cross-compiled objects (LoongArch build): 4 identical, 30 with constant-only ConclusionThis PR produces zero code-generation changes across all platforms and configurations. Every observed diff falls into one of two categories:
No removed include provided macros, constants, type definitions, or declarations that affected any compiled code path on any architecture (x64, ARM64, ARM, RISC-V, WASM, LoongArch64) in any configuration (Debug, Checked, Release). All CI pipelines pass. Feedback WelcomeI understand this is a large mechanical change, so I want to make sure it lands cleanly. If you spot an include that should be kept e.g. for documentation, forward-compatibility, or a platform I may have missed, please flag it and I'm happy to restore it. |
Beta Was this translation helpful? Give feedback.
All reactions
-
👎 1
Replies: 1 comment
-
As described multiple times, this task shouldn't be performed without basic understanding of the codebase, and should never be done mechanically.
- Reduced coupling — removing unnecessary transitive dependencies makes the header graph cleaner and easier to reason about
- Improved code clarity — each file's includes reflect its actual dependencies, making it easier for contributors to understand what a file uses
This is largely incorrect. Many headers are used directly, and the compilation successes because they are happened to brought from transitive dependencies.
For example, callcounting.h is removed from callcounting.cpp. This should never happen with the basic sense of C++ standard practice.
The final result was validated through binary-level comparison of compiled output across all CI platforms and configurations.
This repo contains code that's currently never enabled in any configuration, but still has usage during development. CI is not enough for validating them.
Beta Was this translation helpful? Give feedback.