Skip to content

[math] Remove unused aggregate LinkDef headers - #23004

Open
kutsibalci wants to merge 2 commits into
root-project:masterfrom
kutsibalci:math-remove-unused-aggregate-linkdefs
Open

[math] Remove unused aggregate LinkDef headers#23004
kutsibalci wants to merge 2 commits into
root-project:masterfrom
kutsibalci:math-remove-unused-aggregate-linkdefs

Conversation

@kutsibalci

Copy link
Copy Markdown
Contributor

What

Removes two aggregate LinkDef headers that are no longer part of any dictionary:

  • math/smatrix/inc/LinkDefAll.h
  • math/genvector/inc/Math/LinkDef_GenVectorAll.h

One 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:

// math/smatrix/inc/LinkDefAll.h
#include "LinkDefD32.h"
#include "LinkDef.h"

The build has since returned to two separate dictionaries per package, each with its own LinkDef:

package dictionary LINKDEF
Smatrix G__Smatrix LinkDef.h
Smatrix G__Smatrix32 (MULTIDICT) LinkDefD32.h
GenVector G__GenVector Math/LinkDef_GenVector.h
GenVector G__GenVector32 (MULTIDICT) Math/LinkDef_GenVector32.h

That leaves the aggregate headers unreferenced. Neither name appears in any CMakeLists.txt or
.cmake file in the repository, and no other LinkDef includes them. They are also not reachable
from outside the repository: ROOT_INSTALL_HEADERS excludes 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 the
generated build.ninja contains:

reference occurrences
LinkDefAll 0
LinkDef_GenVectorAll 0
math/smatrix/inc/LinkDef.h 2
LinkDefD32.h 2
LinkDef_GenVector.h 2
LinkDef_GenVector32.h 2

All 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, because guiviewer.cxx/guiviewer.h are still present and documented
in test/Readme.md, so the LinkDef is only as orphaned as the example itself; and the LinkDefs
under math/experimental/genvectorx/, because that package builds no dictionary at all today and
is only configured with sycl AND experimental_genvectorx. Both look like questions for the
owners 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 LinkDef file in the repository (CMake references, #include edges between LinkDefs,
and the default LinkDef.h convention), 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.txt dictionary definitions were checked by hand, the installation exclusion was
located in RootMacros.cmake, and the configuration check above was run against the branch with
both 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.

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.
@kutsibalci

Copy link
Copy Markdown
Contributor Author

One CI job is red — ubuntu22 imt=Off, CMAKE_BUILD_TYPE=Debug. I looked into it and believe it is unrelated to this PR, but flagging it rather than leaving it unexplained.

  • The failing test is roottest-root-io-cpp11Containers-unorderedMap, from the test results artifact for that job. It exercises I/O of C++11 unordered containers, which does not touch Smatrix or GenVector dictionaries.
  • 20 of the 21 jobs in the run passed, including every other platform (alma9, alma10, alma10 arm64, fedora43, fedora44, ubuntu2404, mac14, mac15 ARM64). A change that broke dictionary generation would not be confined to a single Debug configuration.
  • There are no compiler-error annotations on the job; it failed after ~79 minutes at the test stage, not during configure.
  • The same job passed on [tmva] Remove unused LinkDef5.h #23002, which is the same kind of removal.

Happy to rebase if you would like it re-run, or to dig further if you think it is genuinely related.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 16h 20m 3s ⏱️
 3 853 tests  3 852 ✅ 0 💤 1 ❌
79 408 runs  79 406 ✅ 1 💤 1 ❌

For more details on these failures, see this check.

Results for commit 5dbd790.

@kutsibalci

Copy link
Copy Markdown
Contributor Author

Attempt #2 failed the same way, and the re-run annotation adds a detail I did not have when I commented above: the failure is a timeout, not an assertion.

  • The annotation reads 1 out of 22 runs failed: roottest-root-io-cpp11Containers-unorderedMap ... [took 30m 0s]. The exact round duration is the signature of the harness killing the test, and there is no compiler or output-diff annotation on the job.
  • That test is the one in its directory carrying LABELS longtest (roottest/root/io/cpp11Containers/CMakeLists.txt) — its siblings forwardList, unorderedSet and Names do not. It is already classified as long-running upstream.
  • The configuration is imt=Off, CMAKE_BUILD_TYPE=Debug, i.e. no implicit multithreading and an unoptimised build — the slowest combination in the matrix, and the only one of 22 where it did not finish in time.
  • The single skipped test in the same run is consistent with this: Names declares FIXTURES_REQUIRED root-io-cpp11Containers-unorderedMap-fixture, so it is skipped when unorderedMap does not complete. That is a cascade from the timeout, not a second failure.

On relevance to the change itself: this PR deletes 5 lines across math/genvector/inc/Math/LinkDef_GenVectorAll.h and math/smatrix/inc/LinkDefAll.h, and nothing else. unorderedMap builds its dictionary from unorderedMap_selection.xml through ROOTTEST_GENERATE_REFLEX_DICTIONARY and exercises I/O of std::unordered_map; it does not reference Smatrix or GenVector. I do not see a path by which these deletions could slow it down.

I did search the issue tracker for a prior report of this test timing out and found none, so I am not able to point at an existing flaky-test issue. If it would be useful I am happy to open one separately from this PR, or to rebase for another run — whichever you prefer. And if you would rather I keep digging on the assumption it is genuinely related, say so and I will.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants