InterfaceWithTetGen: ODR violation from directory-scoped -DWITH_TETGEN

[dcwhite]

Latent ODR violation on master, present in the default build configuration. Found while diagnosing the WITH_VTK=OFF breakage on #2667; unrelated to VTK, so filing separately.

The problem

InterfaceWithTetGen.h:70 makes the class definition conditional on a preprocessor flag:

#ifndef WITH_TETGEN
  DISABLED_WITHOUT_ABOVE_COMPILE_FLAG      // adds an isImplementationDisabled() override
#endif

WITH_TETGEN is defined with directory-scoped ADD_DEFINITIONS, in exactly two places:

• src/Modules/Legacy/Fields/CMakeLists.txt:232 (covers that directory and Tests/ below it)
• src/Modules/Factory/CMakeLists.txt:101

But the header is also included from a third directory that neither call covers:

• src/Interface/Modules/Fields/InterfaceWithTetGenDialog.cc / .h

So in a WITH_TETGEN=ON build — and note Superbuild.cmake:131 has OPTION(WITH_TETGEN "Build Tetgen." ON), so this is the default — two translation units in the same program compile two different definitions of class InterfaceWithTetGen:

TU	sees WITH_TETGEN	class has the disable override
Modules/Legacy/Fields/InterfaceWithTetGen.cc	yes	no
Modules/Factory/ModuleFactoryImpl1.cc	yes	no
Interface/Modules/Fields/InterfaceWithTetGenDialog.cc	no	yes

That's an ODR violation — ill-formed, no diagnostic required.

Severity: latent, not currently biting

Being honest about the risk, because it argues for a cheap fix rather than an urgent one:

• The override occupies an existing vtable slot (isImplementationDisabled() is already virtual on ModuleInterface), so object layout is unchanged.
• Which definition wins is decided by whichever TU emits the vtable, via the key function — a non-inline virtual defined in InterfaceWithTetGen.cc, which is on the correct side. So the right definition wins today.
• The dialog TU only reaches into the header for Parameters::* names; it never instantiates the module or calls isImplementationDisabled().

So it works by luck of the key function. LTO, a reordered virtual, or someone making the dtor inline would each be enough to change which definition is emitted — and the failure mode would be a module silently reporting itself disabled in a build that actually has Tetgen.

Fix

Minimal and modern-CMake-correct: make the flag a usage requirement of the target rather than a property of a directory.

target_compile_definitions(Modules_Legacy_Fields PUBLIC WITH_TETGEN)

Anything linking Modules_Legacy_Fields — including the dialog target — then inherits it, and the ADD_DEFINITIONS in Modules/Factory/CMakeLists.txt can go away entirely. Fits #2197 (usage requirements) and #2203 / #2654.

Worth auditing WITH_OSPRAY (src/CMakeLists.txt:392) the same way while in here. It is added at top-level src/, so it is probably consistent — but "probably consistent because of where the call happens to sit" is exactly the property that failed here.

Better fix

Stop making class definitions depend on the preprocessor at all — the constexpr build-flag redesign filed alongside this issue removes this whole category by construction. This issue is the cheap stopgap; that one is the real fix.