Skip to content

[tmva] Remove unused LinkDef5.h - #23002

Merged
guitargeek merged 1 commit into
root-project:masterfrom
kutsibalci:tmva-remove-unused-linkdef5
Aug 4, 2026
Merged

[tmva] Remove unused LinkDef5.h#23002
guitargeek merged 1 commit into
root-project:masterfrom
kutsibalci:tmva-remove-unused-linkdef5

Conversation

@kutsibalci

Copy link
Copy Markdown
Contributor

What

Removes tmva/tmva/inc/LinkDef5.h, which has not been part of any dictionary since 2015.

Why

The split of TMVA into libTMVA and libTMVAGui (5f684bf, 2015-04-28) dropped the
#include "LinkDef5.h" line from tmva/tmva/inc/LinkDef.h but left the file itself in place.
It has been unreachable ever since:

  • tmva/tmva/inc/LinkDef.h includes only LinkDef1.h through LinkDef4.h.
  • No CMakeLists.txt or .cmake file in the repository refers to LinkDef5.h, and the TMVA
    package relies on the default LINKDEF value of LinkDef.h (cmake/modules/RootMacros.cmake:1364).
  • Every symbol it declares is already declared in tmva/tmvagui/inc/LinkDef.h, which belongs to
    the module that actually defines those classes — TMVA::MethodInfo and TMVA::StatDialogMVAEffs
    in tmva/tmvagui/inc/TMVA/mvaeffs.h, TMVA::StatDialogBDT in tmva/tmvagui/inc/TMVA/BDT.h.
    Comparing the two files, LinkDef5.h contains no symbol that tmvagui/inc/LinkDef.h does not
    already contain, so nothing is lost by removing it.

Two later commits edited the file without noticing it was dead — eec693f (2020, dropping a
tmvaglob.h pragma) and e5f5cad (2024, the __CINT____CLING__ sweep). That is the
maintenance cost this removal avoids.

Verification

Configured a build with the file 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
LinkDef5 0
tmva/tmva/inc/LinkDef.h 2
tmva/tmvagui/inc/LinkDef.h 2

All three TMVA dictionaries are still generated: G__TMVA.cxx, G__TMVAGui.cxx, G__TMVAUtils.cxx.

AI-assisted coding disclosure

This contribution was AI-assisted (Claude Code). The tool was used to scan LinkDef files across
the repository for entries whose classes are not declared in the module that owns them, and to
draft this description.

The finding was verified before opening the PR rather than taken on trust: the file was confirmed
unreferenced repository-wide, the 2015 commit that removed the include was traced, the symbol sets
of the two LinkDef files were compared, and the configuration check above was run against the
branch with the file already deleted. Three other candidates the same scan produced
(ROOT::Internal::RDF::CountHelper, MeanHelper, BufferedFillHelper) were false positives
caused by the R__CLING_PTRCHECK(off) attribute and were discarded rather than reported.

I have reviewed and understood the change and take responsibility for it.

The 2015 split of TMVA into libTMVA and libTMVAGui (5f684bf) dropped
the `#include "LinkDef5.h"` line from tmva/tmva/inc/LinkDef.h, but left
the file itself in place. It has been unreachable ever since: LinkDef.h
includes only LinkDef1.h through LinkDef4.h, no CMakeLists refers to it,
and the TMVA dictionary uses the default LINKDEF of LinkDef.h.

Every symbol it declares is already declared in
tmva/tmvagui/inc/LinkDef.h, which belongs to the module that actually
defines those classes, so nothing is lost by removing it.

Two later sweeps edited the file without noticing it was dead
(eec693f in 2020, e5f5cad in 2024), which is the maintenance cost
this removal avoids.

@guitargeek guitargeek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Indeed, LinkDef5.h is not referred to anywhere in the source tree.

@guitargeek
guitargeek merged commit 017dc6a into root-project:master Aug 4, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants