Skip to content
Open
Show file tree
Hide file tree
Changes from 19 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
124 changes: 124 additions & 0 deletions clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
//===----------------------------------------------------------------------===//
//
// 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();
});
}

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

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)) {
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};
}

void ShadowedNamespaceFunctionCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
functionDecl(isDefinition(), decl(hasDeclContext(translationUnitDecl())),
unless(anyOf(isImplicit(), isVariadic(), isMain(),
isStaticStorageClass(),
ast_matchers::isTemplateInstantiation())))
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary ast_matchers::?

.bind("func"),
this);
}

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

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

const ASTContext *Context = Result.Context;

const FunctionDecl *ShadowedFunc = nullptr;
const NamespaceDecl *ShadowedNamespace = nullptr;

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;

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

const SourceLocation NameLoc = Func->getLocation();
if (NameLoc.isValid() && !Func->getPreviousDecl()) {
const std::string Fix = NamespaceName + "::";
Diag << FixItHint::CreateInsertion(NameLoc, Fix);
}

diag(ShadowedFunc->getLocation(), "function %0 declared here",
DiagnosticIDs::Note)
<< ShadowedFunc->getDeclName();
}

} // namespace clang::tidy::misc
35 changes: 35 additions & 0 deletions clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//===----------------------------------------------------------------------===//
//
// 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;
}
};

} // namespace clang::tidy::misc

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHADOWEDNAMESPACEFUNCTIONCHECK_H
6 changes: 6 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,12 @@ 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,64 @@
.. 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.
Loading
Loading