Skip to content

Conversation

@denzor200
Copy link

Implementation of this check was mostly generated by DeepSeek.

I've tested it on llvm codebase with -DLLVM_ENABLE_PROJECTS="bolt;clang;clang-tools-extra;compiler-rt;cross-project-tests;libc;libclc;lld;lldb;mlir;openmp;polly"

It provided 6 warnings with fixit hints.
See 11.log based on trunk 070f331

Closes #6739

@github-actions
Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

Comment on lines +61 to +62
- Does not warn about template functions
- Does not warn about variadic functions
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing punctuation at the end?


- Does not warn about template functions
- Does not warn about variadic functions

Copy link
Contributor

Choose a reason for hiding this comment

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

Excessive newline.

}

void ShadowedNamespaceFunctionCheck::registerMatchers(MatchFinder *Finder) {
// Simple matcher for all function definitions
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove unrelated (obvious) comments

Comment on lines +45 to +46
if (!Func || !Result.SourceManager)
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just assert, we expect the matcher to work.
Also, it's strange not to have Source Manager, we never check for it

Comment on lines +104 to +107
std::pair<const FunctionDecl *, const NamespaceDecl *>
ShadowedNamespaceFunctionCheck::findShadowedInNamespace(
const NamespaceDecl *NS, const FunctionDecl *GlobalFunc,
const std::string &GlobalFuncName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need to be a class member and not a regular static function?

Comment on lines +54 to +56
if (Func->isTemplated() || Func->isStatic() || Func->isMain() ||
Func->isImplicit() || Func->isVariadic())
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check it in matchers

Comment on lines +48 to +51
// Skip if not in global namespace
const DeclContext *DC = Func->getDeclContext();
if (!DC->isTranslationUnit())
return;
Copy link
Contributor

@vbvictor vbvictor Nov 17, 2025

Choose a reason for hiding this comment

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

Should be checked in matchers, decl(hasDeclContext(translationUnitDecl()))

<< Func->getDeclName() << NamespaceName
<< ShadowedFunc->getDeclName().getAsString();

// Generate fixit hint to add namespace qualification
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto comments

Diag << FixItHint::CreateInsertion(NameLoc, Fix);
}

// Note: Also show where the shadowed function is declared
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto comments

Comment on lines +6 to +7
Detects free functions in the global namespace that shadow functions declared
in other namespaces. This check helps prevent accidental shadowing of namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Detects free functions in the global namespace that shadow functions declared
in other namespaces. This check helps prevent accidental shadowing of namespace
Detects free functions in the global namespace that shadow functions declared
in other namespaces.
This check helps prevent accidental shadowing of namespace

- New :doc:`misc-shadowed-namespace-function
<clang-tidy/checks/misc/shadowed-namespace-function>` check.

Detects free functions in global namespace that shadow functions from other namespaces.
Copy link
Contributor

Choose a reason for hiding this comment

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

80 char limit

@github-actions
Copy link

🐧 Linux x64 Test Results

  • 3053 tests passed
  • 7 tests skipped

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.

[QOI] warn about free functions which shadow one in a namespace

3 participants