Skip to content

STYLE: Replace non-POD ctkLogger globals with Q_GLOBAL_STATIC_WITH_ARGS#1409

Open
hjmjohnson wants to merge 2 commits intocommontk:masterfrom
BRAINSia:fix-non-pod-constexpr
Open

STYLE: Replace non-POD ctkLogger globals with Q_GLOBAL_STATIC_WITH_ARGS#1409
hjmjohnson wants to merge 2 commits intocommontk:masterfrom
BRAINSia:fix-non-pod-constexpr

Conversation

@hjmjohnson
Copy link
Copy Markdown
Contributor

@hjmjohnson hjmjohnson commented Apr 2, 2026

Summary

Fixes clazy:non-pod-global-static warnings on file-scope ctkLogger globals by converting them to Q_GLOBAL_STATIC_WITH_ARGS.

This is one of two PRs split from a combined approach (the companion PR #1410 handles string constants via constexpr QLatin1String).

Problem

static ctkLogger logger("name") at file scope has a non-trivial constructor and destructor (it connects to the logging framework at startup), triggering the non-pod-global-static clazy warning. This is a real SIOF risk: if another file-scope static calls the logger during its own initialization, the logger may not be constructed yet.

Approach

Q_GLOBAL_STATIC_WITH_ARGS is a Qt macro that wraps the object in a function-local static with a thread-safe once-guard. The object is constructed lazily on first access and destroyed in reverse order at shutdown — SIOF-safe.

// Before (triggers non-pod-global-static, SIOF risk):
static ctkLogger logger("org.commontk.dicom.DICOMDatabase");

// After (lazy, thread-safe, SIOF-safe):
Q_GLOBAL_STATIC_WITH_ARGS(ctkLogger, logger, ("org.commontk.dicom.DICOMDatabase"))

The macro returns a pointer-like proxy, so all logger.method() call sites become logger->method().

Public Header Notice

Two public headers were also updated:

  • Libs/Core/ctkCorePythonQtDecorators.h
  • Libs/DICOM/Core/ctkDICOMCorePythonQtDecorators.h

These headers each declared static ctkLogger logger(...) at file scope, visible to any translation unit that includes them. Although logger has internal linkage (per-TU, not exported), the type changes from ctkLogger to the QGlobalStatic<ctkLogger> proxy.

Impact on downstream code: Any external code that directly referenced logger by name after including these headers (via logger.method()) would need to update to logger->method(). In practice this is extremely unlikely — logger is an internal implementation detail of the decorator methods defined in these headers, not part of any public class interface.

Note: Q_GLOBAL_STATIC_WITH_ARGS in a header gives each including TU its own instance (Qt wraps the declaration in an anonymous namespace), preserving the same per-TU isolation as the original static.

Comparison with PR #1398

Aspect PR #1398 This PR
Logger globals Q_GLOBAL_STATIC_WITH_ARGS Same
String constants Meyers' singleton constexpr QLatin1String (PR #1410)
QStringList Meyers' singleton Deferred with deprecation strategy (PR #1411)

Files Changed

59 files across:

  • Libs/DICOM/Core/ — ctkDICOMDatabase, ctkDICOMIndexer, all DICOM job/worker files, ctkDICOMCorePythonQtDecorators.h
  • Libs/DICOM/Widgets/ — 9 widget files
  • Libs/Core/ctkJobScheduler.cpp, ctkCorePythonQtDecorators.h
  • Libs/Widgets/ — 4 widget files
  • Libs/Visualization/VTK/Core/ and Widgets/ — 8 files

Testing

Built successfully against Qt5 (cmake-build-clazy-qt5, Apple clang) and Qt6 (cmake-build-clazy-qt6) on macOS ARM64.

Part of the fix for #1407. See also companion PR #1410 for constexpr QLatin1String string constant fixes, and draft PR #1411 for the ctkDICOMModalities API deprecation strategy.

🤖 Generated with Claude Code

@hjmjohnson hjmjohnson force-pushed the fix-non-pod-constexpr branch 4 times, most recently from ef1fe38 to f074096 Compare April 2, 2026 16:34
@hjmjohnson hjmjohnson changed the title STYLE: Fix non-pod-global-static clazy warnings using constexpr and Q_GLOBAL_STATIC STYLE: Replace non-POD ctkLogger globals with Q_GLOBAL_STATIC_WITH_ARGS Apr 2, 2026
@hjmjohnson hjmjohnson force-pushed the fix-non-pod-constexpr branch from b460980 to 89eca08 Compare April 2, 2026 17:05
@hjmjohnson hjmjohnson force-pushed the fix-non-pod-constexpr branch from 89eca08 to 429d94d Compare April 2, 2026 20:29
@hjmjohnson hjmjohnson marked this pull request as draft April 2, 2026 20:33
@hjmjohnson hjmjohnson force-pushed the fix-non-pod-constexpr branch from 429d94d to 9cd24a9 Compare April 3, 2026 13:46
@hjmjohnson
Copy link
Copy Markdown
Contributor Author

hjmjohnson commented Apr 3, 2026

/validate-ctk-pr report superseded by a newer validation run.

2 similar comments
@hjmjohnson
Copy link
Copy Markdown
Contributor Author

hjmjohnson commented Apr 3, 2026

/validate-ctk-pr report superseded by a newer validation run.

@hjmjohnson
Copy link
Copy Markdown
Contributor Author

hjmjohnson commented Apr 3, 2026

/validate-ctk-pr report superseded by a newer validation run.

@hjmjohnson
Copy link
Copy Markdown
Contributor Author

CTK Validation Report for PR #1409

Date: 2026-04-03T16:27:30Z
Platform: Linux 6.17.0-1012-oem x86_64

Role CTK Hash Commit
Proposed 9cd24a9b 9cd24a9 (HEAD -> pr-1409) STYLE: Fix static ctkLogger in public PythonQt decorator headers
Reference 935c4e04 935c4e0 (HEAD) ENH: Add refreshBrowser to the DICOM visual browser

✅ = 0 errors/failures (clean) | ⚠️ = warnings increased vs reference | 🚫 = new errors or test failures vs reference

Validation Actions Performed

# Action Scope Qt Proposed (9cd24a9b) Reference (935c4e04)
1 Build CTK standalone Qt5 0 errors, 26 warnings ✅ ⚠️ 0 errors, 21 warnings
2 Build CTK standalone Qt6 0 errors, 30 warnings ✅ ⚠️ 2 errors, 3 warnings
3 Test CTK standalone Qt5 267/280 passed, 13 failed 255/280 passed, 25 failed
4 Test CTK standalone Qt6 245/255 passed, 10 failed 245/255 passed, 10 failed
5 Build CTK-in-Slicer (clean) Qt5 0 errors, 187 warnings ✅ 0 errors, 187 warnings
6 Build Slicer inner build Qt5 0 errors, 0 warnings ✅ ✅ 0 errors, 0 warnings
7 Test Slicer inner build Qt5 661/670 passed, 9 failed 661/670 passed, 9 failed

CTK-in-Slicer Build Detail (classified warnings)

Source Proposed errors Proposed warnings Reference errors Reference warnings
CTK source code 0 ✅ 1 0 1
CTK dependencies 0 ✅ 186 0 186
Slicer (CTK-related) 0 ✅ 0 ✅ 0 0
Slicer (own code) 0 ✅ 0 ✅ 0 0

Slicer Test Detail (classified failures)

Category Proposed (9cd24a9b) Reference (935c4e04)
CTK-related failures 0 ✅ 0
Slicer-own failures 0 ✅ 0
Total failed 9 9
CTK Test Failures (proposed 9cd24a9b)

Qt5 (13 failures):

Test Status Related PR/Issue
ctkFontButtonTest 🔄 Timeout #1395 — WIP: Fix ctkFontButtonTest hang and disable stale modal dialog tests
ctkLanguageComboBoxTest 🔄 Failed #1400 — BUG: Fix ctkLanguageComboBox locale normalization and selection
ctkWidgetsUtilsTestGrabWidget 🖥️ Failed — — Requires OpenGL display — fails in headless environments
ctkColorPickerButtonEventTranslatorPlayerTest1 🔄 Timeout #1395 — Modal dialog timeout — addressed in test fix PR
ctkDirectoryButtonEventTranslatorPlayerTest1 🔄 Timeout #1395 — Modal dialog timeout — addressed in test fix PR
ctkFileDialogEventTranslatorPlayerTest1 🔄 Timeout #1395 — Modal dialog timeout — addressed in test fix PR
ctkMenuComboBoxEventTranslatorPlayerTest1 🔄 Failed #1395 — Modal dialog event translator failure
vtkLightBoxRendererManagerTest1 🖥️ SEGFAULT — — GPU/GLSL infrastructure — SEGFAULT in headless environments
ctkVTKMagnifyViewTest2OddOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2OddEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKTextPropertyWidgetEventTranslatorPlayerTest1 🔄 Timeout #1395 — Modal dialog timeout — addressed in test fix PR

Qt6 (10 failures):

Test Status Related PR/Issue
CTKPluginFrameworkAppTests 🐛 Subprocess aborted — — Qt6 subprocess abort — pre-existing
ctkLanguageComboBoxTest 🔄 Failed #1400 — BUG: Fix ctkLanguageComboBox locale normalization and selection
ctkWidgetsUtilsTestGrabWidget 🖥️ Failed — — Requires OpenGL display — fails in headless environments
ctkWorkflowWidgetTest1 🐛 Failed — — Qt6 QStateMachine async transitions — widgets not visible in time
ctkWorkflowWidgetTest2 🐛 Failed — — Qt6 QStateMachine async transitions — widgets not visible in time
vtkLightBoxRendererManagerTest1 🖥️ SEGFAULT — — GPU/GLSL infrastructure — SEGFAULT in headless environments
ctkVTKMagnifyViewTest2OddOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2OddEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU
CTK Test Failures (reference 935c4e04)

Qt5 (25 failures):

Test Status Related PR/Issue
ctkBackTraceTest 🐛 Failed — — Fixed on master — symbol resolution on Linux with hidden visibility
ctkLoggerTest1 🐛 Failed — — Fixed on master — default log level expectation
ctkFontButtonTest 🔄 Timeout #1395 — WIP: Fix ctkFontButtonTest hang and disable stale modal dialog tests
ctkLanguageComboBoxTest 🔄 Failed #1400 — BUG: Fix ctkLanguageComboBox locale normalization and selection
ctkMessageBoxDontShowAgainTest 🐛 Failed — — Fixed on master — ButtonRole vs StandardButton
ctkWidgetsUtilsTestGrabWidget 🖥️ Failed — — Requires OpenGL display — fails in headless environments
ctkColorPickerButtonEventTranslatorPlayerTest1 🔄 Timeout #1395 — Modal dialog timeout — addressed in test fix PR
ctkDirectoryButtonEventTranslatorPlayerTest1 🔄 Failed #1395 — Modal dialog timeout — addressed in test fix PR
ctkFileDialogEventTranslatorPlayerTest1 🔄 Timeout #1395 — Modal dialog timeout — addressed in test fix PR
ctkMenuComboBoxEventTranslatorPlayerTest1 🔄 Failed #1395 — Modal dialog event translator failure
ctkPathLineEditEventTranslatorPlayerTest1 🐛 Failed — — Fixed on master — XML widget name update
ctkDICOMPatientDelegateTest1 🐛 Failed — — Fixed on master — default header height
vtkLightBoxRendererManagerTest1 🖥️ SEGFAULT — — GPU/GLSL infrastructure — SEGFAULT in headless environments
ctkVTKErrorLogModelFileLoggingTest1 🐛 Failed — — Fixed on master — Qt6 + line stability
ctkVTKHistogramTest3 🐛 Failed — — Fixed on master — out-of-bounds read with signed VTK_CHAR
ctkVTKPropertyWidgetTest 🐛 Failed — — Fixed on master — float precision comparison
ctkVTKThresholdWidgetTest1 🐛 Failed — — Fixed on master — threshold lost after setRange
ctkVTKSurfaceMaterialPropertyWidgetTest1 🐛 Failed — — Fixed on master — opacity comparison
ctkVTKMagnifyViewTest2OddOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2OddEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKRenderViewEventTranslatorPlayerTest1 🐛 Failed — — Fixed on master — Qt6 protected operator=
ctkVTKScalarBarWidgetEventTranslatorPlayerTest1 🐛 Failed — — Fixed on master — missing XML events
ctkVTKTextPropertyWidgetEventTranslatorPlayerTest1 🔄 Timeout #1395 — Modal dialog timeout — addressed in test fix PR

Qt6 (10 failures):

Test Status Related PR/Issue
CTKPluginFrameworkAppTests 🐛 Subprocess aborted — — Qt6 subprocess abort — pre-existing
ctkLanguageComboBoxTest 🔄 Failed #1400 — BUG: Fix ctkLanguageComboBox locale normalization and selection
ctkWidgetsUtilsTestGrabWidget 🖥️ Failed — — Requires OpenGL display — fails in headless environments
ctkWorkflowWidgetTest1 🐛 Failed — — Qt6 QStateMachine async transitions — widgets not visible in time
ctkWorkflowWidgetTest2 🐛 Failed — — Qt6 QStateMachine async transitions — widgets not visible in time
vtkLightBoxRendererManagerTest1 🖥️ SEGFAULT — — GPU/GLSL infrastructure — SEGFAULT in headless environments
ctkVTKMagnifyViewTest2OddOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2OddEven 🖥️ Failed — — VTK OpenGL render — fails without GPU
ctkVTKMagnifyViewTest2EvenOdd 🖥️ Failed — — VTK OpenGL render — fails without GPU

🔄 = addressed in open PR | 🖥️ = infrastructure/display dependent | 🐛 = known bug | ➖ = wontfix


Generated by /validate-ctk-pr skill | cache: /home/johnsonhj/src/CTK/.claude/validate-cache

@hjmjohnson hjmjohnson marked this pull request as ready for review April 3, 2026 16:28
hjmjohnson and others added 2 commits April 13, 2026 14:31
Static ctkLogger variables have non-trivial constructors (they register
with the logging framework) and non-trivial destructors, making them
non-POD global statics and triggering clazy:non-pod-global-static.
Q_GLOBAL_STATIC_WITH_ARGS wraps the object in a function-local static:
initialization is deferred to first use (lazy), thread-safe via a
once-guard, and destruction occurs in well-defined reverse order at exit,
eliminating the Static Initialization Order Fiasco risk.

Because Q_GLOBAL_STATIC returns a pointer-like proxy rather than a
reference, all logger.method() call sites are updated to logger->method().
Files in Libs/Core, Libs/DICOM/Core, Libs/DICOM/Widgets,
Libs/Visualization/VTK/Core, Libs/Visualization/VTK/Widgets, and
Libs/Widgets require an additional #include <QGlobalStatic> where that
header is not already transitively available.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ctkCorePythonQtDecorators.h and ctkDICOMCorePythonQtDecorators.h each
declared 'static ctkLogger logger(...)' at file scope, triggering the
clazy:non-pod-global-static warning. Although these are public headers,
the logger variable has internal linkage and is not part of any exported
class interface.

Replace with Q_GLOBAL_STATIC_WITH_ARGS for SIOF-safe lazy initialization.
Each translation unit that includes the header still gets its own logger
instance (Q_GLOBAL_STATIC uses an anonymous namespace internally), so
behavior is unchanged. All logger.method() call sites in both headers
are updated to logger->method().

Note: downstream code directly referencing 'logger' by name after
including these headers (unlikely in practice) would need the same
.method() -> ->method() update.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hjmjohnson hjmjohnson force-pushed the fix-non-pod-constexpr branch from 9cd24a9 to c159090 Compare April 13, 2026 19:31
@hjmjohnson
Copy link
Copy Markdown
Contributor Author

@jamesobutler I created a script to build against qt5, qt6, and Slicer qt5 and this pr does not introduce any new warnings, errors, or regressions. It does fix some previous issues, and it improves the clazy static analysis.

Let me know if there is something I can do to keep the CTK updates moving forward.

@jamesobutler jamesobutler requested a review from lassoan April 13, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant