GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — fix: allow braces after multiline preprocessor directives by PNHD · Pull Request #480 · cpplint/cpplint

Conversation

PNHD commented Oct 26, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

A brace on its own line is allowed after a preprocessor directive, but the check only looks at its last physical line. Splitting an OpenMP pragma with a backslash therefore produces a whitespace/braces warning.

Follow line continuations back to the start of the directive. The other checks still use the original preceding line. Regression cases cover two- and three-line pragmas, comments and blank lines, and ordinary code that must continue to warn.

Fixes #294.

Validation: 238 tests passed with 96.47% coverage on Python 3.12; Ruff check/format, pylint, and mypy passed. The new regression cases fail on the base revision and pass with the fix.

Summary by CodeRabbit

Bug Fixes
- Fixed false-positive brace-placement warnings when a brace follows a multiline preprocessor directive, including continued OpenMP directives and lines with trailing comments.
- Confirmed that this exception does not suppress brace-placement checks for regular code, such as if statements following a directive.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          fix: allow braces after multiline preprocessor directives

d3f4003

Trace continued lines to their directive before applying the standalone brace warning. Preserve the existing checks for ordinary continued code.

Fixes cpplint#294.

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: a816b67f-010e-4458-b009-8b0c1a57e291

Commits

Reviewing files that changed from the base of the PR and between 926bf4b and d3f4003.

Files selected for processing: 3

CHANGELOG.rst
cpplint.py
cpplint_unittest.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

CheckBraces now follows backslash-continued lines when checking whether a preceding line starts a preprocessor directive. Tests cover brace-only blocks after directives and ordinary code that must still produce a brace-placement warning.

Changes: Brace handling after directives

Layer / File(s)	Summary
Directive continuation handling and validation `cpplint.py`, `cpplint_unittest.py`, `CHANGELOG.rst`	CheckBraces follows continued lines to identify preprocessor directives. Tests cover single-line and continued directives, trailing comments, and ordinary code after directives. The changelog records the fix.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to d3f40

The brace-warning fix appears ready to merge after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: allowing braces after multiline preprocessor directives.
Linked Issues check		The change satisfies issue `#294`. `CheckBraces` follows backslash continuations to the originating preprocessor directive before applying the `whitespace/braces` check. This suppresses the warning for …
Out of Scope Changes check		The reported changes stay within issue `#294`. The `cpplint.py` change implements the required brace-check behavior. The `cpplint_unittest.py` cases provide regression coverage. The `CHANGELOG.rst` entr…

Full details: Linked Issues check

Explanation

The change satisfies issue #294. CheckBraces follows backslash continuations to the originating preprocessor directive before applying the whitespace/braces check. This suppresses the warning for a brace after a multiline OpenMP pragma. New tests cover multiline pragmas, comments, blank lines, and ordinary code that must still warn.

Full details: Out of Scope Changes check

Explanation

The reported changes stay within issue #294. The cpplint.py change implements the required brace-check behavior. The cpplint_unittest.py cases provide regression coverage. The CHANGELOG.rst entry documents the fix. No unrelated change is shown.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Csq57 approved these changes

Oct 27, 2026

View reviewed changes

Csq57 left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I independently reviewed PR #480 at pinned head d3f4003 against develop (926bf4b), using GitHub-hosted Ubuntu, Windows, and macOS with Python 3.12 and 3.15.1 (pytest==9.1.1).

Full-suite comparison (consistent across all three operating systems):

I also ran independent CheckBraces reproductions. Two- and three-line OpenMP directives produce the unwanted whitespace/braces warning on the base but not on this PR. Single-line pragmas remain unaffected, while ordinary if statements, continued if conditions, and separate if statements following pragmas still report the expected warning.

The four Python 3.15.1 failures are identical on the base and PR branches and involve the existing os.path.commonprefix() deprecation warnings. They are unrelated to this PR's changes.

One non-blocking observation: the continuation logic also exempts braces following multiline #define directives. This appears consistent with the checker's existing exemption for immediately preceding preprocessor directives, but an additional regression test could document the intended behavior.

Independent evidence:
https://github.com/Csq57/ai-python-repository-stabilization-case-study/actions/runs/40453763938

The comparison workflow passed, but the Python 3.15.1 pytest runs themselves retained their four known failures.

This review was prepared with AI assistance and grounded in actual independent CI executions.

Approved for the scoped Issue #294 fix based on the tested head.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

DanceoftheAurora added a commit to DanceoftheAurora/cpplint that referenced this pull request

Nov 6, 2026


          Merge current develop into PR cpplint#480; retain both changelog entries

1caf0ad

DanceoftheAurora commented Nov 6, 2026

Copy link

Copy Markdown

I checked the current merge conflict and prepared an optional merge commit on this branch, with parents d3f4003 (this PR) and 880b36e (current develop).

Only CHANGELOG.rst needed manual resolution; it retains both the #294 and #445 entries. The source and tests merged automatically, and the resulting diff against develop preserves this PR's original changes. Locally, 248 tests passed with 96.67% coverage, along with configured Pylint, Mypy, applicable pre-commit hooks and 16 multiline-preprocessor behavior cases. Prepared with AI assistance and independently checked; remote CI has not been run on this merge commit. This is available if it saves you the merge-resolution work.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This branch has not been deployed

No deployments

Reviewers

1 more reviewer

Csq57

Csq57 approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Disable whitespace/braces for blocks after multi-line OpenMP directives

Uh oh!

There was an error while loading. Please reload this page.

3 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

PNHD commented Oct 26, 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 Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Csq57 left a comment

Choose a reason for hiding this comment

Uh oh!

DanceoftheAurora commented Nov 6, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PNHD commented Oct 26, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

coderabbitai Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.