Skip to content
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/misc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ add_clang_library(clangTidyMiscModule STATIC
NonPrivateMemberVariablesInClassesCheck.cpp
OverrideWithDifferentVisibilityCheck.cpp
RedundantExpressionCheck.cpp
ShadowedNamespaceFunctionCheck.cpp
StaticAssertCheck.cpp
ThrowByValueCatchByReferenceCheck.cpp
UnconventionalAssignOperatorCheck.cpp
Expand Down
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "NonPrivateMemberVariablesInClassesCheck.h"
#include "OverrideWithDifferentVisibilityCheck.h"
#include "RedundantExpressionCheck.h"
#include "ShadowedNamespaceFunctionCheck.h"
#include "StaticAssertCheck.h"
#include "ThrowByValueCatchByReferenceCheck.h"
#include "UnconventionalAssignOperatorCheck.h"
Expand Down Expand Up @@ -65,6 +66,8 @@ class MiscModule : public ClangTidyModule {
"misc-non-private-member-variables-in-classes");
CheckFactories.registerCheck<RedundantExpressionCheck>(
"misc-redundant-expression");
CheckFactories.registerCheck<ShadowedNamespaceFunctionCheck>(
"misc-shadowed-namespace-function");
CheckFactories.registerCheck<StaticAssertCheck>("misc-static-assert");
CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
"misc-throw-by-value-catch-by-reference");
Expand Down
140 changes: 140 additions & 0 deletions clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "ShadowedNamespaceFunctionCheck.h"
#include "../utils/FixItHintUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/STLExtras.h"

using namespace clang;
using namespace clang::ast_matchers;
using namespace clang::tidy;

namespace clang::tidy::misc {

static bool hasSameParameters(const FunctionDecl *Func1,
const FunctionDecl *Func2) {
if (Func1->param_size() != Func2->param_size())
return false;

return llvm::all_of_zip(
Func1->parameters(), Func2->parameters(),
[](const ParmVarDecl *Param1, const ParmVarDecl *Param2) {
return Param1->getType().getCanonicalType() ==
Param2->getType().getCanonicalType();
});
}

void ShadowedNamespaceFunctionCheck::registerMatchers(MatchFinder *Finder) {
// Simple matcher for all function definitions
Finder->addMatcher(functionDecl(isDefinition()).bind("func"), this);
}

void ShadowedNamespaceFunctionCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");

if (!Func || !Result.SourceManager)
return;

// Skip if not in global namespace
const DeclContext *DC = Func->getDeclContext();
if (!DC->isTranslationUnit())
return;

// Skip templates, static functions, main, etc.
if (Func->isTemplated() || Func->isStatic() || Func->isMain() ||
Func->isImplicit() || Func->isVariadic())
return;

const std::string FuncName = Func->getNameAsString();
if (FuncName.empty())
return;

const ASTContext *Context = Result.Context;

// Look for functions with the same name in namespaces
const FunctionDecl *ShadowedFunc = nullptr;
const NamespaceDecl *ShadowedNamespace = nullptr;

// Traverse all declarations in the translation unit
for (const auto *Decl : Context->getTranslationUnitDecl()->decls()) {
if (const auto *NS = dyn_cast<NamespaceDecl>(Decl)) {
std::tie(ShadowedFunc, ShadowedNamespace) =
findShadowedInNamespace(NS, Func, FuncName);
if (ShadowedFunc)
break;
}
}

if (!ShadowedFunc || !ShadowedNamespace)
return;

if (ShadowedFunc->getDefinition())
return;

// Generate warning message
const std::string NamespaceName =
ShadowedNamespace->getQualifiedNameAsString();
auto Diag = diag(Func->getLocation(), "free function %0 shadows '%1::%2'")
<< Func->getDeclName() << NamespaceName
<< ShadowedFunc->getDeclName().getAsString();

// Generate fixit hint to add namespace qualification
const SourceLocation NameLoc = Func->getLocation();
if (NameLoc.isValid() && !Func->getPreviousDecl()) {
const std::string Fix = NamespaceName + "::";
Diag << FixItHint::CreateInsertion(NameLoc, Fix);
}

// Note: Also show where the shadowed function is declared
diag(ShadowedFunc->getLocation(), "function %0 declared here",
DiagnosticIDs::Note)
<< ShadowedFunc->getDeclName();
}

std::pair<const FunctionDecl *, const NamespaceDecl *>
ShadowedNamespaceFunctionCheck::findShadowedInNamespace(
const NamespaceDecl *NS, const FunctionDecl *GlobalFunc,
const std::string &GlobalFuncName) {

// Skip anonymous namespaces
if (NS->isAnonymousNamespace())
return {nullptr, nullptr};

for (const auto *Decl : NS->decls()) {
// Check nested namespaces
if (const auto *NestedNS = dyn_cast<NamespaceDecl>(Decl)) {
auto [ShadowedFunc, ShadowedNamespace] =
findShadowedInNamespace(NestedNS, GlobalFunc, GlobalFuncName);
if (ShadowedFunc)
return {ShadowedFunc, ShadowedNamespace};
}

// Check functions
if (const auto *Func = dyn_cast<FunctionDecl>(Decl)) {
// Skip if it's the same function, templates, or definitions
if (Func == GlobalFunc || Func->isTemplated() ||
Func->isThisDeclarationADefinition())
continue;

if (Func->getNameAsString() == GlobalFuncName && !Func->isVariadic() &&
hasSameParameters(Func, GlobalFunc) &&
Func->getReturnType().getCanonicalType() ==
GlobalFunc->getReturnType().getCanonicalType()) {
return {Func, NS};
}
}
}
return {nullptr, nullptr};
}

} // namespace clang::tidy::misc
41 changes: 41 additions & 0 deletions clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHADOWEDNAMESPACEFUNCTIONCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHADOWEDNAMESPACEFUNCTIONCHECK_H

#include "../ClangTidyCheck.h"
#include <tuple>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be moved to .cpp.


namespace clang::tidy::misc {

/// Detects free functions in global namespace that shadow functions from other
/// namespaces.
///
/// For the user-facing documentation see:
/// https://clang.llvm.org/extra/clang-tidy/checks/misc/shadowed-namespace-function.html
class ShadowedNamespaceFunctionCheck : public ClangTidyCheck {
public:
ShadowedNamespaceFunctionCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}

private:
std::pair<const FunctionDecl *, const NamespaceDecl *>
findShadowedInNamespace(const NamespaceDecl *NS,
const FunctionDecl *GlobalFunc,
const std::string &GlobalFuncName);
};

} // namespace clang::tidy::misc

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHADOWEDNAMESPACEFUNCTIONCHECK_H
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,11 @@ New checks
Finds virtual function overrides with different visibility than the function
in the base class.

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

- New :doc:`readability-redundant-parentheses
<clang-tidy/checks/readability/redundant-parentheses>` check.

Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ Clang-Tidy Checks
:doc:`misc-non-private-member-variables-in-classes <misc/non-private-member-variables-in-classes>`,
:doc:`misc-override-with-different-visibility <misc/override-with-different-visibility>`,
:doc:`misc-redundant-expression <misc/redundant-expression>`, "Yes"
:doc:`misc-shadowed-namespace-function <misc/shadowed-namespace-function>`, "Yes"
:doc:`misc-static-assert <misc/static-assert>`, "Yes"
:doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`,
:doc:`misc-unconventional-assign-operator <misc/unconventional-assign-operator>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
.. title:: clang-tidy - misc-shadowed-namespace-function

misc-shadowed-namespace-function
================================

Detects free functions in the global namespace that shadow functions declared
in other namespaces. This check helps prevent accidental shadowing of namespace
functions, which can lead to confusion about which function is being called and
potential linking errors.

Examples
--------

.. code-block:: c++

namespace utils {
void process();
void calculate();
}

// Warning: free function shadows utils::process
void process() {}

// No warning - static function
static void calculate() {}

The check will suggest adding the appropriate namespace qualification:

.. code-block:: diff
- void process() {}
+ void utils::process() {}
The check will not warn about:

- Static functions or member functions;
- Functions in anonymous namespaces;
- The ``main`` function.

Limitations
-----------

- Does not warn about friend functions:

.. code-block:: c++

namespace llvm::gsym {
struct MergedFunctionsInfo {
friend bool operator==(const MergedFunctionsInfo &LHS,
const MergedFunctionsInfo &RHS);
};
}

using namespace llvm::gsym;

bool operator==(const MergedFunctionsInfo &LHS, // no warning in this version
const MergedFunctionsInfo &RHS) {
return LHS.MergedFunctions == RHS.MergedFunctions;
}

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t

namespace { void f1(); }
namespace foo {
void f0();
void f1();
}
namespace {
void f0() {}
void f1() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t


namespace foo { struct A; }
void f1(foo::A);

namespace foo {
struct A{
friend void f0(A);
friend void f1(A);
};
}

// FIXME: provide warning without fixit in these two cases
void f0(foo::A) {}
void f1(foo::A) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t

void f1();
namespace foo {
void f0() {}
void f1() {}
}
void f0() {}
void f1() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t

void f1();
namespace foo {
void f0();
void f1();
}
void foo::f0() {}
void foo::f1() {}
void f0() {}
void f1() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t

void f1(int);
namespace foo {
void f0(short);
void f1(unsigned);
}
void f0(char) {}
void f1(int) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t

int f1();
namespace foo {
char f0();
unsigned f1();
}
short f0() { return {}; }
int f1() { return {}; }
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t

#define VOID_F0 void f0
#define VOID_F1_BRACES_BODY void f1() {}

void f1();
namespace foo {
void f0();
void f1();
}
VOID_F0() {}
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: free function 'f0' shadows 'foo::f0' [misc-shadowed-namespace-function]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
VOID_F1_BRACES_BODY
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: free function 'f1' shadows 'foo::f1' [misc-shadowed-namespace-function]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t

namespace foo {
int main();
}
int main() {}
Loading
Loading