Coding - C++17 modernization sweep (nullptr, using, override, constexpr, throw())#1205
Open
jijinbei wants to merge 8 commits intoOpen-Cascade-SAS:IRfrom
Open
Coding - C++17 modernization sweep (nullptr, using, override, constexpr, throw())#1205jijinbei wants to merge 8 commits intoOpen-Cascade-SAS:IRfrom
jijinbei wants to merge 8 commits intoOpen-Cascade-SAS:IRfrom
Conversation
Mechanical substitution of the NULL macro with the C++11 nullptr keyword across src/. Only code tokens are replaced; occurrences inside string literals, line/block comments, and `#define NULL` directives are preserved. 663 replacements across 75 files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Picks up a file missed by the previous sweep. Same mechanical substitution: NULL → nullptr in code tokens only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies clang-tidy modernize-use-using across the buildable modules
(FoundationClasses, ModelingData, ModelingAlgorithms, Visualization)
plus ApplicationFramework and Draw headers reached through includes.
Manual pre-conversions (13 sites) handle cases that clang-tidy mishandles:
- typedef const T*/&: clang-tidy drops the const qualifier.
- typedef with function-pointer syntax involving const return types.
- One anonymous enum conversion bug (typedef enum { } N) was fixed
across 7 affected files by rewriting to enum N { }.
- One function-type typedef (void(N)()) was corrected to using N = void().
Excluded from modernization:
- Deprecated typedefs in Standard_TypeDef.hxx and Standard_Mutex.hxx
(scheduled for removal, no benefit to touching them).
- Third-party headers: OpenGl_glext.h, OpenGl_khrplatform.h (Khronos).
- The src/Deprecated/ subtree.
Verified by a full Release build (2609/2609 targets, 0 errors, 0 warnings).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies clang-tidy modernize-use-override. Only 2 files required changes; the rest of the codebase already used override consistently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the only remaining throw() in the tree (the pre-C++11 branch of the YY_NOTHROW macro in the bison-generated step.tab.hxx). The branch was already unreachable under OCCT's C++17 requirement, and throw() was removed from the language in C++17. Verified by building TKDESTEP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Converts simple TU-local #define macros that define numeric literals into typed constexpr variables. Only includes macros that are used in expression context (not in #if/#ifdef, not stringified, not pasted) so the substitution is semantically identical. Scope (7 files, 12 macros): - Visualization: NB_VERTICES/NB_BOUNDS/NB_TRIANGLES/NB_FANS, BVH_PRIMITIVE_LIMIT - Draw: MAXPNT, DEFLECTION - DataExchange: FIRSTCHAR - ApplicationFramework: MEMORY_GRANULE, HASH_MASK, MINIMAL_ROOM - FoundationClasses: SLENGTH Not converted: macros used in #ifdef guards (USEOSDREAL), platform compatibility defines (M_SQRT2, MMAP_BASE_ADDRESS), debug-only defines (FLITERAL, MAX_SIBLINGS), and enum-like integer groups (GeomTools surface/curve type codes) which are better addressed with enum class. Verified by building all affected modules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR Open-Cascade-SAS#1205 CI feedback addressed: 1. Revert NULL→nullptr in four .c files (lex.ExprIntrp.c, igesread.c, liriges.c, structiges.c). `nullptr` is a C++ keyword; C only gets it in C23, and the CI targets older C standards (C17/C11). Local builds passed only because GCC 15.2 defaults to gnu23 for .c. The Linux Clang, macOS Clang, and Windows MSVC jobs all failed on this. 2. Apply clang-format 20.1.8 reformatting across 123 files. The NULL→nullptr expansion (+3 chars) desynchronized backslash- continuation alignment in several macros; clang-format restores it. The patch was taken directly from the CI "Check code formatting" job's format.patch artifact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
macOS Clang CI reports -Werror,-Wunused-const-variable on three constexpr variables introduced in the previous commit: NB_BOUNDS, NB_TRIANGLES, NB_FANS. These were dead macros in the original code (declared but never referenced), but #define does not trigger an unused warning while constexpr does. Remove the three unused constants; keep NB_VERTICES which is used. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
CI feedback addressed
|
dpasukhi
added
the
3. CLA waited
Member
|
Thank you for your patch. I'm going to review it, periodically I also apply clang-tidy, but it is always welcome from community. Technically you help me a lot with that script, because I almost forgot before 8.0.0. apply clang tidy |
Member
|
In case If you modify something by hand, not by the script, please let me know ;) |
dpasukhi
added
2. Enhancement
Author
|
Hand edits:
Follow-up fixes:
|
jijinbei
added a commit
to jijinbei/OCCT
that referenced
this pull request
Apr 19, 2026
The four generated/hand-written .c files that got nullptr from the earlier NULL→nullptr sweep fail to compile on Clang (and MSVC in C mode), because nullptr is a C23-only keyword. Linux/macOS Clang and Windows MSVC all report "undeclared identifier 'nullptr'". This is a pre-existing CI failure on master unrelated to the enum class work; same workaround was already written by the user for PR Open-Cascade-SAS#1205 (commit 9419f98). Applied here so the enum-class branch CI can proceed. Affected files: - src/DataExchange/TKDEIGES/IGESFile/igesread.c - src/DataExchange/TKDEIGES/IGESFile/liriges.c - src/DataExchange/TKDEIGES/IGESFile/structiges.c - src/ModelingAlgorithms/TKExpress/ExprIntrp/lex.ExprIntrp.c Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Low-risk, mechanical C++17 modernization sweep across src/. Six commits, each a single-concept change.
Most rewrites were tool-generated (clang-tidy
modernize-use-using/modernize-use-override, plus a small Python script for NULL → nullptr). 13 cases oftypedef const T*and 8 anonymous-enum patterns were hand-converted because clang-tidy mishandles them.Changes
Excluded:
src/Deprecated/, Khronos headers (OpenGl_glext.h,OpenGl_khrplatform.h), deprecated typedefs,#ifdef-guarded macros, enum-like integer groups.Verification
Full Release build on Linux (C++17, GCC 15.2, Ninja,
-O3 -DNDEBUG), all modules. 0 errors, 0 warnings.CLA
Submitted and awaiting approval. Will post the CLA ID here once received.
Notes
CR<issue_id>(no Mantis ticket yet). Can rename.