Merge pull request #815 for LLVM and SPIRV-translator pulldown

Author: vladimirlaz

clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp

@@ -17,9 +17,24 @@ namespace clang {
 namespace tidy {
 namespace hicpp {
 
+SignedBitwiseCheck::SignedBitwiseCheck(StringRef Name,
+                                       ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IgnorePositiveIntegerLiterals(
+          Options.get("IgnorePositiveIntegerLiterals", false)) {}
+
+void SignedBitwiseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnorePositiveIntegerLiterals",
+                IgnorePositiveIntegerLiterals);
+}
+
 void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntegerOperand =
-      expr(ignoringImpCasts(hasType(isSignedInteger()))).bind("signed-operand");
+      (IgnorePositiveIntegerLiterals
+           ? expr(ignoringImpCasts(hasType(isSignedInteger())),
+                  unless(integerLiteral()))
+           : expr(ignoringImpCasts(hasType(isSignedInteger()))))
+          .bind("signed-operand");
 
   // The standard [bitmask.types] allows some integral types to be implemented
   // as signed types. Exclude these types from diagnosing for bitwise or(|) and

clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.h

@@ -22,10 +22,13 @@ namespace hicpp {
 /// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html
 class SignedBitwiseCheck : public ClangTidyCheck {
 public:
-  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Options) override;
+
+private:
+  bool IgnorePositiveIntegerLiterals;
 };
 
 } // namespace hicpp

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

@@ -131,6 +131,20 @@ static bool areEquivalentExpr(const Expr *Left, const Expr *Right) {
   case Stmt::BinaryOperatorClass:
     return cast<BinaryOperator>(Left)->getOpcode() ==
            cast<BinaryOperator>(Right)->getOpcode();
+  case Stmt::UnaryExprOrTypeTraitExprClass:
+    const auto *LeftUnaryExpr =
+        cast<UnaryExprOrTypeTraitExpr>(Left);
+    const auto *RightUnaryExpr =
+        cast<UnaryExprOrTypeTraitExpr>(Right);
+    if (LeftUnaryExpr->isArgumentType() && RightUnaryExpr->isArgumentType())
+      return LeftUnaryExpr->getArgumentType() ==
+             RightUnaryExpr->getArgumentType();
+    else if (!LeftUnaryExpr->isArgumentType() &&
+             !RightUnaryExpr->isArgumentType())
+      return areEquivalentExpr(LeftUnaryExpr->getArgumentExpr(),
+                               RightUnaryExpr->getArgumentExpr());
+
+    return false;
   }
 }
 
@@ -604,23 +618,62 @@ static bool retrieveConstExprFromBothSides(const BinaryOperator *&BinOp,
   return true;
 }
 
+static bool isSameRawIdentifierToken(const Token &T1, const Token &T2,
+                        const SourceManager &SM) {
+  if (T1.getKind() != T2.getKind())
+    return false;
+  if (T1.isNot(tok::raw_identifier))
+    return true;
+  if (T1.getLength() != T2.getLength())
+    return false;
+  return StringRef(SM.getCharacterData(T1.getLocation()), T1.getLength()) ==
+         StringRef(SM.getCharacterData(T2.getLocation()), T2.getLength());
+}
+
+bool isTokAtEndOfExpr(SourceRange ExprSR, Token T, const SourceManager &SM) {
+  return SM.getExpansionLoc(ExprSR.getEnd()) == T.getLocation();
+}
+
+/// Returns true if both LhsEpxr and RhsExpr are
+/// macro expressions and they are expanded
+/// from different macros.
 static bool areExprsFromDifferentMacros(const Expr *LhsExpr,
                                         const Expr *RhsExpr,
                                         const ASTContext *AstCtx) {
   if (!LhsExpr || !RhsExpr)
     return false;
-
-  SourceLocation LhsLoc = LhsExpr->getExprLoc();
-  SourceLocation RhsLoc = RhsExpr->getExprLoc();
-
-  if (!LhsLoc.isMacroID() || !RhsLoc.isMacroID())
+  SourceRange Lsr = LhsExpr->getSourceRange();
+  SourceRange Rsr = RhsExpr->getSourceRange();
+  if (!Lsr.getBegin().isMacroID() || !Rsr.getBegin().isMacroID())
     return false;
 
   const SourceManager &SM = AstCtx->getSourceManager();
   const LangOptions &LO = AstCtx->getLangOpts();
 
-  return !(Lexer::getImmediateMacroName(LhsLoc, SM, LO) ==
-          Lexer::getImmediateMacroName(RhsLoc, SM, LO));
+  std::pair<FileID, unsigned> LsrLocInfo =
+      SM.getDecomposedLoc(SM.getExpansionLoc(Lsr.getBegin()));
+  std::pair<FileID, unsigned> RsrLocInfo =
+      SM.getDecomposedLoc(SM.getExpansionLoc(Rsr.getBegin()));
+  const llvm::MemoryBuffer *MB = SM.getBuffer(LsrLocInfo.first);
+
+  const char *LTokenPos = MB->getBufferStart() + LsrLocInfo.second;
+  const char *RTokenPos = MB->getBufferStart() + RsrLocInfo.second;
+  Lexer LRawLex(SM.getLocForStartOfFile(LsrLocInfo.first), LO,
+                MB->getBufferStart(), LTokenPos, MB->getBufferEnd());
+  Lexer RRawLex(SM.getLocForStartOfFile(RsrLocInfo.first), LO,
+                MB->getBufferStart(), RTokenPos, MB->getBufferEnd());
+
+  Token LTok, RTok;
+  do { // Compare the expressions token-by-token.
+    LRawLex.LexFromRawLexer(LTok);
+    RRawLex.LexFromRawLexer(RTok);
+  } while (!LTok.is(tok::eof) && !RTok.is(tok::eof) &&
+           isSameRawIdentifierToken(LTok, RTok, SM) &&
+           !isTokAtEndOfExpr(Lsr, LTok, SM) &&
+           !isTokAtEndOfExpr(Rsr, RTok, SM));
+  return (!isTokAtEndOfExpr(Lsr, LTok, SM) ||
+          !isTokAtEndOfExpr(Rsr, RTok, SM)) ||
+         !isSameRawIdentifierToken(LTok, RTok, SM);
 }
 
 static bool areExprsMacroAndNonMacro(const Expr *&LhsExpr,

clang-tools-extra/clang-tidy/modernize/UseNodiscardCheck.cpp

@@ -100,7 +100,9 @@ void UseNodiscardCheck::registerMatchers(MatchFinder *Finder) {
       cxxMethodDecl(
           allOf(isConst(), isDefinitionOrInline(),
                 unless(anyOf(
-                    returns(voidType()), isNoReturn(), isOverloadedOperator(),
+                    returns(voidType()),
+                    returns(hasDeclaration(decl(hasAttr(clang::attr::WarnUnusedResult)))),
+                    isNoReturn(), isOverloadedOperator(),
                     isVariadic(), hasTemplateReturnType(),
                     hasClassMutableFields(), isConversionOperator(),
                     hasAttr(clang::attr::WarnUnusedResult),

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

@@ -11,6 +11,7 @@ add_clang_library(clangTidyPerformanceModule
   MoveConstructorInitCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   PerformanceTidyModule.cpp
+  TriviallyDestructibleCheck.cpp
   TypePromotionInMathFnCheck.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp

clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp

@@ -18,6 +18,7 @@
 #include "MoveConstArgCheck.h"
 #include "MoveConstructorInitCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
+#include "TriviallyDestructibleCheck.h"
 #include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
@@ -47,6 +48,8 @@ class PerformanceModule : public ClangTidyModule {
         "performance-move-constructor-init");
     CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
         "performance-noexcept-move-constructor");
+    CheckFactories.registerCheck<TriviallyDestructibleCheck>(
+        "performance-trivially-destructible");
     CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
         "performance-type-promotion-in-math-fn");
     CheckFactories.registerCheck<UnnecessaryCopyInitialization>(

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp

@@ -0,0 +1,82 @@
+//===--- TriviallyDestructibleCheck.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 "TriviallyDestructibleCheck.h"
+#include "../utils/LexerUtils.h"
+#include "../utils/Matchers.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+using namespace clang::ast_matchers::internal;
+using namespace clang::tidy::matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+namespace {
+
+AST_MATCHER(Decl, isFirstDecl) { return Node.isFirstDecl(); }
+
+AST_MATCHER_P(CXXRecordDecl, hasBase, Matcher<QualType>, InnerMatcher) {
+  for (const CXXBaseSpecifier &BaseSpec : Node.bases()) {
+    QualType BaseType = BaseSpec.getType();
+    if (InnerMatcher.matches(BaseType, Finder, Builder))
+      return true;
+  }
+  return false;
+}
+
+} // namespace
+
+void TriviallyDestructibleCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  Finder->addMatcher(
+      cxxDestructorDecl(
+          isDefaulted(),
+          unless(anyOf(isFirstDecl(), isVirtual(),
+                       ofClass(cxxRecordDecl(
+                           anyOf(hasBase(unless(isTriviallyDestructible())),
+                                 has(fieldDecl(unless(
+                                     hasType(isTriviallyDestructible()))))))))))
+          .bind("decl"),
+      this);
+}
+
+void TriviallyDestructibleCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<CXXDestructorDecl>("decl");
+
+  // Get locations of both first and out-of-line declarations.
+  SourceManager &SM = *Result.SourceManager;
+  const auto *FirstDecl = cast<CXXMethodDecl>(MatchedDecl->getFirstDecl());
+  const SourceLocation FirstDeclEnd = utils::lexer::findNextTerminator(
+      FirstDecl->getEndLoc(), SM, getLangOpts());
+  const CharSourceRange SecondDeclRange = CharSourceRange::getTokenRange(
+      MatchedDecl->getBeginLoc(),
+      utils::lexer::findNextTerminator(MatchedDecl->getEndLoc(), SM,
+                                       getLangOpts()));
+  if (FirstDeclEnd.isInvalid() || SecondDeclRange.isInvalid())
+    return;
+
+  // Report diagnostic.
+  diag(FirstDecl->getLocation(),
+       "class %0 can be made trivially destructible by defaulting the "
+       "destructor on its first declaration")
+      << FirstDecl->getParent()
+      << FixItHint::CreateInsertion(FirstDeclEnd, " = default")
+      << FixItHint::CreateRemoval(SecondDeclRange);
+  diag(MatchedDecl->getLocation(), "destructor definition is here",
+       DiagnosticIDs::Note);
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.h

@@ -0,0 +1,40 @@
+//===--- TriviallyDestructibleCheck.h - clang-tidy --------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TRIVIALLYDESTRUCTIBLECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TRIVIALLYDESTRUCTIBLECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// A check that finds classes that would be trivial if not for the defaulted
+/// destructors declared out-of-line:
+/// struct A: TrivialClass {
+///   ~A();
+///   TrivialClass trivial_fields;
+/// };
+/// A::~A() = default;
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-trivially-destructible.html
+class TriviallyDestructibleCheck : public ClangTidyCheck {
+public:
+  TriviallyDestructibleCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_TRIVIALLYDESTRUCTIBLECHECK_H

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

@@ -15,12 +15,14 @@ add_clang_library(clangTidyReadabilityModule
   InconsistentDeclarationParameterNameCheck.cpp
   IsolateDeclarationCheck.cpp
   MagicNumbersCheck.cpp
+  MakeMemberFunctionConstCheck.cpp
   MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
   ReadabilityTidyModule.cpp
+  RedundantAccessSpecifiersCheck.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp

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

@@ -691,10 +691,11 @@ static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
   if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
     return;
 
-  if (!Failure.ShouldFix)
+  if (!Failure.ShouldFix())
     return;
 
-  Failure.ShouldFix = utils::rangeCanBeFixed(Range, SourceMgr);
+  if (!utils::rangeCanBeFixed(Range, SourceMgr))
+    Failure.FixStatus = IdentifierNamingCheck::ShouldFixStatus::InsideMacro;
 }
 
 /// Convenience method when the usage to be added is a NamedDecl
@@ -873,6 +874,16 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {
           DeclarationNameInfo(Decl->getDeclName(), Decl->getLocation())
               .getSourceRange();
 
+      const IdentifierTable &Idents = Decl->getASTContext().Idents;
+      auto CheckNewIdentifier = Idents.find(Fixup);
+      if (CheckNewIdentifier != Idents.end()) {
+        const IdentifierInfo *Ident = CheckNewIdentifier->second;
+        if (Ident->isKeyword(getLangOpts()))
+          Failure.FixStatus = ShouldFixStatus::ConflictsWithKeyword;
+        else if (Ident->hasMacroDefinition())
+          Failure.FixStatus = ShouldFixStatus::ConflictsWithMacroDefinition;
+      }
+
       Failure.Fixup = std::move(Fixup);
       Failure.KindName = std::move(KindName);
       addUsage(NamingCheckFailures, Decl, Range);
@@ -935,24 +946,35 @@ void IdentifierNamingCheck::onEndOfTranslationUnit() {
     if (Failure.KindName.empty())
       continue;
 
-    if (Failure.ShouldFix) {
-      auto Diag = diag(Decl.first, "invalid case style for %0 '%1'")
-                  << Failure.KindName << Decl.second;
-
-      for (const auto &Loc : Failure.RawUsageLocs) {
-        // We assume that the identifier name is made of one token only. This is
-        // always the case as we ignore usages in macros that could build
-        // identifier names by combining multiple tokens.
-        //
-        // For destructors, we alread take care of it by remembering the
-        // location of the start of the identifier and not the start of the
-        // tilde.
-        //
-        // Other multi-token identifiers, such as operators are not checked at
-        // all.
-        Diag << FixItHint::CreateReplacement(
-            SourceRange(SourceLocation::getFromRawEncoding(Loc)),
-            Failure.Fixup);
+    if (Failure.ShouldNotify()) {
+      auto Diag =
+          diag(Decl.first,
+               "invalid case style for %0 '%1'%select{|" // Case 0 is empty on
+                                                         // purpose, because we
+                                                         // intent to provide a
+                                                         // fix
+               "; cannot be fixed because '%3' would conflict with a keyword|"
+               "; cannot be fixed because '%3' would conflict with a macro "
+               "definition}2")
+          << Failure.KindName << Decl.second
+          << static_cast<int>(Failure.FixStatus) << Failure.Fixup;
+
+      if (Failure.ShouldFix()) {
+        for (const auto &Loc : Failure.RawUsageLocs) {
+          // We assume that the identifier name is made of one token only. This
+          // is always the case as we ignore usages in macros that could build
+          // identifier names by combining multiple tokens.
+          //
+          // For destructors, we already take care of it by remembering the
+          // location of the start of the identifier and not the start of the
+          // tilde.
+          //
+          // Other multi-token identifiers, such as operators are not checked at
+          // all.
+          Diag << FixItHint::CreateReplacement(
+              SourceRange(SourceLocation::getFromRawEncoding(Loc)),
+              Failure.Fixup);
+        }
       }
     }
   }