COMP: Fix NOTIFY warnings in Q_PROPERTIES #1242
Draft
Conversation
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
hjmjohnson
changed the title
Contributor
Author
ninja clean; cmake . ;
ninja > logger 2>&1 ;
grep PROP logger| grep CTK |fgrep -v CTK-build |sort |uniq -c |sort -nr > XX.err |
hjmjohnson
force-pushed
the
fix-NOTIFY-warnings
branch
2 times, most recently
from
6e87200 to
7817ef4
Compare
This was referenced Jul 8, 2025
hjmjohnson
force-pushed
the
fix-NOTIFY-warnings
branch
3 times, most recently
from
27053ce to
b7bb3c0
Compare
This was referenced Jul 14, 2025
hjmjohnson
force-pushed
the
fix-NOTIFY-warnings
branch
from
b7bb3c0 to
7816c38
Compare
Contributor
Author
|
I need to build slicer against this DRAFT PR to minimize the number of compiler warnings that originate from CTK. It is a long chain of commits that are being isolated for independant reviewing. |
hjmjohnson
force-pushed
the
fix-NOTIFY-warnings
branch
2 times, most recently
from
0c22878 to
5dc84eb
Compare
jcfr
force-pushed
the
fix-NOTIFY-warnings
branch
from
5dc84eb to
b1517c0
Compare
Ensure QObject-derived classes properly use the `Q_OBJECT` macro for meta-object functionality and include corresponding `.moc` files to support Qt's meta-object compiler. Add missing Q_OBJECT to complete Qt inheritance The Q_OBJECT macro is mandatory for any class that: • Inherits from QObject • Uses signals or slots • Declares properties (Q_PROPERTY) • Needs qobject_cast, metaObject(), or other Qt RTTI features Without it: • Qt’s meta-object compiler (moc) won’t generate necessary code • You may encounter runtime errors or silently broken signals/slots • Clazy and Qt's build system may warn or fail Need to include full class definitions instead of forward definitions. Replace forward declarations that were common in Qt 5, but are now discouraged in Qt 6. Full declarations are required when the Q_OBJECT macro is included as recommended to respect the Qt inheritance expectations.
Added `${CMAKE_CURRENT_SOURCE_DIR}/Libs/QtTesting` and
`${CMAKE_CURRENT_BINARY_DIR}/Libs/QtTesting` to include directories
to ensure proper visibility of QtTesting headers.
The AUTO* cmake support removes Qt5 specific wrapping
mechanisms that would be different in Qt6. Qt6 has
more strict dependance on full class type definitions
that are more easily managed with the AUTO* cmake
builds.
Define UIC search paths in CMake configuration
Simplify and modernize Qt build system integration
Refactor CMake scripts to utilize Qt auto-tools (AUTOMOC, AUTOUIC,
AUTORCC) where applicable, streamline resource and UI handling, and
add support for Qt6. Remove deprecated functions and conditional
processing for obsolete Qt5 versions.
NOTE: This creates a build error that is fixed in the next commit.
```txt
error: incomplete type 'ctkDICOMSchedulerPrivate' named
in nested name specifier
```
Remove unnecessary manual wrapping
AUTO* cmake tools preform the wrapping behaviors in the background.
Remove manual wrapping elements.
• Requires explicit declaration of the enum in Q_ENUMS(...). • Does not support compile-time introspection via QMetaEnum::fromType<T>(). • Print conversions via qDebug() or QVariant conversions not automatically supported. • Replace all Q_ENUMS(...) with Q_ENUM(...) in your QObject classes. • Make sure the enum is declared immediately before the Q_ENUM macro. • After migration, there is seamless integration into Qt’s meta-object system, better introspection, and compatibility with modern Qt5 (5.5+) and Qt6.
As recommended for Qt best practices For improved compliance and to address warnings, marked read-only Q_PROPERTIES as CONSTANT across multiple modules, ensuring adherence to modern Qt conventions.
Emit change signals and add NOTIFY specifiers to Q_PROPERTIES for enhanced reactivity and conformance with modern Qt practices. Recommended changes to address the warning were performed. ```txt warning: Q_PROPERTY should have either NOTIFY or CONSTANT [-Wclazy-qproperty-without-notify] ``` Introduced `CTK_SET_CPP_EMIT` macro for setter functionality with signal emission. Convert signal parameters to be `const reference` Refactor signal definitions to use `const` reference types, ensuring improved performance and adherence to modern Qt coding standards
hjmjohnson
force-pushed
the
fix-NOTIFY-warnings
branch
from
b1517c0 to
82ee9b4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The last 2 commits are the new items that need reviewing.
There are the most prominent warnings from clazy.
ENH: Update Q_PROPERTIES with CONSTANT specifier for Qt best practicesENH: Add NOTIFY signals for Q_PROPERTIESNote
Changes originally integrated through this pull request have been extracted and contributed as independent pull request by @jcfr: