Modernize C++ to C++17: enum class, unique_ptr, constexpr, dead code removal#30
Merged
Modernize C++ to C++17: enum class, unique_ptr, constexpr, dead code removal#30
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request modernizes the C++ codebase from C++11 to C++17 standard with comprehensive updates to improve code quality and safety. The changes include upgrading compiler flags, converting traditional enums to enum classes, replacing raw pointers with unique_ptr, modernizing type aliases, and making appropriate use of constexpr. The PR also fixes a bug where pB was never initialized in NonlinearLayer.cpp and removes dead code. All 601 tests pass.
Changes:
- Build system upgraded to C++17 with ARM compatibility improvements
- Six enums converted to
enum classwith Cython interop macro - Eight factory functions converted to return
std::unique_ptrfor better memory safety - Type aliases modernized from
typedeftousing, constants upgraded toconstexpr
Reviewed changes
Copilot reviewed 13 out of 49 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Upgraded C++ standard to C++17, added ARM architecture detection for SSE3 flag, removed force=True from cythonize |
| Common.h/cpp | Converted type aliases, constants to constexpr, enums to enum class, added ENUM_CLASS_CYTHON_COMPAT macro |
| NonlinearTMM.h/cpp | Updated return types to unique_ptr, qualified enum values, removed dead code |
| NonlinearLayer.h/cpp | Fixed pB initialization bug, qualified enum values with scope |
| Waves.h/cpp | Converted WaveType to enum class, updated function signatures |
| SecondOrderNLTMM.h/cpp | Updated return types to unique_ptr, qualified enum values |
| Material.h/cpp | Line ending normalization only |
| CppSecondOrderNLTMM.pxd | Added unique_ptr import, updated enum string aliases for enum class qualification |
| SecondOrderNLTMM.pyx | Added .release() calls for unique_ptr returns |
| Python files | Line ending normalization only |
| Test files | Line ending normalization only |
| Documentation files | Line ending normalization only |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s, emplace_back, std::array, [[maybe_unused]]
d786a1f to
f26c906
Compare
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
Modernize the C++ codebase from C++11 to C++17 with corresponding Cython binding updates. All 601 tests pass.
Changes
Build system (
setup.py)-std=c++11to-std=c++17(Linux/macOS) and add/std:c++17(Windows MSVC)-msse3flag behind x86 architecture check for ARM compatibilityforce=Truefromcythonize()C++ modernization (
Common.h/cpp)typedef→usingaliases (dcomplex,Tensor3d,pairdd)const→constexprfor compile-time constants;inline constforconstI(non-literal type)enum class:WaveDirection,NonlinearProcess,Polarization,NonlinearTmmMode,TMMParam,TMMParamTypeENUM_CLASS_CYTHON_COMPATmacro providingoperator|,operator<<,operator*for Cython interopWlToOmega,OmegaToWl,sqrtoconstexpr/inlinein headerconst&forApplyRotationMatrixToTensorparametersC++ modernization (NonlinearLayer, NonlinearTMM, Waves, SecondOrderNLTMM)
enum classscope throughoutconstexpr int iF/iBpattern forArray2cdindexing with enum class valuesstd::unique_ptrnew/deletewith stack allocation ormake_uniqueNULLwithnullptrthroughoutcerrbeforethrow,breakafterreturn/throwBug fix
pBwas never initialized inNonlinearLayer.cpp(line was duplicatingpF = Vector3cd::Zero())Cython updates (
.pxd/.pyx)unique_ptrimport fromlibcpp.memoryenum classqualification.release()to 8unique_ptrcall sitesunique_ptr[T]Line endings
.gitattributes