fix(braces): exempt C++20 templated lambdas from trailing-semicolon check - #458
fix(braces): exempt C++20 templated lambdas from trailing-semicolon check#458breidenbach0 wants to merge 2 commits into
Conversation
…heck
A C++20 templated lambda like []<typename T>(T&& t) { ... }; was falsely
flagged with readability/braces 'You don't need a ; after a }'. The lambda
exemption relied on the text before '(' ending in ']' (the capture), but a
template parameter list sits between ']' and '(' so the prefix ends in '>'
and the exemption missed it. Also recognize the ]<...> introducer.
Fixes cpplint#385
for more information, see https://pre-commit.ci
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe trailing-semicolon check now suppresses false warnings for C++20 templated lambdas. Regression tests cover single-line and multi-line lambdas and retain warnings for ordinary templated functions. The changelog documents the fix. ChangesTemplated Lambda Semicolon Handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Reviewed at c6a64d4 against current develop.
The same-line / simple multi-line-body cases are fixed, including the exact #385 example, and a templated function still warns. Full suite: 206 passed.
The \\s*<.*>\\s*$ check only looks at the text before ( on that line, so these valid forms still emit readability/braces here (clean on #454):
auto identity = []<
typename T
>(T&& t) { return t; };
auto identity = []
<typename T>
(T&& t) { return t; };
auto identity = []<typename T>
(T&& t) { return t; };
auto identity = []<typename T>
requires std::integral<T>
(T&& t) { return t; };Each compiles under g++ -std=c++20 -pedantic-errors -fsyntax-only. Given the overlap/conflict with #454, I’d lean toward closing this in favor of that PR unless you extend coverage to the multiline/requires cases.
yangfan-yf-yf
left a comment
There was a problem hiding this comment.
I independently rechecked c6a64d4 and confirmed that the multiline template and requires-clause cases already reported in the existing review still emit readability/braces on this head, while #454 handles the same inputs and preserves diagnostics for multiline operator[] declarations and constrained functions. Since the two PRs overlap and conflict, I don't think this narrower implementation should merge as-is. Unless this branch is broadened beyond #454, I recommend closing it in favor of #454.
Fixes #385.
Problem: a C++20 templated lambda like
[]<typename T>(T&& t) { ... };was falsely flaggedreadability/braces— "You don't need a ; after a }."Cause:
CheckTrailingSemicolonexempts lambdas by checking that the text before(ends in](the capture). A templated lambda puts<T>between]and(, so the prefix ends in>and the exemption missed it.Fix: also recognise the
]<...>lambda introducer.Testing:
[]<T>,[&]<T>, multi-param, multi-line,[]<auto>) → no longer flagged;(function /for/while/if, including a templated function) → still flaggedpytest cpplint_unittest.py→ 206 passed; the new regression tests fail without the fix.Summary by CodeRabbit
Bug Fixes
Tests
Documentation