Fix IndexError in constructor args with bitwise operators by ravenCrown0627 · Pull Request #426 · cpplint/cpplint

ravenCrown0627 · 2026-03-24T11:53:54Z

Used Claude to assist me to resolve Issue#223. The result looks good to me. Let me know if there are any room could be improved for these changes.

Summary by CodeRabbit

Bug Fixes
- Fixed linting behavior that could miscount angle brackets and mis-handle constructor default arguments containing operator sequences (e.g., <<, <<=, <=), eliminating crashes and spurious warnings.
Tests
- Added multi-line tests to verify constructors with operator-containing default parameter expressions are processed correctly without warnings or failures.

…constructor args When a constructor has a default parameter value containing the bitwise left-shift operator (e.g. `A(int b, int c, int a = 1 << 1)`), the argument-collapsing loop in CheckForNonStandardConstructs crashed with IndexError because the two '<' characters in '<<' were counted as unmatched template angle brackets, causing the code to seek a non-existent next element. Fix by stripping '<<' occurrences before counting '<' and '>' for bracket-balance detection, and add an explicit bounds guard so that a malformed argument list cannot produce an out-of-range access. Adds a regression test for the exact snippet reported in issue cpplint#223. Fixes cpplint#223 https://claude.ai/code/session_01Ey7pVULvYhZPH5Unf1ZNe1

…well The previous fix only stripped '<<' before bracket-balance counting. Any other operator that starts with '<' (specifically '<=' and the compound '<<=') also contributes a spurious '<' to the count, causing the arg-collapsing loop to incorrectly merge the following constructor argument (or, if there is none, to raise an IndexError without the bounds check). Changes: * Replace the single-pattern re.sub(r"<<") with a compiled regex that strips '<<=', '<=', and '<<' in longest-first order so that compound operators are matched whole rather than partially. * Move the compiled regex object above the outer while-loop so it is created once per call rather than once per argument. * Add regression tests for '<<=' and '<=' default-value cases. Note: '>>' and '>=' are intentionally left alone because an excess '>' count never triggers the joining loop, and stripping '>>' would break nested-template counting (e.g. vector<vector<int>>). https://claude.ai/code/session_01Ey7pVULvYhZPH5Unf1ZNe1

coderabbitai · 2026-03-24T11:54:09Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e5b87dbb-d97f-485f-90a7-1b9eddf321d1

📥 Commits

Reviewing files that changed from the base of the PR and between b34d8f7 and 1489d3e.

📒 Files selected for processing (1)

cpplint.py

🚧 Files skipped from review as they are similar to previous changes (1)

cpplint.py

📝 Walkthrough

Walkthrough

Updated the constructor-argument "comma collapsing" logic in CheckForNonStandardConstructs to avoid miscounting angle brackets when constructor default expressions contain <-starting operators by stripping <<, <<=, and <= sequences and guarding the join loop against index overrun.

Changes

Cohort / File(s)	Summary
Angle-bracket operator handling fix `cpplint.py`	Added `_LANGLE_OPS_RE` and per-argument `cleaned_arg` used to strip `<<`, `<<=`, `<=` before counting `<`/`>` and `(`/`)`. Recomputed `cleaned_arg` after joining arguments and added a guard to break when `i + 1 >= len(constructor_args)` to avoid overrunning the list.
Operator handling test coverage `cpplint_unittest.py`	Added multi-line test cases in `testExplicitSingleArgumentConstructors` to cover constructors whose default parameter expressions include `<<`, `<<=`, and `<=`, ensuring no crash or incorrect lint warnings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I'm a rabbit with a regex bright,
I chase the stray < through code at night,
<< and <= tucked out of sight,
Constructors tidy, linting light,
Hop—no crash—everything's alright! 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing an IndexError that occurs in constructor arguments containing bitwise operators, which aligns perfectly with the changeset modifications to cpplint.py and test additions.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (1)

cpplint_unittest.py (1)

1829-1854: Nice regression coverage for <-prefixed operators; consider one malformed-input guard test too.

The <<, <<=, and <= cases are a strong addition for Issue #223. As a follow-up, adding one malformed constructor-arg snippet would explicitly lock in the new bounds-guard no-crash behavior.

♻️ Proposed addition

             # less-or-equal <=
             self.TestMultiLineLint(
                 """
           class A {
             A(int b, int c, int a = b <= c ? 1 : 0) {}
           };""",
                 "",
             )
+            # Malformed argument list should not crash (bounds-guard regression)
+            malformed_result = self.PerformMultiLineLint(
+                """
+          class A {
+            A(int b, int c = (b <= c ? 1 : 0) {}
+          };"""
+            )
+            assert isinstance(malformed_result, (str, list))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cpplint_unittest.py` around lines 1829 - 1854, Add one
malformed-constructor-argument regression test alongside the existing
TestMultiLineLint cases to ensure the linter doesn't crash on bad input: create
another self.TestMultiLineLint call using the same class/constructor symbol
(class A / A(...)) with a syntactically malformed default parameter containing a
'<' (for example an unterminated shift or stray '<' in the default expression)
and assert an empty string expected output so the test verifies no crash or
exception; place it near the existing <<, <<=, <= tests and reuse the
TestMultiLineLint helper to keep coverage consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@cpplint_unittest.py`:
- Around line 1829-1854: Add one malformed-constructor-argument regression test
alongside the existing TestMultiLineLint cases to ensure the linter doesn't
crash on bad input: create another self.TestMultiLineLint call using the same
class/constructor symbol (class A / A(...)) with a syntactically malformed
default parameter containing a '<' (for example an unterminated shift or stray
'<' in the default expression) and assert an empty string expected output so the
test verifies no crash or exception; place it near the existing <<, <<=, <=
tests and reuse the TestMultiLineLint helper to keep coverage consistent.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97f0a1ad-55c3-4a83-8a72-fe07a6e139c3

📥 Commits

Reviewing files that changed from the base of the PR and between eeaa43b and b34d8f7.

📒 Files selected for processing (2)

cpplint.py
cpplint_unittest.py

aaronliu0130

Thanks for contributing! All of these comments should be addressable by hand.

aaronliu0130 · 2026-03-27T01:14:05Z

cpplint.py


        # collapse arguments so that commas in template parameter lists and function
-        # argument parameter lists don't split arguments in two
+        # argument parameter lists don't split arguments in two.


If you're making this a sentence, please also capitalize.

aaronliu0130 · 2026-03-27T01:16:43Z

cpplint.py


+# Operator sequences beginning with '<' (used to strip before counting angle brackets).
+# Longer sequences must come first so '<<=' is not partially matched as '<<'.
+_LANGLE_OPS_RE = re.compile(r"<<=|<=|<<")


What's "LANGLE"?

If this variable is necessary, it should not be all the way up here for more global things; instead should be before the function and named _RE_PATTERN_SOMETHING_DESCRIPTIVE .

aaronliu0130 · 2026-03-27T01:19:38Z

cpplint.py

+        # brackets.  Longer sequences must come first so that '<<=' is not
+        # partially matched as '<<' leaving a stray '='.
+        # Note: '>>' and '>=' are intentionally left alone because extra '>'
+        # characters do not trigger the joining loop (condition is
+        # count('<') > count('>')), and stripping '>>' would break nested
+        # template types such as vector<vector<int>>.


These important comments about the design of the compiled regex should not be separated from the compiled regex.

aaronliu0130 · 2026-03-27T01:22:03Z

cpplint_unittest.py

+                "",
+            )
+            # No crash or warning for constructors with '<'-containing operators
+            # in default parameter values (regression test for issue #223).


Suggested change

# in default parameter values (regression test for issue #223).

# `<`-containing operators in constructors are not confused with improper templates

aaronliu0130 · 2026-03-27T01:23:45Z

cpplint.py

+        # Strip operator sequences that begin with '<' before counting angle
+        # brackets, to avoid confusing operators with unmatched template


could be terser

aaronliu0130 · 2026-03-27T01:39:57Z

cpplint.py

@@ -3972,15 +3976,27 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, nesting_state,
            constructor_args = explicit_constructor_match.group(2).split(",")


Why not make one regex replacement here instead of needing to define a variable?

aaronliu0130 · 2026-03-27T01:48:53Z

cpplint_unittest.py

+            self.TestMultiLineLint(
+                """
+          class A {
+            A(int b, int c, int a = (x <<= 1)) {}


Since the error involves comma-joining logic, we should rotate the parameter order in tests, even though I suspect doing so won't catch any real bugs.

claude and others added 3 commits February 26, 2026 12:45

Merge branch 'cpplint:develop' into claude/resolve-cpplint-223-Eq1fW

b34d8f7

coderabbitai bot reviewed Mar 24, 2026

View reviewed changes

fix(regex): add regex for left angle operator sequences in cpplint.py

1489d3e

aaronliu0130 requested changes Mar 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix IndexError in constructor args with bitwise operators#426

Fix IndexError in constructor args with bitwise operators#426
ravenCrown0627 wants to merge 4 commits intocpplint:developfrom
ravenCrown0627:claude/resolve-cpplint-223-Eq1fW

ravenCrown0627 commented Mar 24, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 24, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

aaronliu0130 left a comment

Uh oh!

aaronliu0130 Mar 27, 2026

Uh oh!

aaronliu0130 Mar 27, 2026

Uh oh!

aaronliu0130 Mar 27, 2026

Uh oh!

aaronliu0130 Mar 27, 2026

Uh oh!

aaronliu0130 Mar 27, 2026

Uh oh!

aaronliu0130 Mar 27, 2026

Uh oh!

aaronliu0130 Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# in default parameter values (regression test for issue #223).
	# `<`-containing operators in constructors are not confused with improper templates

		# Strip operator sequences that begin with '<' before counting angle
		# brackets, to avoid confusing operators with unmatched template

		@@ -3972,15 +3976,27 @@ def CheckForNonStandardConstructs(filename, clean_lines, linenum, nesting_state,
		constructor_args = explicit_constructor_match.group(2).split(",")

Conversation

ravenCrown0627 commented Mar 24, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 left a comment

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

aaronliu0130 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ravenCrown0627 commented Mar 24, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 24, 2026 •

edited

Loading