LLVM and SPIRV-LLVM-Translator pulldown (WW37)

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

@@ -38,6 +38,7 @@
 #include "NotNullTerminatedResultCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
+#include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
@@ -119,6 +120,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-move-forwarding-reference");
     CheckFactories.registerCheck<MultipleStatementMacroCheck>(
         "bugprone-multiple-statement-macro");
+    CheckFactories.registerCheck<RedundantBranchConditionCheck>(
+        "bugprone-redundant-branch-condition");
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
         "bugprone-narrowing-conversions");
     CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");

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

@@ -33,6 +33,7 @@ add_clang_library(clangTidyBugproneModule
   NotNullTerminatedResultCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp
+  RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp
   SignedCharMisuseCheck.cpp
   SizeofContainerCheck.cpp

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

@@ -0,0 +1,153 @@
+//===--- RedundantBranchConditionCheck.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 "RedundantBranchConditionCheck.h"
+#include "../utils/Aliasing.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+using clang::tidy::utils::hasPtrOrReferenceInFunc;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static const char CondVarStr[] = "cond_var";
+static const char OuterIfStr[] = "outer_if";
+static const char InnerIfStr[] = "inner_if";
+static const char FuncStr[] = "func";
+
+/// Returns whether `Var` is changed in `S` before `NextS`.
+static bool isChangedBefore(const Stmt *S, const Stmt *NextS,
+                            const VarDecl *Var, ASTContext *Context) {
+  ExprMutationAnalyzer MutAn(*S, *Context);
+  const auto &SM = Context->getSourceManager();
+  const Stmt *MutS = MutAn.findMutation(Var);
+  return MutS &&
+         SM.isBeforeInTranslationUnit(MutS->getEndLoc(), NextS->getBeginLoc());
+}
+
+void RedundantBranchConditionCheck::registerMatchers(MatchFinder *Finder) {
+  const auto ImmutableVar =
+      varDecl(anyOf(parmVarDecl(), hasLocalStorage()), hasType(isInteger()),
+              unless(hasType(isVolatileQualified())))
+          .bind(CondVarStr);
+  Finder->addMatcher(
+      ifStmt(
+          hasCondition(ignoringParenImpCasts(anyOf(
+              declRefExpr(hasDeclaration(ImmutableVar)),
+              binaryOperator(hasOperatorName("&&"),
+                             hasEitherOperand(ignoringParenImpCasts(declRefExpr(
+                                 hasDeclaration(ImmutableVar)))))))),
+          hasThen(hasDescendant(
+              ifStmt(hasCondition(ignoringParenImpCasts(
+                         anyOf(declRefExpr(hasDeclaration(
+                                   varDecl(equalsBoundNode(CondVarStr)))),
+                               binaryOperator(
+                                   hasAnyOperatorName("&&", "||"),
+                                   hasEitherOperand(ignoringParenImpCasts(
+                                       declRefExpr(hasDeclaration(varDecl(
+                                           equalsBoundNode(CondVarStr)))))))))))
+                  .bind(InnerIfStr))),
+          forFunction(functionDecl().bind(FuncStr)))
+          .bind(OuterIfStr),
+      this);
+  // FIXME: Handle longer conjunctive and disjunctive clauses.
+}
+
+void RedundantBranchConditionCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *OuterIf = Result.Nodes.getNodeAs<IfStmt>(OuterIfStr);
+  const auto *InnerIf = Result.Nodes.getNodeAs<IfStmt>(InnerIfStr);
+  const auto *CondVar = Result.Nodes.getNodeAs<VarDecl>(CondVarStr);
+  const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>(FuncStr);
+
+  // If the variable has an alias then it can be changed by that alias as well.
+  // FIXME: could potentially support tracking pointers and references in the
+  // future to improve catching true positives through aliases.
+  if (hasPtrOrReferenceInFunc(Func, CondVar))
+    return;
+
+  if (isChangedBefore(OuterIf->getThen(), InnerIf, CondVar, Result.Context))
+    return;
+
+  auto Diag = diag(InnerIf->getBeginLoc(), "redundant condition %0") << CondVar;
+
+  // For standalone condition variables and for "or" binary operations we simply
+  // remove the inner `if`.
+  const auto *BinOpCond = dyn_cast<BinaryOperator>(InnerIf->getCond());
+  if (isa<DeclRefExpr>(InnerIf->getCond()->IgnoreParenImpCasts()) ||
+      (BinOpCond && BinOpCond->getOpcode() == BO_LOr)) {
+    SourceLocation IfBegin = InnerIf->getBeginLoc();
+    const Stmt *Body = InnerIf->getThen();
+    const Expr *OtherSide = nullptr;
+    if (BinOpCond) {
+      const auto *LeftDRE =
+          dyn_cast<DeclRefExpr>(BinOpCond->getLHS()->IgnoreParenImpCasts());
+      if (LeftDRE && LeftDRE->getDecl() == CondVar)
+        OtherSide = BinOpCond->getRHS();
+      else
+        OtherSide = BinOpCond->getLHS();
+    }
+
+    SourceLocation IfEnd = Body->getBeginLoc().getLocWithOffset(-1);
+
+    // For compound statements also remove the left brace.
+    if (isa<CompoundStmt>(Body))
+      IfEnd = Body->getBeginLoc();
+
+    // If the other side has side effects then keep it.
+    if (OtherSide && OtherSide->HasSideEffects(*Result.Context)) {
+      SourceLocation BeforeOtherSide =
+          OtherSide->getBeginLoc().getLocWithOffset(-1);
+      SourceLocation AfterOtherSide =
+          Lexer::findNextToken(OtherSide->getEndLoc(), *Result.SourceManager,
+                               getLangOpts())
+              ->getLocation();
+      Diag << FixItHint::CreateRemoval(
+                  CharSourceRange::getTokenRange(IfBegin, BeforeOtherSide))
+           << FixItHint::CreateInsertion(AfterOtherSide, ";")
+           << FixItHint::CreateRemoval(
+                  CharSourceRange::getTokenRange(AfterOtherSide, IfEnd));
+    } else {
+      Diag << FixItHint::CreateRemoval(
+          CharSourceRange::getTokenRange(IfBegin, IfEnd));
+    }
+
+    // For comound statements also remove the right brace at the end.
+    if (isa<CompoundStmt>(Body))
+      Diag << FixItHint::CreateRemoval(
+          CharSourceRange::getTokenRange(Body->getEndLoc(), Body->getEndLoc()));
+
+    // For "and" binary operations we remove the "and" operation with the
+    // condition variable from the inner if.
+  } else {
+    const auto *CondOp = cast<BinaryOperator>(InnerIf->getCond());
+    const auto *LeftDRE =
+        dyn_cast<DeclRefExpr>(CondOp->getLHS()->IgnoreParenImpCasts());
+    if (LeftDRE && LeftDRE->getDecl() == CondVar) {
+      SourceLocation BeforeRHS =
+          CondOp->getRHS()->getBeginLoc().getLocWithOffset(-1);
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+          CondOp->getLHS()->getBeginLoc(), BeforeRHS));
+    } else {
+      SourceLocation AfterLHS =
+          Lexer::findNextToken(CondOp->getLHS()->getEndLoc(),
+                               *Result.SourceManager, getLangOpts())
+              ->getLocation();
+      Diag << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+          AfterLHS, CondOp->getRHS()->getEndLoc()));
+    }
+  }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

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

@@ -0,0 +1,35 @@
+//===--- RedundantBranchConditionCheck.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_BUGPRONE_REDUNDANTBRANCHCONDITIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_REDUNDANTBRANCHCONDITIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds condition variables in nested `if` statements that were also checked
+/// in the outer `if` statement and were not changed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-redundant-branch-condition.html
+class RedundantBranchConditionCheck : public ClangTidyCheck {
+public:
+  RedundantBranchConditionCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_REDUNDANTBRANCHCONDITIONCHECK_H

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

@@ -13,6 +13,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
   NarrowingConversionsCheck.cpp
   NoMallocCheck.cpp
   OwningMemoryCheck.cpp
+  PreferMemberInitializerCheck.cpp
   ProBoundsArrayToPointerDecayCheck.cpp
   ProBoundsConstantArrayIndexCheck.cpp
   ProBoundsPointerArithmeticCheck.cpp

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

@@ -22,6 +22,7 @@
 #include "NarrowingConversionsCheck.h"
 #include "NoMallocCheck.h"
 #include "OwningMemoryCheck.h"
+#include "PreferMemberInitializerCheck.h"
 #include "ProBoundsArrayToPointerDecayCheck.h"
 #include "ProBoundsConstantArrayIndexCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
@@ -66,6 +67,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
         "cppcoreguidelines-non-private-member-variables-in-classes");
     CheckFactories.registerCheck<OwningMemoryCheck>(
         "cppcoreguidelines-owning-memory");
+    CheckFactories.registerCheck<PreferMemberInitializerCheck>(
+        "cppcoreguidelines-prefer-member-initializer");
     CheckFactories.registerCheck<ProBoundsArrayToPointerDecayCheck>(
         "cppcoreguidelines-pro-bounds-array-to-pointer-decay");
     CheckFactories.registerCheck<ProBoundsConstantArrayIndexCheck>(