Merge pull request #331

Private/sync to llvm svn 362662

Author: vladimirlaz

clang-tools-extra/clang-apply-replacements/CMakeLists.txt

@@ -10,7 +10,7 @@ add_clang_library(clangApplyReplacements
   clangBasic
   clangRewrite
   clangToolingCore
-  clangToolingRefactor
+  clangToolingRefactoring
   )
 
 include_directories(

clang-tools-extra/clang-apply-replacements/tool/CMakeLists.txt

@@ -12,7 +12,7 @@ target_link_libraries(clang-apply-replacements
   clangFormat
   clangRewrite
   clangToolingCore
-  clangToolingRefactor
+  clangToolingRefactoring
   )
 
 install(TARGETS clang-apply-replacements

clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp

@@ -16,6 +16,18 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 
+UnhandledSelfAssignmentCheck::UnhandledSelfAssignmentCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      WarnOnlyIfThisHasSuspiciousField(
+          Options.get("WarnOnlyIfThisHasSuspiciousField", true)) {}
+
+void UnhandledSelfAssignmentCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WarnOnlyIfThisHasSuspiciousField",
+                WarnOnlyIfThisHasSuspiciousField);
+}
+
 void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
@@ -61,29 +73,32 @@ void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
       cxxMethodDecl(unless(hasDescendant(cxxMemberCallExpr(callee(cxxMethodDecl(
           hasName("operator="), ofClass(equalsBoundNode("class"))))))));
 
-  // Matcher for standard smart pointers.
-  const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType(
-      recordType(hasDeclaration(classTemplateSpecializationDecl(
-          hasAnyName("::std::shared_ptr", "::std::unique_ptr",
-                     "::std::weak_ptr", "::std::auto_ptr"),
-          templateArgumentCountIs(1))))));
-
-  // We will warn only if the class has a pointer or a C array field which
-  // probably causes a problem during self-assignment (e.g. first resetting the
-  // pointer member, then trying to access the object pointed by the pointer, or
-  // memcpy overlapping arrays).
-  const auto ThisHasSuspiciousField = cxxMethodDecl(ofClass(cxxRecordDecl(
-      has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
-                          hasType(arrayType())))))));
-
-  Finder->addMatcher(
-      cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")),
-                    isCopyAssignmentOperator(), IsUserDefined,
-                    HasReferenceParam, HasNoSelfCheck,
-                    unless(HasNonTemplateSelfCopy), unless(HasTemplateSelfCopy),
-                    HasNoNestedSelfAssign, ThisHasSuspiciousField)
-          .bind("copyAssignmentOperator"),
-      this);
+  DeclarationMatcher AdditionalMatcher = cxxMethodDecl();
+  if (WarnOnlyIfThisHasSuspiciousField) {
+    // Matcher for standard smart pointers.
+    const auto SmartPointerType = qualType(hasUnqualifiedDesugaredType(
+        recordType(hasDeclaration(classTemplateSpecializationDecl(
+            hasAnyName("::std::shared_ptr", "::std::unique_ptr",
+                       "::std::weak_ptr", "::std::auto_ptr"),
+            templateArgumentCountIs(1))))));
+
+    // We will warn only if the class has a pointer or a C array field which
+    // probably causes a problem during self-assignment (e.g. first resetting
+    // the pointer member, then trying to access the object pointed by the
+    // pointer, or memcpy overlapping arrays).
+    AdditionalMatcher = cxxMethodDecl(ofClass(cxxRecordDecl(
+        has(fieldDecl(anyOf(hasType(pointerType()), hasType(SmartPointerType),
+                            hasType(arrayType())))))));
+  }
+
+  Finder->addMatcher(cxxMethodDecl(ofClass(cxxRecordDecl().bind("class")),
+                                   isCopyAssignmentOperator(), IsUserDefined,
+                                   HasReferenceParam, HasNoSelfCheck,
+                                   unless(HasNonTemplateSelfCopy),
+                                   unless(HasTemplateSelfCopy),
+                                   HasNoNestedSelfAssign, AdditionalMatcher)
+                         .bind("copyAssignmentOperator"),
+                     this);
 }
 
 void UnhandledSelfAssignmentCheck::check(

clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.h

@@ -23,10 +23,14 @@ namespace bugprone {
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unhandled-self-assignment.html
 class UnhandledSelfAssignmentCheck : public ClangTidyCheck {
 public:
-  UnhandledSelfAssignmentCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UnhandledSelfAssignmentCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool WarnOnlyIfThisHasSuspiciousField;
 };
 
 } // namespace bugprone

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "../bugprone/UnhandledSelfAssignmentCheck.h"
 #include "../google/UnnamedNamespaceInHeaderCheck.h"
 #include "../misc/NewDeleteOverloadsCheck.h"
 #include "../misc/NonCopyableObjects.h"
@@ -49,6 +50,8 @@ class CERTModule : public ClangTidyModule {
     // OOP
     CheckFactories.registerCheck<performance::MoveConstructorInitCheck>(
         "cert-oop11-cpp");
+    CheckFactories.registerCheck<bugprone::UnhandledSelfAssignmentCheck>(
+        "cert-oop54-cpp");
     // ERR
     CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>(
         "cert-err09-cpp");
@@ -85,6 +88,7 @@ class CERTModule : public ClangTidyModule {
     ClangTidyOptions Options;
     ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
     Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
+    Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "0";
     return Options;
   }
 };

clang-tools-extra/clang-tidy/cert/CMakeLists.txt

@@ -20,6 +20,7 @@ add_clang_library(clangTidyCERTModule
   clangBasic
   clangLex
   clangTidy
+  clangTidyBugproneModule
   clangTidyGoogleModule
   clangTidyMiscModule
   clangTidyPerformanceModule

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp

@@ -250,7 +250,8 @@ void fixInitializerList(const ASTContext &Context, DiagnosticBuilder &Diag,
 ProTypeMemberInitCheck::ProTypeMemberInitCheck(StringRef Name,
                                                ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IgnoreArrays(Options.get("IgnoreArrays", false)) {}
+      IgnoreArrays(Options.get("IgnoreArrays", false)),
+      UseAssignment(Options.getLocalOrGlobal("UseAssignment", false)) {}
 
 void ProTypeMemberInitCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
@@ -314,6 +315,7 @@ void ProTypeMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
 
 void ProTypeMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreArrays", IgnoreArrays);
+  Options.store(Opts, "UseAssignment", UseAssignment);
 }
 
 // FIXME: Copied from clang/lib/Sema/SemaDeclCXX.cpp.
@@ -338,6 +340,56 @@ static bool isEmpty(ASTContext &Context, const QualType &Type) {
   return isIncompleteOrZeroLengthArrayType(Context, Type);
 }
 
+static const char *getInitializer(QualType QT, bool UseAssignment) {
+  const char *DefaultInitializer = "{}";
+  if (!UseAssignment)
+    return DefaultInitializer;
+
+  if (QT->isPointerType())
+    return " = nullptr";
+
+  const BuiltinType *BT =
+      dyn_cast<BuiltinType>(QT.getCanonicalType().getTypePtr());
+  if (!BT)
+    return DefaultInitializer;
+
+  switch (BT->getKind()) {
+  case BuiltinType::Bool:
+    return " = false";
+  case BuiltinType::Float:
+    return " = 0.0F";
+  case BuiltinType::Double:
+    return " = 0.0";
+  case BuiltinType::LongDouble:
+    return " = 0.0L";
+  case BuiltinType::SChar:
+  case BuiltinType::Char_S:
+  case BuiltinType::WChar_S:
+  case BuiltinType::Char16:
+  case BuiltinType::Char32:
+  case BuiltinType::Short:
+  case BuiltinType::Int:
+    return " = 0";
+  case BuiltinType::UChar:
+  case BuiltinType::Char_U:
+  case BuiltinType::WChar_U:
+  case BuiltinType::UShort:
+  case BuiltinType::UInt:
+    return " = 0U";
+  case BuiltinType::Long:
+    return " = 0L";
+  case BuiltinType::ULong:
+    return " = 0UL";
+  case BuiltinType::LongLong:
+    return " = 0LL";
+  case BuiltinType::ULongLong:
+    return " = 0ULL";
+
+  default:
+    return DefaultInitializer;
+  }
+}
+
 void ProTypeMemberInitCheck::checkMissingMemberInitializer(
     ASTContext &Context, const CXXRecordDecl &ClassDecl,
     const CXXConstructorDecl *Ctor) {
@@ -420,7 +472,7 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
     for (const FieldDecl *Field : FieldsToFix) {
       Diag << FixItHint::CreateInsertion(
           getLocationForEndOfToken(Context, Field->getSourceRange().getEnd()),
-          "{}");
+          getInitializer(Field->getType(), UseAssignment));
     }
   } else if (Ctor) {
     // Otherwise, rewrite the constructor's initializer list.

clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h

@@ -64,6 +64,11 @@ class ProTypeMemberInitCheck : public ClangTidyCheck {
 
   // Whether arrays need to be initialized or not. Default is false.
   bool IgnoreArrays;
+
+  // Whether fix-its for initialization of fundamental type use assignment
+  // instead of brace initalization. Only effective in C++11 mode. Default is
+  // false.
+  bool UseAssignment;
 };
 
 } // namespace cppcoreguidelines

clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp

@@ -0,0 +1,130 @@
+//===--- AvoidNSObjectNewCheck.cpp - clang-tidy ---------------------------===//
+//
+// 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 "AvoidNSObjectNewCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <map>
+#include <string>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace google {
+namespace objc {
+
+static bool isMessageExpressionInsideMacro(const ObjCMessageExpr *Expr) {
+  SourceLocation ReceiverLocation = Expr->getReceiverRange().getBegin();
+  if (ReceiverLocation.isMacroID())
+    return true;
+
+  SourceLocation SelectorLocation = Expr->getSelectorStartLoc();
+  if (SelectorLocation.isMacroID())
+    return true;
+
+  return false;
+}
+
+// Walk up the class hierarchy looking for an -init method, returning true
+// if one is found and has not been marked unavailable.
+static bool isInitMethodAvailable(const ObjCInterfaceDecl *ClassDecl) {
+  while (ClassDecl != nullptr) {
+    for (const auto *MethodDecl : ClassDecl->instance_methods()) {
+      if (MethodDecl->getSelector().getAsString() == "init")
+        return !MethodDecl->isUnavailable();
+    }
+    ClassDecl = ClassDecl->getSuperClass();
+  }
+
+  // No -init method found in the class hierarchy. This should occur only rarely
+  // in Objective-C code, and only really applies to classes not derived from
+  // NSObject.
+  return false;
+}
+
+// Returns the string for the Objective-C message receiver. Keeps any generics
+// included in the receiver class type, which are stripped if the class type is
+// used. While the generics arguments will not make any difference to the
+// returned code at this time, the style guide allows them and they should be
+// left in any fix-it hint.
+static StringRef getReceiverString(SourceRange ReceiverRange,
+                                   const SourceManager &SM,
+                                   const LangOptions &LangOpts) {
+  CharSourceRange CharRange = Lexer::makeFileCharRange(
+      CharSourceRange::getTokenRange(ReceiverRange), SM, LangOpts);
+  return Lexer::getSourceText(CharRange, SM, LangOpts);
+}
+
+static FixItHint getCallFixItHint(const ObjCMessageExpr *Expr,
+                                  const SourceManager &SM,
+                                  const LangOptions &LangOpts) {
+  // Check whether the messaged class has a known factory method to use instead
+  // of -init.
+  StringRef Receiver =
+      getReceiverString(Expr->getReceiverRange(), SM, LangOpts);
+  // Some classes should use standard factory methods instead of alloc/init.
+  std::map<StringRef, StringRef> ClassToFactoryMethodMap = {{"NSDate", "date"},
+                                                            {"NSNull", "null"}};
+  auto FoundClassFactory = ClassToFactoryMethodMap.find(Receiver);
+  if (FoundClassFactory != ClassToFactoryMethodMap.end()) {
+    StringRef ClassName = FoundClassFactory->first;
+    StringRef FactorySelector = FoundClassFactory->second;
+    std::string NewCall =
+        llvm::formatv("[{0} {1}]", ClassName, FactorySelector);
+    return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall);
+  }
+
+  if (isInitMethodAvailable(Expr->getReceiverInterface())) {
+    std::string NewCall = llvm::formatv("[[{0} alloc] init]", Receiver);
+    return FixItHint::CreateReplacement(Expr->getSourceRange(), NewCall);
+  }
+
+  return {}; // No known replacement available.
+}
+
+void AvoidNSObjectNewCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().ObjC)
+    return;
+
+  // Add two matchers, to catch calls to +new and implementations of +new.
+  Finder->addMatcher(
+      objcMessageExpr(isClassMessage(), hasSelector("new")).bind("new_call"),
+      this);
+  Finder->addMatcher(
+      objcMethodDecl(isClassMethod(), isDefinition(), hasName("new"))
+          .bind("new_override"),
+      this);
+}
+
+void AvoidNSObjectNewCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *CallExpr =
+          Result.Nodes.getNodeAs<ObjCMessageExpr>("new_call")) {
+    // Don't warn if the call expression originates from a macro expansion.
+    if (isMessageExpressionInsideMacro(CallExpr))
+      return;
+
+    diag(CallExpr->getExprLoc(), "do not create objects with +new")
+        << getCallFixItHint(CallExpr, *Result.SourceManager,
+                            Result.Context->getLangOpts());
+  }
+
+  if (const auto *DeclExpr =
+          Result.Nodes.getNodeAs<ObjCMethodDecl>("new_override")) {
+    diag(DeclExpr->getBeginLoc(), "classes should not override +new");
+  }
+}
+
+} // namespace objc
+} // namespace google
+} // namespace tidy
+} // namespace clang