This or r261737 seem to be causing a bot failure: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/636
-- Sean Silva On Wed, Feb 24, 2016 at 5:35 AM, Alexander Kornienko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > 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 { > +}; > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
