Skip to content

Detect C functions that can be declared with static linkage#7127

Merged
danmar merged 2 commits intocppcheck-opensource:mainfrom
mptre:function-static
Jan 10, 2025
Merged

Detect C functions that can be declared with static linkage#7127
danmar merged 2 commits intocppcheck-opensource:mainfrom
mptre:function-static

Conversation

@mptre
Copy link
Copy Markdown
Contributor

@mptre mptre commented Dec 23, 2024

Currently limited to C as I assume the anonymous namespace should be favored for C++.

@chrchr-github
Copy link
Copy Markdown
Collaborator

clang-tidy has misc-use-internal-linkage: https://clang.llvm.org/extra/clang-tidy/checks/misc/use-internal-linkage.html
Not sure if we should attempt to duplicate that functionality.

@mptre
Copy link
Copy Markdown
Contributor Author

mptre commented Dec 23, 2024 via email

@firewave
Copy link
Copy Markdown
Collaborator

There is also the compiler warning -Wmissing-prototypes.

@firewave
Copy link
Copy Markdown
Collaborator

There is also the reverse case to be detected - prototypes which do not have an implementation - see https://trac.cppcheck.net/ticket/10670.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jan 2, 2025

Although, clang-tidy is less effective as it cannot detect functions used in a single TU while it's prototype resides in a header file.

It makes sense to me I think we should have this. Please finish it. 👍

There is also the compiler warning -Wmissing-prototypes.

That does not warn when there is a prototype but the function is only used in 1 TU.

@mptre mptre force-pushed the function-static branch 2 times, most recently from 54c8c8c to 8e5472e Compare January 3, 2025 09:28
@mptre
Copy link
Copy Markdown
Contributor Author

mptre commented Jan 3, 2025

The failing tests have been resolved by now. Anything else that needs to be addressed before this can be merged?

Comment thread lib/checkunusedfunctions.h Outdated
@mptre
Copy link
Copy Markdown
Contributor Author

mptre commented Jan 5, 2025 via email

mptre added 2 commits January 8, 2025 14:15
Currently limited to C as I assume the anonymous namespace should be favored for
C++.
@mptre
Copy link
Copy Markdown
Contributor Author

mptre commented Jan 8, 2025

Rebased to resolve merge conflicts.

@chrchr-github
Copy link
Copy Markdown
Collaborator

It would be pretty easy to cover C++ as well by conditionally adding a sentence about anonymous namespaces.

@danmar danmar merged commit 8b29139 into cppcheck-opensource:main Jan 10, 2025
@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Jan 10, 2025

It would be pretty easy to cover C++ as well by conditionally adding a sentence about anonymous namespaces.

I agree. We probably want to suggest 'static' keyword for C code. But for C++ code we can suggest using either a 'static' keyword or anonymous namespace, I don't feel we need to favor a particular solution.

@chrchr-github
Copy link
Copy Markdown
Collaborator

Also needs an entry in releasenotes.txt

@mptre mptre deleted the function-static branch January 11, 2025 06:33
mptre added a commit to mptre/cppcheck that referenced this pull request Jan 11, 2025
mptre added a commit to mptre/cppcheck that referenced this pull request Jan 11, 2025
danmar pushed a commit that referenced this pull request Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants