fix(clang-tidy): re-enable bugprone-assignment-in-if-condition#12552
Open
vish0012 wants to merge 2 commits intoautowarefoundation:mainfrom
Open
fix(clang-tidy): re-enable bugprone-assignment-in-if-condition#12552vish0012 wants to merge 2 commits intoautowarefoundation:mainfrom
vish0012 wants to merge 2 commits intoautowarefoundation:mainfrom
Conversation
Add NOLINTNEXTLINE markers to the 45 sites flagged by bugprone-assignment-in-if-condition and re-enable the check in .clang-tidy-ci. Both files are derived from upstream third-party code where the assignment-in-if pattern is intentional: - planning/autoware_freespace_planning_algorithms/src/reeds_shepp.cpp is derived from OMPL's ReedsSheppStateSpace.cpp (BSD). Each path family computes a candidate length L and compares it against L_min in the same expression; rewriting all 39 sites would diverge from the OMPL upstream and make future cross-checking harder. - perception/autoware_bytetrack/lib/src/lapjv.cpp is the upstream LAPJV port (MIT, Yifu Zhang). The 6 sites are all expansions of the NEW(x, t, n) macro from lapjv.h, which uses the canonical malloc-or-bail idiom 'if ((x = malloc(...)) == 0) return -1;'. Refs: autowarefoundation#12450 Signed-off-by: Vishal Chauhan <40782713+vish0012@users.noreply.github.com>
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12552 +/- ##
===========================================
+ Coverage 18.65% 33.13% +14.47%
===========================================
Files 1917 175 -1742
Lines 131368 23658 -107710
Branches 44506 12306 -32200
===========================================
- Hits 24501 7838 -16663
+ Misses 86756 8206 -78550
+ Partials 20111 7614 -12497
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Description
Re-enables
bugprone-assignment-in-if-conditionin.clang-tidy-ci.The check was suppressed in #12431 with 45 errors across two files. This PR addresses both affected packages from the issue list.
Refs #12450
Approach
Both flagged files are derived from upstream third-party code where the assignment-in-condition pattern is intentional. Rather than refactoring 45 sites and diverging from the upstream reference implementations, this PR adds per-line
// NOLINTNEXTLINE(bugprone-assignment-in-if-condition)markers.I used
NOLINTNEXTLINEinstead ofNOLINTBEGIN/NOLINTENDbecause the Autoware pre-commit/cpplint pipeline does not accept the block syntax.planning/autoware_freespace_planning_algorithms/src/reeds_shepp.cpp— 39 sitesThis file is derived from OMPL's Reeds-Shepp implementation. The assignment-in-condition pattern is intentional: each path family computes a candidate path length
Land compares it againstL_minin the same expression.Example pattern:
if (LpSpLp(x, y, phi, t, u, v) && L_min > (L = std::abs(t) + std::abs(u) + std::abs(v))) {These patterns are repeated across path families with time-flip, reflect, and time-flip+reflect variants. Refactoring all 39 sites would make the file diverge from the OMPL reference and make future cross-checking harder.
perception/autoware_bytetrack/lib/src/lapjv.cpp— 6 sitesThis file is part of the LAPJV / ByteTrack code. All 6 reported sites are expansions of the
NEW(x, t, n)macro defined inlapjv.h.The macro uses the canonical malloc-or-bail idiom:
The assignment is intentional here. The
NOLINTNEXTLINEcomments are placed at the call sites inlapjv.cppbecause these are the locations reported by clang-tidy.How to reproduce
From the colcon workspace root:
Verification
Reproduced the original errors locally with
clang-tidy-18, then confirmed both affected packages return 0 errors after the fix.Result:
Notes
No local-only include-path workarounds are included in this PR. The only
.clang-tidy-cichange is removing the suppression line:- -bugprone-assignment-in-if-condition,