GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix(readability/casting): ignore calling-convention function pointers by androvonx95 · Pull Request #448 · cpplint/cpplint

Conversation

@androvonx95

@androvonx95 androvonx95 commented Jul 19, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

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

Copy link
Copy Markdown

Summary

  • Fixes false positives in readability/casting for function-pointer typedefs and alias declarations that use a calling-convention macro before * (e.g. Node.js NAPI_CDECL).
  • Extends function-pointer detection in CheckCasts and skips lookalike pointer casts in typedef/using lines inside CheckCStyleCast.
  • Adds regression tests and a CHANGELOG entry.

Fixes #409

Test plan

  • pytest --no-cov cpplint_unittest.py -k 'testDeprecatedCast or testCStyleCast'
  • Full pytest --no-cov cpplint_unittest.py (205 passed locally)
  • CI green on this PR

Summary by CodeRabbit

  • Bug Fixes
    • Reduced false-positive readability/casting warnings for function-pointer typedef and alias declarations using calling-convention macros, including array declarators.
    • Improved recognition of valid function-pointer type patterns during cast-related linting.
  • Tests
    • Added coverage to ensure these declarations are not incorrectly reported.
  • Documentation
    • Added a changelog entry describing the lint correction.

Sorry, something went wrong.

Uh oh!

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

All reactions
@androvonx95 @cursoragent
fix(readability/casting): ignore calling-convention function pointers
659350a
Treat typedef/using forms like (CALLCONV* name)(args) as function
pointer types instead of deprecated or C-style casts (cpplint#409).

Co-authored-by: Cursor <[email protected]>
@coderabbitai

coderabbitai Bot commented Jul 19, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The casting checks now avoid reporting function-pointer typedef and alias declarations with calling-convention macros before *. Unit tests cover the affected forms, and the changelog documents the fix.

Changes

Casting false-positive fix

Layer / File(s) Summary
Function-pointer declaration detection
cpplint.py
CheckCasts recognizes an additional function-pointer pattern, while CheckCStyleCast skips typedef and using declarations whose remainder begins with (.
Regression coverage and changelog
cpplint_unittest.py, CHANGELOG.rst
Tests cover function-pointer aliases with calling-convention macros before *, and the TBA changelog records the fix.

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

Possibly related PRs

  • cpplint/cpplint#377: Extends deprecated/C-style cast detection suppression for function-pointer patterns with corresponding tests.
🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the fix for false positives involving calling-convention function pointers.
Linked Issues check ✅ Passed The changes prevent the false positive described in issue #409 and add regression coverage for the affected declarations.
Out of Scope Changes check ✅ Passed The code changes, regression tests, and changelog entry directly support the linked issue and PR objectives.
✨ 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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

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.

coderabbitai[bot]
coderabbitai Bot reviewed Jul 19, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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.

🧹 Nitpick comments (2)
cpplint.py (2)

6689-6689: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider supporting arrays of function pointers.

Using \w* for the function pointer name restricts it to word characters. This means the regex will fail to match arrays of function pointers, such as (CALLCONV* FuncArray[3])(void), because \w* does not match the [ and ] characters.

To seamlessly handle these valid cases while avoiding spaces and parentheses, consider replacing \w* with [^() ]*. This mirrors the logic used in the adjacent pointer-to-member regex (line 6688).

🛠 Proposed fix
-                    or re.match(r"\((?:\w+\s*)?\*\s*\w*\)\s*\(", matched_funcptr)
+                    or re.match(r"\((?:\w+\s*)?\*\s*[^() ]*\)\s*\(", matched_funcptr)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpplint.py` at line 6689, Update the function-pointer regex alternative near
the adjacent pointer-to-member pattern to allow array declarators in names such
as FuncArray[3]. Replace the restrictive word-character name match with a
character class that excludes parentheses and spaces, preserving the existing
surrounding function-pointer matching behavior.

6831-6836: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Consider edge cases like function parameters and trailing return types.

The typedef and using check perfectly addresses the alias cases mentioned in the issue. However, be aware that this same false positive can still occur if the function-pointer type is used directly as a function parameter or trailing return type, for example:
void Register(int32_t(CALLCONV *)(void));

Since (CALLCONV *) is followed by (void), it will still be flagged as a C-style cast here because the line lacks typedef or using.

While fully addressing this might be out of scope for this specific PR, it's worth keeping in mind or adding a TODO for future improvement.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpplint.py` around lines 6831 - 6836, Extend the false-positive guard near
the existing typedef/using check to recognize function-pointer type syntax used
directly in function parameters or trailing return types, including patterns
such as `(CALLCONV *)` followed by `(void)`. Preserve the current alias handling
and add a focused TODO if complete support is out of scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cpplint.py`:
- Line 6689: Update the function-pointer regex alternative near the adjacent
pointer-to-member pattern to allow array declarators in names such as
FuncArray[3]. Replace the restrictive word-character name match with a character
class that excludes parentheses and spaces, preserving the existing surrounding
function-pointer matching behavior.
- Around line 6831-6836: Extend the false-positive guard near the existing
typedef/using check to recognize function-pointer type syntax used directly in
function parameters or trailing return types, including patterns such as
`(CALLCONV *)` followed by `(void)`. Preserve the current alias handling and add
a focused TODO if complete support is out of scope.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c4f26bba-62e4-47aa-828a-ae55db35b0dd

📥 Commits

Reviewing files that changed from the base of the PR and between 6be492a and 659350a.

📒 Files selected for processing (3)
  • CHANGELOG.rst
  • cpplint.py
  • cpplint_unittest.py

Sorry, something went wrong.

Uh oh!

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

All reactions
@coderabbitai coderabbitai Bot mentioned this pull request Jul 21, 2026
fix(readability/casting): noexcept *functions #377
Open
@androvonx95 androvonx95 mentioned this pull request Jul 22, 2026
Looking for more maintainers #402
Open
yangfan-yf-yf
yangfan-yf-yf suggested changes Jul 25, 2026
View reviewed changes

@yangfan-yf-yf yangfan-yf-yf left a comment •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

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.

One declaration form in the stated scope is still flagged:

typedef int32_t(CALLCONV* FuncArray[3])(void);

On the current head it emits Using deprecated casting style. Use static_cast<int32_t>(...) instead. The new function-pointer alternative uses \w* for the declarator name, so it stops before [3]; the adjacent member-function-pointer pattern already accepts array declarators. Please broaden that declarator portion and add a regression case for this typedef form.

Sorry, something went wrong.

Uh oh!

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

All reactions
@cclauss

cclauss commented Jul 25, 2026 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Below: This branch has conflicts that must be resolved

All reactions

Sorry, something went wrong.

Uh oh!

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

androvonx95 added 2 commits July 25, 2026 22:11
@androvonx95
Merge upstream develop into fix/409-casting-typedef-funcptr
6991b8e
@androvonx95
fix(readability/casting): allow arrays of function pointers in typede…
b2b1cc8
…f/using

Broaden the declarator part of the function-pointer pattern so that array
declarators such as `typedef int32_t(CALLCONV* FuncArray[3])(void);` are
recognized instead of being reported as a deprecated cast.
@aaronliu0130

aaronliu0130 commented Jul 25, 2026

Copy link
Copy Markdown
Member

I'm going to see if I can port changelog-addition to towncrier.

All reactions

Sorry, something went wrong.

Uh oh!

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

@cclauss

cclauss commented Jul 25, 2026

Copy link
Copy Markdown
Member

For slow moving repos, Towncrier always seems to be more process complexity than rebasing but do what you think is best.

aaronliu0130 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@androvonx95

androvonx95 commented Jul 25, 2026

Copy link
Copy Markdown
Author

\w* stopped at the [ so array-of-function-pointer typedefs were still getting flagged. I changed it to [^() ]* (matching the adjacent member-function-pointer pattern) and added regression tests for both calling-convention and regular array typedefs.

I also reran the relevant tests plus the full suite, and everything passes. I resolved the CHANGELOG.rst conflict in the same push as well.

All reactions

Sorry, something went wrong.

Uh oh!

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

@cclauss

cclauss commented Aug 1, 2026

Copy link
Copy Markdown
Member

Re: Towncrier...

  • Bump the "dependencies" group with 3 updates across multiple ecosystems beeware/cricket#105
All reactions

Sorry, something went wrong.

Uh oh!

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

@freakboy3742

freakboy3742 commented Aug 2, 2026

Copy link
Copy Markdown

Re: Towncrier...

  • Bump the "dependencies" group with 3 updates across multiple ecosystems beeware/cricket#105

FTR: A rebase isn't needed; we have a workflow that automatically adds a changenote to dependabot PRs, but in the case of the PR you've tagged, it was misconfigured (missing a required permission) as part of a recent spring cleaning in that project.

cclauss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

@androvonx95
Merge branch 'develop' into fix/409-casting-typedef-funcptr
62c0094
coderabbitai[bot]
coderabbitai Bot reviewed Aug 3, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cpplint_unittest.py (1)

5761-5766: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the result of the foo.cc case.

The test reuses error_collector from the previous Bar.cc checks and asserts bar_header_error. A new error from the foo.cc/foo.hpp case cannot fail this assertion. Reset error_collector before ProcessFileData and assert foo_header_error for this case.

Proposed fix
+            error_collector = ErrorCollector(self.assertTrue)
             cpplint.ProcessFileData(
                 "test/foo.cc",
                 "cc",
                 [r'`#include` "foo.hpp"', ""],
                 error_collector,
             )
-            assert error_collector.Results().count(bar_header_error) == 0
+            assert error_collector.Results().count(foo_header_error) == 0
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpplint_unittest.py` around lines 5761 - 5766, Update the foo.cc test case
around ProcessFileData to reset error_collector before processing, then assert
the collected results against foo_header_error rather than bar_header_error,
preserving the existing Bar.cc assertions.
🧹 Nitpick comments (1)
cpplint_unittest.py (1)

1119-1133: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a positive control for the new suppression.

These tests verify only that function-pointer declarations produce no warning. Add a nearby case that must still report a real cast with similar tokens. This will detect overmatching in the new typedef and using suppression.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpplint_unittest.py` around lines 1119 - 1133, Add a nearby positive-control
test in the relevant cpplint unittest alongside the existing function-pointer
cases, using similar calling-convention/function-pointer tokens but an actual
C-style cast that must produce the expected warning. Keep the current no-warning
assertions unchanged and use TestLint’s expected-output argument to verify the
suppression does not overmatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cpplint_unittest.py`:
- Around line 5761-5766: Update the foo.cc test case around ProcessFileData to
reset error_collector before processing, then assert the collected results
against foo_header_error rather than bar_header_error, preserving the existing
Bar.cc assertions.

---

Nitpick comments:
In `@cpplint_unittest.py`:
- Around line 1119-1133: Add a nearby positive-control test in the relevant
cpplint unittest alongside the existing function-pointer cases, using similar
calling-convention/function-pointer tokens but an actual C-style cast that must
produce the expected warning. Keep the current no-warning assertions unchanged
and use TestLint’s expected-output argument to verify the suppression does not
overmatch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 682e69f1-af3c-4a0c-90ba-f742bc819288

📥 Commits

Reviewing files that changed from the base of the PR and between b2b1cc8 and 62c0094.

📒 Files selected for processing (3)
  • CHANGELOG.rst
  • cpplint.py
  • cpplint_unittest.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • CHANGELOG.rst
  • cpplint.py

Sorry, something went wrong.

Uh oh!

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

All reactions
yangfan-yf-yf
yangfan-yf-yf approved these changes Aug 4, 2026
View reviewed changes

@yangfan-yf-yf yangfan-yf-yf left a comment

Copy link
Copy Markdown

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.

Rechecked at 62c0094.

The function-pointer declarator pattern now accepts array declarators, so the previously reported typedef int32_t(CALLCONV* FuncArray[3])(void); case is clean. I also verified a using alias with the calling-convention form stays clean while an ordinary C-style cast still reports readability/casting.

Verified locally:

python -m pytest --no-cov cpplint_unittest.py -k 'testDeprecatedCast or testCStyleCast' -q
2 passed

python -m pytest --no-cov -q
231 passed

git diff --check upstream/develop...HEAD
Passed

LGTM.

Sorry, something went wrong.

Uh oh!

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

All reactions
PNHD
PNHD approved these changes Aug 14, 2026
View reviewed changes

@PNHD PNHD 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.

Re-reviewed current head 62c0094. The previously reported array-declarator case is fixed, the calling-convention typedef/alias cases are covered, and ordinary C-style casts remain diagnosed. I found no remaining blocker on this head. LGTM.

Sorry, something went wrong.

Uh oh!

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

All reactions
walidbi200
walidbi200 approved these changes Sep 20, 2026
View reviewed changes

@walidbi200 walidbi200 left a comment

Copy link
Copy Markdown

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.

Reviewed current head 62c0094. I reproduced the original calling-convention function-pointer false positive on the base commit and the earlier array-declarator failure, and confirmed both are fixed on this head.

I also exercised typedef and using forms, array declarators, calling-convention spacing variants, ordinary function pointers, pointer-to-member cases, and real C-style cast controls. The broadened function-pointer matching did not suppress the ordinary casts I tested.

The focused casting tests and full suite pass locally, along with pylint, mypy, pre-commit, and git diff --check. I found no regression introduced by this change and no remaining blocker on this head. LGTM.

Sorry, something went wrong.

Uh oh!

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

All reactions
DanceoftheAurora
DanceoftheAurora suggested changes Nov 6, 2026
View reviewed changes

@DanceoftheAurora DanceoftheAurora left a comment

Copy link
Copy Markdown

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.

AI-assisted review of 62c0094. The calling-convention typedef/alias and array-declarator fixes work, but the new exemption at cpplint.py:6811-6812 also hides real casts inside type aliases:

void* p = nullptr;
using Ptr = decltype((int*)(p));
typedef decltype((int*)(p)) Ptr2;

Both aliases pass clang++ -std=c++17 -pedantic-errors -fsyntax-only. Each produces readability/casting on the base, but no casting diagnostic on this head. These are pointer casts, not function-pointer declarators.

This concretely exercises the earlier positive-control concern: typedef/using anywhere on the line plus a parenthesized operand is too broad. Please restrict the exemption to the declarator context and add these real-cast controls. Ordinary casts still report, and the two existing focused casting tests pass locally (Python 3.12/macOS).

Sorry, something went wrong.

Uh oh!

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

All reactions
@DanceoftheAurora

DanceoftheAurora commented Nov 6, 2026

Copy link
Copy Markdown

I prepared a follow-up for the alias-cast regression on the unchanged head 62c0094: commit 901f6f6. Feel free to cherry-pick or adapt it.

It limits the exemption to the top level of the declared type, preserving casts inside decltype, template arguments, and later statements. The controls retain your calling-convention/array forms, attributed aliases, and aliases inside same-line namespace/class scopes. All 9 real-cast cases warn after the fix; all 13 valid declaration controls remain clean. The complete control fixture compiles with strict C++17.

Local validation on Python 3.12/macOS: 232 tests passed, 96.44% coverage; Pylint, Mypy, and all configured pre-commit hooks passed. This is still a focused same-line heuristic, not a complete declarator parser. AI-assisted implementation and review; the reported checks were executed.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
+4 more reviewers
@DanceoftheAurora DanceoftheAurora DanceoftheAurora requested changes
@PNHD PNHD PNHD approved these changes
@walidbi200 walidbi200 walidbi200 approved these changes
@yangfan-yf-yf yangfan-yf-yf yangfan-yf-yf 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.

[readability/casting] false positive with typedef

Uh oh!

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

8 participants

@androvonx95 @cclauss @aaronliu0130 @freakboy3742 @DanceoftheAurora @PNHD @walidbi200 @yangfan-yf-yf
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.