WIP interface categories - #2142
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the FeatureProviderInterface on FeatureExtraction, FeatureMatching, and StructureFromMotion nodes, defining the getFeaturesFolders method for each. It also updates the SUM_OR_DYNAMIC macro in numeric.hpp to cast arguments to integers. Feedback on the macro recommends parenthesizing arguments and the overall expression to avoid precedence issues, and using static_cast instead of functional casts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| Mat3 LookAt2(const Vec3& eyePosition3D, const Vec3& center3D = Vec3::Zero(), const Vec3& upVector3D = Vec3::UnitY()); | ||
|
|
||
| #define SUM_OR_DYNAMIC(x, y) (x == Eigen::Dynamic || y == Eigen::Dynamic) ? Eigen::Dynamic : (x + y) | ||
| #define SUM_OR_DYNAMIC(x, y) (x == Eigen::Dynamic || y == Eigen::Dynamic) ? Eigen::Dynamic : (int(x) + int(y)) |
There was a problem hiding this comment.
Precedence and Casting Issues in Macro Definition
- Lack of Parenthesization: The macro arguments
xandyare not parenthesized inside the macro body, and the entire macro expression is not wrapped in outer parentheses. This can lead to unexpected operator precedence bugs when the macro is used in larger expressions (for example,5 + SUM_OR_DYNAMIC(a, b)orSUM_OR_DYNAMIC(a, b) * 2). - C-style/Functional Casts: Using functional casts like
int(x)is discouraged in C++. It is safer and more idiomatic to usestatic_cast<int>(x)to avoid potential casting issues and adhere to modern C++ standards.
Suggested Refactoring:
Wrap the macro arguments and the entire expression in parentheses, and use static_cast<int> for explicit type conversion.
| #define SUM_OR_DYNAMIC(x, y) (x == Eigen::Dynamic || y == Eigen::Dynamic) ? Eigen::Dynamic : (int(x) + int(y)) | |
| #define SUM_OR_DYNAMIC(x, y) (((x) == Eigen::Dynamic || (y) == Eigen::Dynamic) ? Eigen::Dynamic : (static_cast<int>(x) + static_cast<int>(y))) |
7f41a5d to
086b30f
Compare
|
1 participant
See WIP interface categories