refactor(parameter): introduce macros to eliminate CheckCompatibility boilerplate#2153
Conversation
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Require kind labelWonderful, this rule succeeded.
🟢 Require version labelWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a set of helper macros (PARAM_CAST_OR_RETURN, CHECK_FIELD_EQ, and CHECK_SUB_PARAM) in a new header file src/utils/param_compat_macros.h to standardize and simplify parameter compatibility checks across various classes, significantly reducing boilerplate code. The review feedback is highly constructive and should be addressed: it identifies a potential null pointer dereference hazard in the CHECK_SUB_PARAM macro when sub-parameters are null, and recommends parenthesizing macro arguments in PARAM_CAST_OR_RETURN to prevent operator precedence issues.
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.
… boilerplate Add PARAM_CAST_OR_RETURN, CHECK_FIELD_EQ, and CHECK_SUB_PARAM macros in src/utils/param_compat_macros.h to replace the repetitive dynamic_pointer_cast + logger::error + return false pattern that was duplicated across 23 parameter files. Signed-off-by: LHT129 <tianlan.lht@antgroup.com> Co-authored-by: opencode <opencode@anthropic.com>
817e5a2 to
3394d98
Compare
Summary
Introduce
PARAM_CAST_OR_RETURN,CHECK_FIELD_EQ, andCHECK_SUB_PARAMmacros insrc/utils/param_compat_macros.hto replace the repetitivedynamic_pointer_cast+logger::error+return falsepattern duplicated across 23 parameter files.24 files changed, +175 / -549 lines (net -374 lines)
Changes
src/utils/param_compat_macros.h— three utility macros for parameter compatibility checksCheckCompatibility()implementations to use the new macrosstd::is_permutation, conditional sub-param checks) is preserved; only the mechanical cast/compare/log boilerplate is replacedTest Plan
[CheckCompatibility]tagged tests pass (46 assertions in 4 test cases)[GNOIMIParameter]and[VectorTransformerParameter]tests passCloses: #2152