[clang-tools-extra] r261737 - [clang-tidy] Added a check for forward declaration in the potentially wrong namespace
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 24 05:35:33 PST 2016
Author: alexfh
Date: Wed Feb 24 07:35:32 2016
New Revision: 261737
URL: http://llvm.org/viewvc/llvm-project?rev=261737&view=rev
Log:
[clang-tidy] Added a check for forward declaration in the potentially wrong namespace
Adds a new check "misc-forward-declaration-namespace".
In check, A forward declaration is considerred in a potentially wrong namespace
if there is any definition/declaration with the same name exists in a different
namespace.
Reviewers: akuegel, hokein, alexfh
Patch by Eric Liu!
Differential Revision: http://reviews.llvm.org/D17195
Added:
clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-forward-declaration-namespace.rst
clang-tools-extra/trunk/test/clang-tidy/misc-forward-declaration-namespace.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=261737&r1=261736&r2=261737&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Wed Feb 24 07:35:32 2016
@@ -6,6 +6,7 @@ add_clang_library(clangTidyMiscModule
AssignOperatorSignatureCheck.cpp
BoolPointerImplicitConversionCheck.cpp
DefinitionsInHeadersCheck.cpp
+ ForwardDeclarationNamespaceCheck.cpp
InaccurateEraseCheck.cpp
IncorrectRoundings.cpp
InefficientAlgorithmCheck.cpp
Added: clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp?rev=261737&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.cpp Wed Feb 24 07:35:32 2016
@@ -0,0 +1,174 @@
+//===--- ForwardDeclarationNamespaceCheck.cpp - clang-tidy ------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ForwardDeclarationNamespaceCheck.h"
+#include <stack>
+#include <string>
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void ForwardDeclarationNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+ // Match all class declarations/definitions *EXCEPT*
+ // 1. implicit classes, e.g. `class A {};` has implicit `class A` inside `A`.
+ // 2. nested classes declared/defined inside another class.
+ // 3. template class declaration, template instantiation or
+ // specialization (NOTE: extern specialization is filtered out by
+ // `unless(hasAncestor(cxxRecordDecl()))`).
+ auto IsInSpecialization = hasAncestor(
+ decl(anyOf(cxxRecordDecl(isExplicitTemplateSpecialization()),
+ functionDecl(isExplicitTemplateSpecialization()))));
+ Finder->addMatcher(
+ cxxRecordDecl(
+ hasParent(decl(anyOf(namespaceDecl(), translationUnitDecl()))),
+ unless(isImplicit()), unless(hasAncestor(cxxRecordDecl())),
+ unless(isInstantiated()), unless(IsInSpecialization),
+ unless(classTemplateSpecializationDecl()))
+ .bind("record_decl"),
+ this);
+
+ // Match all friend declarations. Classes used in friend declarations are not
+ // marked as referenced in AST. We need to record all record classes used in
+ // friend declarations.
+ Finder->addMatcher(friendDecl().bind("friend_decl"), this);
+}
+
+void ForwardDeclarationNamespaceCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ if (const auto *RecordDecl =
+ Result.Nodes.getNodeAs<CXXRecordDecl>("record_decl")) {
+ StringRef DeclName = RecordDecl->getName();
+ if (RecordDecl->isThisDeclarationADefinition()) {
+ DeclNameToDefinitions[DeclName].push_back(RecordDecl);
+ } else {
+ // If a declaration has no definition, the definition could be in another
+ // namespace (a wrong namespace).
+ // NOTE: even a declaration does have definition, we still need it to
+ // compare with other declarations.
+ DeclNameToDeclarations[DeclName].push_back(RecordDecl);
+ }
+ } else {
+ const auto *Decl = Result.Nodes.getNodeAs<FriendDecl>("friend_decl");
+ assert(Decl && "Decl is neither record_decl nor friend decl!");
+
+ // Classes used in friend delarations are not marked referenced in AST,
+ // so we need to check classes used in friend declarations manually to
+ // reduce the rate of false positive.
+ // For example, in
+ // \code
+ // struct A;
+ // struct B { friend A; };
+ // \endcode
+ // `A` will not be marked as "referenced" in the AST.
+ if (const TypeSourceInfo *Tsi = Decl->getFriendType()) {
+ QualType Desugared = Tsi->getType().getDesugaredType(*Result.Context);
+ FriendTypes.insert(Desugared.getTypePtr());
+ }
+ }
+}
+
+static bool haveSameNamespaceOrTranslationUnit(const CXXRecordDecl *Decl1,
+ const CXXRecordDecl *Decl2) {
+ const DeclContext *ParentDecl1 = Decl1->getLexicalParent();
+ const DeclContext *ParentDecl2 = Decl2->getLexicalParent();
+
+ // Since we only matched declarations whose parent is Namespace or
+ // TranslationUnit declaration, the parent should be either a translation unit
+ // or namespace.
+ if (ParentDecl1->getDeclKind() == Decl::TranslationUnit ||
+ ParentDecl2->getDeclKind() == Decl::TranslationUnit) {
+ return ParentDecl1 == ParentDecl2;
+ }
+ assert(ParentDecl1->getDeclKind() == Decl::Namespace &&
+ "ParentDecl1 declaration must be a namespace");
+ assert(ParentDecl2->getDeclKind() == Decl::Namespace &&
+ "ParentDecl2 declaration must be a namespace");
+ auto *Ns1 = NamespaceDecl::castFromDeclContext(ParentDecl1);
+ auto *Ns2 = NamespaceDecl::castFromDeclContext(ParentDecl2);
+ return Ns1->getOriginalNamespace() == Ns2->getOriginalNamespace();
+}
+
+static std::string getNameOfNamespace(const CXXRecordDecl *Decl) {
+ const auto *ParentDecl = Decl->getLexicalParent();
+ if (ParentDecl->getDeclKind() == Decl::TranslationUnit) {
+ return "(global)";
+ }
+ const auto *NsDecl = cast<NamespaceDecl>(ParentDecl);
+ std::string Ns;
+ llvm::raw_string_ostream OStream(Ns);
+ NsDecl->printQualifiedName(OStream);
+ OStream.flush();
+ return Ns.empty() ? "(global)" : Ns;
+}
+
+void ForwardDeclarationNamespaceCheck::onEndOfTranslationUnit() {
+ // Iterate each group of declarations by name.
+ for (const auto &KeyValuePair : DeclNameToDeclarations) {
+ const auto &Declarations = KeyValuePair.second;
+ // If more than 1 declaration exists, we check if all are in the same
+ // namespace.
+ for (const auto *CurDecl : Declarations) {
+ if (CurDecl->hasDefinition() || CurDecl->isReferenced()) {
+ continue; // Skip forward declarations that are used/referenced.
+ }
+ if (FriendTypes.count(CurDecl->getTypeForDecl()) != 0) {
+ continue; // Skip forward declarations referenced as friend.
+ }
+ if (CurDecl->getLocation().isMacroID() ||
+ CurDecl->getLocation().isInvalid()) {
+ continue;
+ }
+ // Compare with all other declarations with the same name.
+ for (const auto *Decl : Declarations) {
+ if (Decl == CurDecl) {
+ continue; // Don't compare with self.
+ }
+ if (!CurDecl->hasDefinition() &&
+ !haveSameNamespaceOrTranslationUnit(CurDecl, Decl)) {
+ diag(CurDecl->getLocation(),
+ "declaration '%0' is never referenced, but a declaration with "
+ "the same name found in another namespace '%1'")
+ << CurDecl->getName() << getNameOfNamespace(Decl);
+ diag(Decl->getLocation(), "a declaration of '%0' is found here",
+ DiagnosticIDs::Note)
+ << Decl->getName();
+ break; // FIXME: We only generate one warning for each declaration.
+ }
+ }
+ // Check if a definition in another namespace exists.
+ const auto DeclName = CurDecl->getName();
+ if (DeclNameToDefinitions.find(DeclName) == DeclNameToDefinitions.end()) {
+ continue; // No definition in this translation unit, we can skip it.
+ }
+ // Make a warning for each definition with the same name (in other
+ // namespaces).
+ const auto &Definitions = DeclNameToDefinitions[DeclName];
+ for (const auto *Def : Definitions) {
+ diag(CurDecl->getLocation(),
+ "no definition found for '%0', but a definition with "
+ "the same name '%1' found in another namespace '%2'")
+ << CurDecl->getName() << Def->getName() << getNameOfNamespace(Def);
+ diag(Def->getLocation(), "a definition of '%0' is found here",
+ DiagnosticIDs::Note)
+ << Def->getName();
+ }
+ }
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.h?rev=261737&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/ForwardDeclarationNamespaceCheck.h Wed Feb 24 07:35:32 2016
@@ -0,0 +1,59 @@
+//===--- ForwardDeclarationNamespaceCheck.h - clang-tidy --------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FORWARDDECLARATIONNAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FORWARDDECLARATIONNAMESPACECHECK_H
+
+#include <set>
+#include <vector>
+#include "llvm/ADT/SmallPtrSet.h"
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Checks if an unused forward declaration is in a wrong namespace.
+///
+/// The check inspects all unused forward declarations and checks if there is
+/// any declaration/definition with the same name, which could indicate
+/// that the forward declaration is potentially in a wrong namespace.
+///
+/// \code
+/// namespace na { struct A; }
+/// namespace nb { struct A {} };
+/// nb::A a;
+/// // warning : no definition found for 'A', but a definition with the same
+/// name 'A' found in another namespace 'nb::'
+/// \endcode
+///
+/// This check can only generate warnings, but it can't suggest fixes at this
+/// point.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-forward-declaration-namespace.html
+class ForwardDeclarationNamespaceCheck : public ClangTidyCheck {
+public:
+ ForwardDeclarationNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void onEndOfTranslationUnit() override;
+
+private:
+ llvm::StringMap<std::vector<const CXXRecordDecl *>> DeclNameToDefinitions;
+ llvm::StringMap<std::vector<const CXXRecordDecl *>> DeclNameToDeclarations;
+ llvm::SmallPtrSet<const Type *, 16> FriendTypes;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_FORWARDDECLARATIONNAMESPACECHECK_H
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=261737&r1=261736&r2=261737&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Wed Feb 24 07:35:32 2016
@@ -15,6 +15,7 @@
#include "AssignOperatorSignatureCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
#include "DefinitionsInHeadersCheck.h"
+#include "ForwardDeclarationNamespaceCheck.h"
#include "InaccurateEraseCheck.h"
#include "IncorrectRoundings.h"
#include "InefficientAlgorithmCheck.h"
@@ -55,6 +56,8 @@ public:
"misc-bool-pointer-implicit-conversion");
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
+ CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
+ "misc-forward-declaration-namespace");
CheckFactories.registerCheck<InaccurateEraseCheck>(
"misc-inaccurate-erase");
CheckFactories.registerCheck<IncorrectRoundings>(
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=261737&r1=261736&r2=261737&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Feb 24 07:35:32 2016
@@ -51,6 +51,7 @@ Clang-Tidy Checks
misc-assign-operator-signature
misc-bool-pointer-implicit-conversion
misc-definitions-in-headers
+ misc-forward-declaration-namespace
misc-inaccurate-erase
misc-incorrect-roundings
misc-inefficient-algorithm
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-forward-declaration-namespace.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-forward-declaration-namespace.rst?rev=261737&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-forward-declaration-namespace.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-forward-declaration-namespace.rst Wed Feb 24 07:35:32 2016
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - misc-forward-declaration-namespace
+
+misc-forward-declaration-namespace
+==================================
+
+Checks if an unused forward declaration is in a wrong namespace.
+
+The check inspects all unused forward declarations and checks if there is any
+declaration/definition with the same name existing, which could indicate that
+the forward declaration is in a potentially wrong namespace.
+
+.. code:: c++
+ namespace na { struct A; }
+ namespace nb { struct A {}; }
+ nb::A a;
+ // warning : no definition found for 'A', but a definition with the same name
+ // 'A' found in another namespace 'nb::'
+
+This check can only generate warnings, but it can't suggest a fix at this point.
Added: clang-tools-extra/trunk/test/clang-tidy/misc-forward-declaration-namespace.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-forward-declaration-namespace.cpp?rev=261737&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-forward-declaration-namespace.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-forward-declaration-namespace.cpp Wed Feb 24 07:35:32 2016
@@ -0,0 +1,163 @@
+// RUN: %check_clang_tidy %s misc-forward-declaration-namespace %t
+
+namespace {
+// This is a declaration in a wrong namespace.
+class T_A;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration 'T_A' is never referenced, but a declaration with the same name found in another namespace 'na' [misc-forward-declaration-namespace]
+// CHECK-MESSAGES: note: a declaration of 'T_A' is found here
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: no definition found for 'T_A', but a definition with the same name 'T_A' found in another namespace '(global)' [misc-forward-declaration-namespace]
+// CHECK-MESSAGES: note: a definition of 'T_A' is found here
+}
+
+namespace na {
+// This is a declaration in a wrong namespace.
+class T_A;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration 'T_A' is never referenced, but a declaration with the same name found in another namespace '(anonymous)'
+// CHECK-MESSAGES: note: a declaration of 'T_A' is found here
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: no definition found for 'T_A', but a definition with the same name 'T_A' found in another namespace '(global)'
+// CHECK-MESSAGES: note: a definition of 'T_A' is found here
+}
+
+class T_A;
+
+class T_A {
+ int x;
+};
+
+class NESTED;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: no definition found for 'NESTED', but a definition with the same name 'NESTED' found in another namespace '(anonymous namespace)::nq::(anonymous)'
+// CHECK-MESSAGES: note: a definition of 'NESTED' is found here
+
+namespace {
+namespace nq {
+namespace {
+class NESTED {};
+}
+}
+}
+
+namespace na {
+class T_B;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration 'T_B' is never referenced, but a declaration with the same name found in another namespace 'nb'
+// CHECK-MESSAGES: note: a declaration of 'T_B' is found here
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: no definition found for 'T_B', but a definition with the same name 'T_B' found in another namespace 'nb'
+// CHECK-MESSAGES: note: a definition of 'T_B' is found here
+}
+
+namespace nb {
+class T_B;
+}
+
+namespace nb {
+class T_B {
+ int x;
+};
+}
+
+namespace na {
+class T_B;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration 'T_B' is never referenced, but a declaration with the same name found in another namespace 'nb'
+// CHECK-MESSAGES: note: a declaration of 'T_B' is found here
+// CHECK-MESSAGES: :[[@LINE-3]]:7: warning: no definition found for 'T_B', but a definition with the same name 'T_B' found in another namespace 'nb'
+// CHECK-MESSAGES: note: a definition of 'T_B' is found here
+}
+
+// A simple forward declaration. Although it is never used, but no declaration
+// with the same name is found in other namespace.
+class OUTSIDER;
+
+namespace na {
+// This class is referenced declaration, we don't generate warning.
+class OUTSIDER_1;
+}
+
+void f(na::OUTSIDER_1);
+
+namespace nc {
+// This class is referenced as friend in OOP.
+class OUTSIDER_1;
+
+class OOP {
+ friend struct OUTSIDER_1;
+};
+}
+
+namespace nd {
+class OUTSIDER_1;
+void f(OUTSIDER_1 *);
+}
+
+namespace nb {
+class OUTSIDER_1;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration 'OUTSIDER_1' is never referenced, but a declaration with the same name found in another namespace 'na'
+// CHECK-MESSAGES: note: a declaration of 'OUTSIDER_1' is found here
+}
+
+
+namespace na {
+template<typename T>
+class T_C;
+}
+
+namespace nb {
+// FIXME: this is an error, but we don't consider template class declaration
+// now.
+template<typename T>
+class T_C;
+}
+
+namespace na {
+template<typename T>
+class T_C {
+ int x;
+};
+}
+
+namespace na {
+
+template <typename T>
+class T_TEMP {
+ template <typename _Tp1>
+ struct rebind { typedef T_TEMP<_Tp1> other; };
+};
+
+// We ignore class template specialization.
+template class T_TEMP<char>;
+}
+
+namespace nb {
+
+template <typename T>
+class T_TEMP_1 {
+ template <typename _Tp1>
+ struct rebind { typedef T_TEMP_1<_Tp1> other; };
+};
+
+// We ignore class template specialization.
+extern template class T_TEMP_1<char>;
+}
+
+namespace nd {
+class D;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration 'D' is never referenced, but a declaration with the same name found in another namespace 'nd::ne'
+// CHECK-MESSAGES: note: a declaration of 'D' is found here
+}
+
+namespace nd {
+namespace ne {
+class D;
+}
+}
+
+int f(nd::ne::D &d);
+
+
+// This should be ignored by the check.
+template <typename... Args>
+class Observer {
+ class Impl;
+};
+
+template <typename... Args>
+class Observer<Args...>::Impl {
+};
More information about the cfe-commits
mailing list