Merge from 'master' to 'sycl-web' (#6)

  CONFLICT (content): Merge conflict in clang/lib/CodeGen/CGClass.cpp

Author: Erich Keane

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

@@ -77,6 +77,10 @@ void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
 }
 
 void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
+  // FIXME:
+  // Some of the checks should not match in template code to avoid false
+  // positives if sizeof is applied on template argument.
+
   const auto IntegerExpr = ignoringParenImpCasts(integerLiteral());
   const auto ConstantExpr = expr(ignoringParenImpCasts(
       anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
@@ -132,6 +136,7 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
                      this);
 
   // Detect sizeof(ptr) where ptr points to an aggregate (i.e. sizeof(&S)).
+  // Do not find it if RHS of a 'sizeof(arr) / sizeof(arr[0])' expression.
   const auto ArrayExpr = expr(ignoringParenImpCasts(
       expr(hasType(qualType(hasCanonicalType(arrayType()))))));
   const auto ArrayCastExpr = expr(anyOf(
@@ -151,13 +156,31 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
       hasType(qualType(hasCanonicalType(PointerToStructType))),
       unless(cxxThisExpr()))));
 
-  Finder->addMatcher(
-      expr(anyOf(sizeOfExpr(has(expr(ignoringParenImpCasts(
-               anyOf(ArrayCastExpr, PointerToArrayExpr, StructAddrOfExpr,
-                     PointerToStructExpr))))),
-                            sizeOfExpr(has(PointerToStructType))))
-          .bind("sizeof-pointer-to-aggregate"),
-      this);
+  const auto ArrayOfPointersExpr = expr(ignoringParenImpCasts(expr(hasType(
+      qualType(hasCanonicalType(arrayType(hasElementType(pointerType()))
+                                    .bind("type-of-array-of-pointers")))))));
+  const auto ArrayOfSamePointersExpr =
+      expr(ignoringParenImpCasts(expr(hasType(qualType(hasCanonicalType(
+          arrayType(equalsBoundNode("type-of-array-of-pointers"))))))));
+  const auto ZeroLiteral =
+      expr(ignoringParenImpCasts(integerLiteral(equals(0))));
+  const auto ArrayOfSamePointersZeroSubscriptExpr =
+      expr(ignoringParenImpCasts(arraySubscriptExpr(
+          hasBase(ArrayOfSamePointersExpr), hasIndex(ZeroLiteral))));
+  const auto ArrayLengthExprDenom =
+      expr(hasParent(expr(ignoringParenImpCasts(
+               binaryOperator(hasOperatorName("/"),
+                              hasLHS(expr(ignoringParenImpCasts(expr(
+                                  sizeOfExpr(has(ArrayOfPointersExpr)))))))))),
+           sizeOfExpr(has(ArrayOfSamePointersZeroSubscriptExpr)));
+
+  Finder->addMatcher(expr(anyOf(sizeOfExpr(has(expr(ignoringParenImpCasts(anyOf(
+                                    ArrayCastExpr, PointerToArrayExpr,
+                                    StructAddrOfExpr, PointerToStructExpr))))),
+                                sizeOfExpr(has(PointerToStructType))),
+                          unless(ArrayLengthExprDenom))
+                         .bind("sizeof-pointer-to-aggregate"),
+                     this);
 
   // Detect expression like: sizeof(epxr) <= k for a suspicious constant 'k'.
   if (WarnOnSizeOfCompareToConstant) {
@@ -178,6 +201,12 @@ void SizeofExpressionCheck::registerMatchers(MatchFinder *Finder) {
                      this);
 
   // Detect sizeof(...) /sizeof(...));
+  // FIXME:
+  // Re-evaluate what cases to handle by the checker.
+  // Probably any sizeof(A)/sizeof(B) should be error if
+  // 'A' is not an array (type) and 'B' the (type of the) first element of it.
+  // Except if 'A' and 'B' are non-pointers, then use the existing size division
+  // rule.
   const auto ElemType =
       arrayType(hasElementType(recordType().bind("elem-type")));
   const auto ElemPtrType = pointerType(pointee(type().bind("elem-ptr-type")));

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

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "StringConstructorCheck.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/FixIt.h"
@@ -21,17 +22,36 @@ namespace {
 AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
   return Node.getValue().getZExtValue() > N;
 }
+
+const char DefaultStringNames[] =
+    "::std::basic_string;::std::basic_string_view";
+
+static std::vector<StringRef>
+removeNamespaces(const std::vector<std::string> &Names) {
+  std::vector<StringRef> Result;
+  Result.reserve(Names.size());
+  for (StringRef Name : Names) {
+    std::string::size_type ColonPos = Name.rfind(':');
+    Result.push_back(
+        Name.substr(ColonPos == std::string::npos ? 0 : ColonPos + 1));
+  }
+  return Result;
+}
+
 } // namespace
 
 StringConstructorCheck::StringConstructorCheck(StringRef Name,
                                                ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       WarnOnLargeLength(Options.get("WarnOnLargeLength", true)),
-      LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)) {}
+      LargeLengthThreshold(Options.get("LargeLengthThreshold", 0x800000)),
+      StringNames(utils::options::parseStringList(
+          Options.get("StringNames", DefaultStringNames))) {}
 
 void StringConstructorCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnLargeLength", WarnOnLargeLength);
   Options.store(Opts, "LargeLengthThreshold", LargeLengthThreshold);
+  Options.store(Opts, "StringNames", DefaultStringNames);
 }
 
 void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
@@ -80,7 +100,8 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
   // parameters. [i.e. string (const char* s, size_t n);]
   Finder->addMatcher(
       cxxConstructExpr(
-          hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+          hasDeclaration(cxxConstructorDecl(ofClass(
+              cxxRecordDecl(hasAnyName(removeNamespaces(StringNames)))))),
           hasArgument(0, hasType(CharPtrType)),
           hasArgument(1, hasType(isInteger())),
           anyOf(
@@ -100,11 +121,17 @@ void StringConstructorCheck::registerMatchers(MatchFinder *Finder) {
   // Check the literal string constructor with char pointer.
   // [i.e. string (const char* s);]
   Finder->addMatcher(
-    traverse(TK_AsIs,
-      cxxConstructExpr(hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
-                       hasArgument(0, expr().bind("from-ptr")),
-                       hasArgument(1, unless(hasType(isInteger()))))
-          .bind("constructor")),
+      traverse(TK_AsIs,
+               cxxConstructExpr(
+                   hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl(
+                       hasAnyName(removeNamespaces(StringNames)))))),
+                   hasArgument(0, expr().bind("from-ptr")),
+                   // do not match std::string(ptr, int)
+                   // match std::string(ptr, alloc)
+                   // match std::string(ptr)
+                   anyOf(hasArgument(1, unless(hasType(isInteger()))),
+                         argumentCountIs(1)))
+                   .bind("constructor")),
       this);
 }
 

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

@@ -32,6 +32,7 @@ class StringConstructorCheck : public ClangTidyCheck {
 private:
   const bool WarnOnLargeLength;
   const unsigned int LargeLengthThreshold;
+  std::vector<std::string> StringNames;
 };
 
 } // namespace bugprone

clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp

@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/FixIt.h"
 #include "llvm/ADT/SmallVector.h"
 
@@ -19,10 +20,30 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
-static const char ReturnStr[] = "return";
-static const char ContinueStr[] = "continue";
-static const char BreakStr[] = "break";
-static const char ThrowStr[] = "throw";
+namespace {
+
+class PPConditionalCollector : public PPCallbacks {
+public:
+  PPConditionalCollector(
+      ElseAfterReturnCheck::ConditionalBranchMap &Collections,
+      const SourceManager &SM)
+      : Collections(Collections), SM(SM) {}
+  void Endif(SourceLocation Loc, SourceLocation IfLoc) override {
+    if (!SM.isWrittenInSameFile(Loc, IfLoc))
+      return;
+    SmallVectorImpl<SourceRange> &Collection = Collections[SM.getFileID(Loc)];
+    assert(Collection.empty() || Collection.back().getEnd() < Loc);
+    Collection.emplace_back(IfLoc, Loc);
+  }
+
+private:
+  ElseAfterReturnCheck::ConditionalBranchMap &Collections;
+  const SourceManager &SM;
+};
+
+} // namespace
+
+static const char InterruptingStr[] = "interrupting";
 static const char WarningMessage[] = "do not use 'else' after '%0'";
 static const char WarnOnUnfixableStr[] = "WarnOnUnfixable";
 static const char WarnOnConditionVariablesStr[] = "WarnOnConditionVariables";
@@ -140,11 +161,18 @@ void ElseAfterReturnCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, WarnOnConditionVariablesStr, WarnOnConditionVariables);
 }
 
+void ElseAfterReturnCheck::registerPPCallbacks(const SourceManager &SM,
+                                               Preprocessor *PP,
+                                               Preprocessor *ModuleExpanderPP) {
+  PP->addPPCallbacks(
+      std::make_unique<PPConditionalCollector>(this->PPConditionals, SM));
+}
+
 void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
-  const auto InterruptsControlFlow =
-      stmt(anyOf(returnStmt().bind(ReturnStr), continueStmt().bind(ContinueStr),
-                 breakStmt().bind(BreakStr),
-                 expr(ignoringImplicit(cxxThrowExpr().bind(ThrowStr)))));
+  const auto InterruptsControlFlow = stmt(anyOf(
+      returnStmt().bind(InterruptingStr), continueStmt().bind(InterruptingStr),
+      breakStmt().bind(InterruptingStr),
+      expr(ignoringImplicit(cxxThrowExpr().bind(InterruptingStr)))));
   Finder->addMatcher(
       compoundStmt(
           forEach(ifStmt(unless(isConstexpr()),
@@ -157,21 +185,72 @@ void ElseAfterReturnCheck::registerMatchers(MatchFinder *Finder) {
       this);
 }
 
+static bool hasPreprocessorBranchEndBetweenLocations(
+    const ElseAfterReturnCheck::ConditionalBranchMap &ConditionalBranchMap,
+    const SourceManager &SM, SourceLocation StartLoc, SourceLocation EndLoc) {
+
+  SourceLocation ExpandedStartLoc = SM.getExpansionLoc(StartLoc);
+  SourceLocation ExpandedEndLoc = SM.getExpansionLoc(EndLoc);
+  if (!SM.isWrittenInSameFile(ExpandedStartLoc, ExpandedEndLoc))
+    return false;
+
+  // StartLoc and EndLoc expand to the same macro.
+  if (ExpandedStartLoc == ExpandedEndLoc)
+    return false;
+
+  assert(ExpandedStartLoc < ExpandedEndLoc);
+
+  auto Iter = ConditionalBranchMap.find(SM.getFileID(ExpandedEndLoc));
+
+  if (Iter == ConditionalBranchMap.end() || Iter->getSecond().empty())
+    return false;
+
+  const SmallVectorImpl<SourceRange> &ConditionalBranches = Iter->getSecond();
+
+  assert(llvm::is_sorted(ConditionalBranches,
+                         [](const SourceRange &LHS, const SourceRange &RHS) {
+                           return LHS.getEnd() < RHS.getEnd();
+                         }));
+
+  // First conditional block that ends after ExpandedStartLoc.
+  const auto *Begin =
+      llvm::lower_bound(ConditionalBranches, ExpandedStartLoc,
+                        [](const SourceRange &LHS, const SourceLocation &RHS) {
+                          return LHS.getEnd() < RHS;
+                        });
+  const auto *End = ConditionalBranches.end();
+  for (; Begin != End && Begin->getEnd() < ExpandedEndLoc; ++Begin)
+    if (Begin->getBegin() < ExpandedStartLoc)
+      return true;
+  return false;
+}
+
+static StringRef getControlFlowString(const Stmt &Stmt) {
+  if (isa<ReturnStmt>(Stmt))
+    return "return";
+  if (isa<ContinueStmt>(Stmt))
+    return "continue";
+  if (isa<BreakStmt>(Stmt))
+    return "break";
+  if (isa<CXXThrowExpr>(Stmt))
+    return "throw";
+  llvm_unreachable("Unknown control flow interruptor");
+}
+
 void ElseAfterReturnCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
   const auto *Else = Result.Nodes.getNodeAs<Stmt>("else");
   const auto *OuterScope = Result.Nodes.getNodeAs<CompoundStmt>("cs");
-
-  bool IsLastInScope = OuterScope->body_back() == If;
+  const auto *Interrupt = Result.Nodes.getNodeAs<Stmt>(InterruptingStr);
   SourceLocation ElseLoc = If->getElseLoc();
 
-  auto ControlFlowInterruptor = [&]() -> llvm::StringRef {
-    for (llvm::StringRef BindingName :
-         {ReturnStr, ContinueStr, BreakStr, ThrowStr})
-      if (Result.Nodes.getNodeAs<Stmt>(BindingName))
-        return BindingName;
-    return {};
-  }();
+  if (hasPreprocessorBranchEndBetweenLocations(
+          PPConditionals, *Result.SourceManager, Interrupt->getBeginLoc(),
+          ElseLoc))
+    return;
+
+  bool IsLastInScope = OuterScope->body_back() == If;
+  StringRef ControlFlowInterruptor = getControlFlowString(*Interrupt);
 
   if (!IsLastInScope && containsDeclInScope(Else)) {
     if (WarnOnUnfixable) {

clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.h

@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_ELSEAFTERRETURNCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseMap.h"
 
 namespace clang {
 namespace tidy {
@@ -23,12 +24,18 @@ class ElseAfterReturnCheck : public ClangTidyCheck {
   ElseAfterReturnCheck(StringRef Name, ClangTidyContext *Context);
 
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
+  using ConditionalBranchMap =
+      llvm::DenseMap<FileID, SmallVector<SourceRange, 1>>;
+
 private:
   const bool WarnOnUnfixable;
   const bool WarnOnConditionVariables;
+  ConditionalBranchMap PPConditionals;
 };
 
 } // namespace readability

clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp

@@ -18,7 +18,8 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
-const char DefaultStringNames[] = "::std::basic_string";
+const char DefaultStringNames[] =
+    "::std::basic_string_view;::std::basic_string";
 
 static ast_matchers::internal::Matcher<NamedDecl>
 hasAnyNameStdString(std::vector<std::string> Names) {