[math] Remove unused aggregate LinkDef headers - #23004
Open
kutsibalci wants to merge 2 commits into
Open
Conversation
LinkDefAll.h was added in 880fe6d (2014) when Smatrix built a single combined dictionary, and it exists only to include LinkDef.h and LinkDefD32.h together. The build has since gone back to two separate dictionaries: Smatrix uses LINKDEF LinkDef.h and the G__Smatrix32 MULTIDICT uses LINKDEF LinkDefD32.h. Nothing refers to LinkDefAll.h any more, and LinkDef headers are excluded from installation (RootMacros.cmake:1237), so it is not reachable by downstream users either.
Same situation as the Smatrix aggregate header: added in 880fe6d (2014) for a combined dictionary, and it only includes LinkDef_GenVector.h and LinkDef_GenVector32.h. GenVector now builds two separate dictionaries, one per LinkDef, so the aggregate header is unreferenced. LinkDef headers are excluded from installation (RootMacros.cmake:1237) and so cannot be used downstream.
Contributor
Author
|
One CI job is red —
Happy to rebase if you would like it re-run, or to dig further if you think it is genuinely related. |
Test Results 23 files 23 suites 3d 16h 20m 3s ⏱️ For more details on these failures, see this check. Results for commit 5dbd790. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes two aggregate
LinkDefheaders that are no longer part of any dictionary:math/smatrix/inc/LinkDefAll.hmath/genvector/inc/Math/LinkDef_GenVectorAll.hOne commit per module, since the two are independently revertable.
Why
Both were added in 880fe6d (2014-01-30, "Fix the CMake build"), when each package built a
single combined dictionary. Each file exists only to pull its two real LinkDefs together:
The build has since returned to two separate dictionaries per package, each with its own LinkDef:
LINKDEFG__SmatrixLinkDef.hG__Smatrix32(MULTIDICT)LinkDefD32.hG__GenVectorMath/LinkDef_GenVector.hG__GenVector32(MULTIDICT)Math/LinkDef_GenVector32.hThat leaves the aggregate headers unreferenced. Neither name appears in any
CMakeLists.txtor.cmakefile in the repository, and no otherLinkDefincludes them. They are also not reachablefrom outside the repository:
ROOT_INSTALL_HEADERSexcludes LinkDef headers from installation(
cmake/modules/RootMacros.cmake:1237,set(options REGEX "LinkDef" EXCLUDE)).Verification
Configured a build with both files removed (Ninja,
-DCMAKE_BUILD_TYPE=Release -Dtesting=ON -Droottest=OFF, GCC 13.3 on Ubuntu 24.04). Configuration completes with no errors, and thegenerated
build.ninjacontains:LinkDefAllLinkDef_GenVectorAllmath/smatrix/inc/LinkDef.hLinkDefD32.hLinkDef_GenVector.hLinkDef_GenVector32.hAll four dictionaries are still generated:
G__Smatrix.cxx,G__Smatrix32.cxx,G__GenVector.cxx,G__GenVector32.cxx.Related
Same class of leftover as #23002 (
[tmva] Remove unused LinkDef5.h), found by the same sweep.The two are independent; this one can be taken or dropped on its own.
For completeness, the sweep flagged two further candidates that I am not proposing here:
test/guiviewerLinkDef.h, becauseguiviewer.cxx/guiviewer.hare still present and documentedin
test/Readme.md, so the LinkDef is only as orphaned as the example itself; and the LinkDefsunder
math/experimental/genvectorx/, because that package builds no dictionary at all today andis only configured with
sycl AND experimental_genvectorx. Both look like questions for theowners rather than obvious removals — happy to open an issue on either if useful.
AI-assisted coding disclosure
This contribution was AI-assisted (Claude Code). The tool was used to build a reachability graph
over every
LinkDeffile in the repository (CMake references,#includeedges between LinkDefs,and the default
LinkDef.hconvention), and to draft this description.The results were verified before opening the PR rather than taken on trust: both files were
confirmed unreferenced repository-wide, the 2014 commit that introduced them was read, the current
CMakeLists.txtdictionary definitions were checked by hand, the installation exclusion waslocated in
RootMacros.cmake, and the configuration check above was run against the branch withboth files already deleted. Two of the four candidates the sweep produced were set aside as
described above rather than bundled in.
I have reviewed and understood the change and take responsibility for it.