Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -12,6 +12,7 @@ MiscTidyModule.cpp MoveConstructorInitCheck.cpp NoexceptMoveConstructorCheck.cpp + NonCopyableObjects.cpp SizeofContainerCheck.cpp StaticAssertCheck.cpp SwappedArgumentsCheck.cpp Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -20,6 +20,7 @@ #include "MacroRepeatedSideEffectsCheck.h" #include "MoveConstructorInitCheck.h" #include "NoexceptMoveConstructorCheck.h" +#include "NonCopyableObjects.h" #include "SizeofContainerCheck.h" #include "StaticAssertCheck.h" #include "SwappedArgumentsCheck.h" @@ -55,8 +56,9 @@ "misc-move-constructor-init"); CheckFactories.registerCheck( "misc-noexcept-move-constructor"); - CheckFactories.registerCheck( - "misc-sizeof-container"); + CheckFactories.registerCheck( + "misc-non-copyable-objects"); + CheckFactories.registerCheck("misc-sizeof-container"); CheckFactories.registerCheck( "misc-static-assert"); CheckFactories.registerCheck( Index: clang-tidy/misc/NonCopyableObjects.h =================================================================== --- clang-tidy/misc/NonCopyableObjects.h +++ clang-tidy/misc/NonCopyableObjects.h @@ -0,0 +1,31 @@ +//===--- NonCopyableObjects.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_NONCOPYABLEOBJECTS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONCOPYABLEOBJECTS_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// The check flags dereferences and non-pointer declarations of objects that +/// are not meant to be passed by value, such as C FILE objects. +class NonCopyableObjectsCheck : public ClangTidyCheck { +public: + NonCopyableObjectsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONCOPYABLEOBJECTS_H Index: clang-tidy/misc/NonCopyableObjects.cpp =================================================================== --- clang-tidy/misc/NonCopyableObjects.cpp +++ clang-tidy/misc/NonCopyableObjects.cpp @@ -0,0 +1,90 @@ +//===--- NonCopyableObjects.cpp - clang-tidy-------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "NonCopyableObjects.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace { +bool isPOSIXTypeName(StringRef ClassName) { + static const char *TypeNames[] = { + "::pthread_cond_t", + "::pthread_mutex_t", + "pthread_cond_t", + "pthread_mutex_t" + }; + return std::binary_search(std::begin(TypeNames), std::end(TypeNames), + ClassName); +} + +bool isFILETypeName(StringRef ClassName) { + static const char *TypeNames[] = { + "::FILE", + "FILE", + "std::FILE" + }; + return std::binary_search(std::begin(TypeNames), std::end(TypeNames), + ClassName); +} + +AST_MATCHER(NamedDecl, isFILEType) { + return isFILETypeName(Node.getQualifiedNameAsString()); +} + +AST_MATCHER(NamedDecl, isPOSIXType) { + return isPOSIXTypeName(Node.getQualifiedNameAsString()); +} +} // namespace + +namespace tidy { +void NonCopyableObjectsCheck::registerMatchers(MatchFinder *Finder) { + // There are two ways to get into trouble with objects like FILE *: + // dereferencing the pointer type to be a non-pointer type, and declaring + // the type as a non-pointer type in the first place. While the declaration + // itself could technically be well-formed in the case where the type is not + // an opaque type, it's highly suspicious behavior. + // + // POSIX types are a bit different in that it's reasonable to declare a + // non-pointer variable or data member of the type, but it is not reasonable + // to dereference a pointer to the type, or declare a parameter of non-pointer + // type. + auto BadFILEType = hasType(namedDecl(isFILEType()).bind("type_decl")); + auto BadPOSIXType = hasType(namedDecl(isPOSIXType()).bind("type_decl")); + auto BadEitherType = anyOf(BadFILEType, BadPOSIXType); + + Finder->addMatcher( + namedDecl(anyOf(varDecl(BadFILEType), fieldDecl(BadFILEType))) + .bind("decl"), + this); + Finder->addMatcher(parmVarDecl(BadPOSIXType).bind("decl"), this); + Finder->addMatcher( + expr(unaryOperator(hasOperatorName("*"), BadEitherType)).bind("expr"), + this); +} + +void NonCopyableObjectsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *D = Result.Nodes.getNodeAs("decl"); + const auto *BD = Result.Nodes.getNodeAs("type_decl"); + const auto *E = Result.Nodes.getNodeAs("expr"); + + if (D && BD) + diag(D->getLocation(), "'%0' declared as type '%1'; did you mean '%1 *'?") + << D->getName() << BD->getName(); + else if (E) + diag(E->getExprLoc(), "expression has suspicious type '%0'") + << BD->getName(); +} + +} // namespace tidy +} // namespace clang + Index: test/clang-tidy/misc-non-copyable-objects.c =================================================================== --- test/clang-tidy/misc-non-copyable-objects.c +++ test/clang-tidy/misc-non-copyable-objects.c @@ -0,0 +1,43 @@ +// RUN: %python %S/check_clang_tidy.py %s misc-non-copyable-objects %t + +typedef struct FILE {} FILE; +typedef struct pthread_cond_t {} pthread_cond_t; +typedef int pthread_mutex_t; + +// CHECK-MESSAGES: :[[@LINE+1]]:13: warning: 'f' declared as type 'FILE'; did you mean 'FILE *'? +void g(FILE f); +// CHECK-MESSAGES: :[[@LINE+1]]:24: warning: 'm' declared as type 'pthread_mutex_t'; did you mean 'pthread_mutex_t *'? +void h(pthread_mutex_t m); +// CHECK-MESSAGES: :[[@LINE+1]]:23: warning: 'c' declared as type 'pthread_cond_t'; did you mean 'pthread_cond_t *'? +void i(pthread_cond_t c); + +struct S { + pthread_cond_t c; // ok + // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: 'f' declared as type 'FILE'; did you mean 'FILE *'? + FILE f; +}; + +void func(FILE *f) { + // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: 'f1' declared as type 'FILE'; did you mean 'FILE *'? + FILE f1; // match + // CHECK-MESSAGES: :[[@LINE+2]]:8: warning: 'f2' declared as type 'FILE'; did you mean 'FILE *'? + // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: expression has suspicious type 'FILE' + FILE f2 = *f; + // CHECK-MESSAGES: :[[@LINE+1]]:15: warning: 'f3' declared as type 'FILE'; did you mean 'FILE *'? + struct FILE f3; + // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: expression has suspicious type 'FILE' + (void)sizeof(*f); + (void)sizeof(FILE); + // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: expression has suspicious type 'FILE' + g(*f); + + pthread_mutex_t m; // ok + h(m); // ok + + pthread_cond_t c; // ok + i(c); // ok + + pthread_mutex_t *m1 = &m; // ok + // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: expression has suspicious type 'pthread_mutex_t' + h(*m1); +} Index: test/clang-tidy/misc-non-copyable-objects.cpp =================================================================== --- test/clang-tidy/misc-non-copyable-objects.cpp +++ test/clang-tidy/misc-non-copyable-objects.cpp @@ -0,0 +1,26 @@ +// RUN: %python %S/check_clang_tidy.py %s misc-non-copyable-objects %t + +namespace std { +typedef struct FILE {} FILE; +} +using namespace std; + +// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: 'f' declared as type 'FILE'; did you mean 'FILE *'? +void g(std::FILE f); + +struct S { + // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: 'f' declared as type 'FILE'; did you mean 'FILE *'? + ::FILE f; +}; + +void func(FILE *f) { + // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: 'f1' declared as type 'FILE'; did you mean 'FILE *'? + std::FILE f1; // match + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: 'f2' declared as type 'FILE'; did you mean 'FILE *'? + // CHECK-MESSAGES: :[[@LINE+1]]:15: warning: expression has suspicious type 'FILE' + ::FILE f2 = *f; // match, match + // CHECK-MESSAGES: :[[@LINE+1]]:15: warning: 'f3' declared as type 'FILE'; did you mean 'FILE *'? + struct FILE f3; // match + // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: expression has suspicious type 'FILE' + (void)sizeof(*f); // match +}