llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Denis Mikhailov (denzor200) <details> <summary>Changes</summary> 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](https://github.com/user-attachments/files/23588696/11.log) based on trunk https://github.com/llvm/llvm-project/commit/070f3310ccfd040271aae982f36c008f2c34e11d Closes https://github.com/llvm/llvm-project/issues/6739 --- Full diff: https://github.com/llvm/llvm-project/pull/168406.diff 8 Files Affected: - (modified) clang-tools-extra/clang-tidy/misc/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp (+3) - (added) clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.cpp (+124) - (added) clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.h (+35) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/misc/shadowed-namespace-function.rst (+64) - (added) clang-tools-extra/test/clang-tidy/checkers/misc/shadowed-namespace-function.cpp (+182) ``````````diff diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt index 6214ee92927f2..d18af237283ff 100644 --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -34,6 +34,7 @@ add_clang_library(clangTidyMiscModule STATIC NonPrivateMemberVariablesInClassesCheck.cpp OverrideWithDifferentVisibilityCheck.cpp RedundantExpressionCheck.cpp + ShadowedNamespaceFunctionCheck.cpp StaticAssertCheck.cpp ThrowByValueCatchByReferenceCheck.cpp UnconventionalAssignOperatorCheck.cpp diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp index 347fa2a82e43c..2706fe647fe14 100644 --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -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" @@ -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"); diff --git a/clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.cpp b/clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.cpp new file mode 100644 index 0000000000000..b1865ca459088 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.cpp @@ -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()))) + .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 diff --git a/clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.h b/clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.h new file mode 100644 index 0000000000000..93dc69f157281 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/ShadowedNamespaceFunctionCheck.h @@ -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> + +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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 666865cfb2fcd..42772b5335dd1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -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. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index e2875604af72b..473d82033758f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -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>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/shadowed-namespace-function.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/shadowed-namespace-function.rst new file mode 100644 index 0000000000000..5dc8930dde782 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/shadowed-namespace-function.rst @@ -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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/shadowed-namespace-function.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/shadowed-namespace-function.cpp new file mode 100644 index 0000000000000..28e2f0c56df54 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/shadowed-namespace-function.cpp @@ -0,0 +1,182 @@ +// RUN: %check_clang_tidy %s misc-shadowed-namespace-function %t + +void f1_general(); +namespace foo_general { + void f0_general(); + void f1_general(); +} +void f0_general() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: free function 'f0_general' shadows 'foo_general::f0_general' [misc-shadowed-namespace-function] +// CHECK-FIXES: void foo_general::f0_general() {} +void f1_general() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: free function 'f1_general' shadows 'foo_general::f1_general' [misc-shadowed-namespace-function] +// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes + +////////////////////////////////////////////////////////////////////////////////////////// + +void f1_variadic(...); +namespace foo_variadic { + void f0_variadic(...); + void f1_variadic(...); +} + +// FIXME: warning in these two cases?? +// FIXME: fixit for f0?? +void f0_variadic(...) {} +void f1_variadic(...) {} + +////////////////////////////////////////////////////////////////////////////////////////// + +using my_int = int; +using my_short = short; +using my_short2 = short; + +my_int f1_using(my_short2); +namespace foo_using { + int f0_using(short); + int f1_using(short); +} +my_int f0_using(my_short) {} +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: free function 'f0_using' shadows 'foo_using::f0_using' [misc-shadowed-namespace-function] +// CHECK-FIXES: my_int foo_using::f0_using(my_short) {} +my_int f1_using(my_short) {} +// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: free function 'f1_using' shadows 'foo_using::f1_using' [misc-shadowed-namespace-function] +// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes + +////////////////////////////////////////////////////////////////////////////////////////// + +template<typename T> +void f1_template(); +namespace foo { + template<typename T> + void f0_template(); + template<typename T> + void f1_template(); +} + +// FIXME: provide warning in these two cases +// FIXME: provide fixit for f0 +template<typename T> +void f0_template() {} +template<typename T> +void f1_template() {} + +////////////////////////////////////////////////////////////////////////////////////////// + +static void f1_static(); +namespace foo_static { + void f0_static(); + void f1_static(); +} +static void f0_static() {} +static void f1_static() {} + +////////////////////////////////////////////////////////////////////////////////////////// + +void f1_nested(); +namespace foo_nested::foo2::foo3 { + void f0_nested(); + void f1_nested(); +} +void f0_nested() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: free function 'f0_nested' shadows 'foo_nested::foo2::foo3::f0_nested' [misc-shadowed-namespace-function] +// CHECK-FIXES: void foo_nested::foo2::foo3::f0_nested() {} +void f1_nested() {} +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: free function 'f1_nested' shadows 'foo_nested::foo2::foo3::f1_nested' [misc-shadowed-namespace-function] +// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes + +////////////////////////////////////////////////////////////////////////////////////////// + +namespace foo_main { + int main(); +} +int main() {} + +////////////////////////////////////////////////////////////////////////////////////////// + +#define VOID_F0 void f0_macro +#define VOID_F1_BRACES_BODY void f1_macro() {} + +void f1_macro(); +namespace foo_macro { + void f0_macro(); + void f1_macro(); +} +VOID_F0() {} +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: free function 'f0_macro' shadows 'foo_macro::f0_macro' [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_macro' shadows 'foo_macro::f1_macro' [misc-shadowed-namespace-function] +// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes + +////////////////////////////////////////////////////////////////////////////////////////// + +void f1_good(); +namespace foo_good { + void f0_good() {} + void f1_good() {} +} +void f0_good() {} +void f1_good() {} + +////////////////////////////////////////////////////////////////////////////////////////// + +void f1_good2(); +namespace foo_good2 { + void f0_good2(); + void f1_good2(); +} +void foo_good2::f0_good2() {} +void foo_good2::f1_good2() {} +void f0_good2() {} +void f1_good2() {} + +////////////////////////////////////////////////////////////////////////////////////////// + +void f1_good3(int); +namespace foo_good3 { + void f0_good3(short); + void f1_good3(unsigned); +} +void f0_good3(char) {} +void f1_good3(int) {} + +////////////////////////////////////////////////////////////////////////////////////////// + +int f1_good4(); +namespace foo_good4 { + char f0_good4(); + unsigned f1_good4(); +} +short f0_good4() { return {}; } +int f1_good4() { return {}; } + +////////////////////////////////////////////////////////////////////////////////////////// + +namespace foo_friend { struct A; } +void f1_friend(foo_friend::A); + +namespace foo_friend { + struct A{ + friend void f0_friend(A); + friend void f1_friend(A); + }; +} + +// FIXME: provide warning without fixit in these two cases +void f0_friend(foo_friend::A) {} +void f1_friend(foo_friend::A) {} + +////////////////////////////////////////////////////////////////////////////////////////// + +namespace { void f1_anon(); } +namespace foo_anon { + void f0_anon(); + void f1_anon(); +} +namespace { +void f0_anon() {} +void f1_anon() {} +} + +////////////////////////////////////////////////////////////////////////////////////////// `````````` </details> https://github.com/llvm/llvm-project/pull/168406 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
