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

  CONFLICT (content): Merge conflict in clang/include/clang/Basic/Attr.td

Author: bader

clang-tools-extra/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp

@@ -52,10 +52,22 @@ void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
         callee(functionDecl(hasName("::absl::FDivDuration"))),
         hasArgument(0, expr().bind("arg")), hasArgument(1, factory_matcher));
 
+    // Matcher which matches a duration argument being scaled,
+    // e.g. `absl::ToDoubleSeconds(dur) * 2`
+    auto scalar_matcher = ignoringImpCasts(
+        binaryOperator(hasOperatorName("*"),
+                       hasEitherOperand(expr(ignoringParenImpCasts(
+                           callExpr(callee(functionDecl(hasAnyName(
+                                        FloatConversion, IntegerConversion))),
+                                    hasArgument(0, expr().bind("arg")))
+                               .bind("inner_call")))))
+            .bind("binop"));
+
     Finder->addMatcher(
         callExpr(callee(functionDecl(hasName(DurationFactory))),
                  hasArgument(0, anyOf(inverse_function_matcher,
-                                      division_operator_matcher, fdiv_matcher)))
+                                      division_operator_matcher, fdiv_matcher,
+                                      scalar_matcher)))
             .bind("call"),
         this);
   }
@@ -64,16 +76,41 @@ void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
 void DurationUnnecessaryConversionCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *OuterCall = Result.Nodes.getNodeAs<Expr>("call");
-  const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
 
   if (isInMacro(Result, OuterCall))
     return;
 
+  FixItHint Hint;
+  if (const auto *Binop = Result.Nodes.getNodeAs<BinaryOperator>("binop")) {
+    const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg");
+    const auto *InnerCall = Result.Nodes.getNodeAs<Expr>("inner_call");
+    const Expr *LHS = Binop->getLHS();
+    const Expr *RHS = Binop->getRHS();
+
+    if (LHS->IgnoreParenImpCasts() == InnerCall) {
+      Hint = FixItHint::CreateReplacement(
+          OuterCall->getSourceRange(),
+          (llvm::Twine(tooling::fixit::getText(*Arg, *Result.Context)) + " * " +
+           tooling::fixit::getText(*RHS, *Result.Context))
+              .str());
+    } else {
+      assert(RHS->IgnoreParenImpCasts() == InnerCall &&
+             "Inner call should be find on the RHS");
+
+      Hint = FixItHint::CreateReplacement(
+          OuterCall->getSourceRange(),
+          (llvm::Twine(tooling::fixit::getText(*LHS, *Result.Context)) + " * " +
+           tooling::fixit::getText(*Arg, *Result.Context))
+              .str());
+    }
+  } else if (const auto *Arg = Result.Nodes.getNodeAs<Expr>("arg")) {
+    Hint = FixItHint::CreateReplacement(
+        OuterCall->getSourceRange(),
+        tooling::fixit::getText(*Arg, *Result.Context));
+  }
   diag(OuterCall->getBeginLoc(),
        "remove unnecessary absl::Duration conversions")
-      << FixItHint::CreateReplacement(
-             OuterCall->getSourceRange(),
-             tooling::fixit::getText(*Arg, *Result.Context));
+      << Hint;
 }
 
 } // namespace abseil

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

@@ -18,6 +18,8 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 
+static constexpr int UnsignedASCIIUpperBound = 127;
+
 static Matcher<TypedefDecl> hasAnyListedName(const std::string &Names) {
   const std::vector<std::string> NameList =
       utils::options::parseStringList(Names);
@@ -33,70 +35,114 @@ void SignedCharMisuseCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "CharTypdefsToIgnore", CharTypdefsToIgnoreList);
 }
 
-void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+// Create a matcher for char -> integer cast.
+BindableMatcher<clang::Stmt> SignedCharMisuseCheck::charCastExpression(
+    bool IsSigned, const Matcher<clang::QualType> &IntegerType,
+    const std::string &CastBindName) const {
   // We can ignore typedefs which are some kind of integer types
   // (e.g. typedef char sal_Int8). In this case, we don't need to
   // worry about the misinterpretation of char values.
   const auto IntTypedef = qualType(
       hasDeclaration(typedefDecl(hasAnyListedName(CharTypdefsToIgnoreList))));
 
-  const auto SignedCharType = expr(hasType(qualType(
-      allOf(isAnyCharacter(), isSignedInteger(), unless(IntTypedef)))));
-
-  const auto IntegerType = qualType(allOf(isInteger(), unless(isAnyCharacter()),
-                                          unless(booleanType())))
-                               .bind("integerType");
+  auto CharTypeExpr = expr();
+  if (IsSigned) {
+    CharTypeExpr = expr(hasType(
+        qualType(isAnyCharacter(), isSignedInteger(), unless(IntTypedef))));
+  } else {
+    CharTypeExpr = expr(hasType(qualType(
+        isAnyCharacter(), unless(isSignedInteger()), unless(IntTypedef))));
+  }
 
-  // We are interested in signed char -> integer conversion.
   const auto ImplicitCastExpr =
-      implicitCastExpr(hasSourceExpression(SignedCharType),
+      implicitCastExpr(hasSourceExpression(CharTypeExpr),
                        hasImplicitDestinationType(IntegerType))
-          .bind("castExpression");
+          .bind(CastBindName);
 
   const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
   const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
   const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
 
   // We catch any type of casts to an integer. We need to have these cast
   // expressions explicitly to catch only those casts which are direct children
-  // of an assignment/declaration.
-  const auto CastExpr = expr(anyOf(ImplicitCastExpr, CStyleCastExpr,
-                                   StaticCastExpr, FunctionalCastExpr));
+  // of the checked expressions. (e.g. assignment, declaration).
+  return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
+                    FunctionalCastExpr));
+}
 
-  // Catch assignments with the suspicious type conversion.
-  const auto AssignmentOperatorExpr = expr(binaryOperator(
-      hasOperatorName("="), hasLHS(hasType(IntegerType)), hasRHS(CastExpr)));
+void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IntegerType =
+      qualType(isInteger(), unless(isAnyCharacter()), unless(booleanType()))
+          .bind("integerType");
+  const auto SignedCharCastExpr =
+      charCastExpression(true, IntegerType, "signedCastExpression");
+  const auto UnSignedCharCastExpr =
+      charCastExpression(false, IntegerType, "unsignedCastExpression");
+
+  // Catch assignments with singed char -> integer conversion.
+  const auto AssignmentOperatorExpr =
+      expr(binaryOperator(hasOperatorName("="), hasLHS(hasType(IntegerType)),
+                          hasRHS(SignedCharCastExpr)));
 
   Finder->addMatcher(AssignmentOperatorExpr, this);
 
-  // Catch declarations with the suspicious type conversion.
-  const auto Declaration =
-      varDecl(isDefinition(), hasType(IntegerType), hasInitializer(CastExpr));
+  // Catch declarations with singed char -> integer conversion.
+  const auto Declaration = varDecl(isDefinition(), hasType(IntegerType),
+                                   hasInitializer(SignedCharCastExpr));
 
   Finder->addMatcher(Declaration, this);
+
+  // Catch signed char/unsigned char comparison.
+  const auto CompareOperator =
+      expr(binaryOperator(hasAnyOperatorName("==", "!="),
+                          anyOf(allOf(hasLHS(SignedCharCastExpr),
+                                      hasRHS(UnSignedCharCastExpr)),
+                                allOf(hasLHS(UnSignedCharCastExpr),
+                                      hasRHS(SignedCharCastExpr)))))
+          .bind("comparison");
+
+  Finder->addMatcher(CompareOperator, this);
 }
 
 void SignedCharMisuseCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *CastExpression =
-      Result.Nodes.getNodeAs<ImplicitCastExpr>("castExpression");
-  const auto *IntegerType = Result.Nodes.getNodeAs<QualType>("integerType");
-  assert(CastExpression);
-  assert(IntegerType);
+  const auto *SignedCastExpression =
+      Result.Nodes.getNodeAs<ImplicitCastExpr>("signedCastExpression");
 
-  // Ignore the match if we know that the value is not negative.
+  // Ignore the match if we know that the signed char's value is not negative.
   // The potential misinterpretation happens for negative values only.
   Expr::EvalResult EVResult;
-  if (!CastExpression->isValueDependent() &&
-      CastExpression->getSubExpr()->EvaluateAsInt(EVResult, *Result.Context)) {
-    llvm::APSInt Value1 = EVResult.Val.getInt();
-    if (Value1.isNonNegative())
+  if (!SignedCastExpression->isValueDependent() &&
+      SignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult,
+                                                        *Result.Context)) {
+    llvm::APSInt Value = EVResult.Val.getInt();
+    if (Value.isNonNegative())
       return;
   }
 
-  diag(CastExpression->getBeginLoc(),
-       "'signed char' to %0 conversion; "
-       "consider casting to 'unsigned char' first.")
-      << *IntegerType;
+  if (const auto *Comparison = Result.Nodes.getNodeAs<Expr>("comparison")) {
+    const auto *UnSignedCastExpression =
+        Result.Nodes.getNodeAs<ImplicitCastExpr>("unsignedCastExpression");
+
+    // We can ignore the ASCII value range also for unsigned char.
+    Expr::EvalResult EVResult;
+    if (!UnSignedCastExpression->isValueDependent() &&
+        UnSignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult,
+                                                            *Result.Context)) {
+      llvm::APSInt Value = EVResult.Val.getInt();
+      if (Value <= UnsignedASCIIUpperBound)
+        return;
+    }
+
+    diag(Comparison->getBeginLoc(),
+         "comparison between 'signed char' and 'unsigned char'");
+  } else if (const auto *IntegerType =
+                 Result.Nodes.getNodeAs<QualType>("integerType")) {
+    diag(SignedCastExpression->getBeginLoc(),
+         "'signed char' to %0 conversion; "
+         "consider casting to 'unsigned char' first.")
+        << *IntegerType;
+  } else
+    llvm_unreachable("Unexpected match");
 }
 
 } // namespace bugprone

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

@@ -15,13 +15,11 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 
-/// Finds ``signed char`` -> integer conversions which might indicate a programming
-/// error. The basic problem with the ``signed char``, that it might store the
-/// non-ASCII characters as negative values. The human programmer probably
-/// expects that after an integer conversion the converted value matches with the
-/// character code (a value from [0..255]), however, the actual value is in
-/// [-128..127] interval. This also applies to the plain ``char`` type on
-/// those implementations which represent ``char`` similar to ``signed char``.
+/// Finds those ``signed char`` -> integer conversions which might indicate a
+/// programming error. The basic problem with the ``signed char``, that it might
+/// store the non-ASCII characters as negative values. This behavior can cause a
+/// misunderstanding of the written code both when an explicit and when an
+/// implicit conversion happens.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-signed-char-misuse.html
@@ -34,6 +32,11 @@ class SignedCharMisuseCheck : public ClangTidyCheck {
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
+  ast_matchers::internal::BindableMatcher<clang::Stmt> charCastExpression(
+      bool IsSigned,
+      const ast_matchers::internal::Matcher<clang::QualType> &IntegerType,
+      const std::string &CastBindName) const;
+
   const std::string CharTypdefsToIgnoreList;
 };
 

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

@@ -0,0 +1,67 @@
+//===--- AvoidNonConstGlobalVariablesCheck.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 "AvoidNonConstGlobalVariablesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
+  auto GlobalVariable = varDecl(
+      hasGlobalStorage(),
+      unless(anyOf(
+          isConstexpr(), hasType(isConstQualified()),
+          hasType(referenceType())))); // References can't be changed, only the
+                                       // data they reference can be changed.
+
+  auto GlobalReferenceToNonConst =
+      varDecl(hasGlobalStorage(), hasType(referenceType()),
+              unless(hasType(references(qualType(isConstQualified())))));
+
+  auto GlobalPointerToNonConst =
+      varDecl(hasGlobalStorage(),
+              hasType(pointerType(pointee(unless(isConstQualified())))));
+
+  Finder->addMatcher(GlobalVariable.bind("non-const_variable"), this);
+  Finder->addMatcher(GlobalReferenceToNonConst.bind("indirection_to_non-const"),
+                     this);
+  Finder->addMatcher(GlobalPointerToNonConst.bind("indirection_to_non-const"),
+                     this);
+}
+
+void AvoidNonConstGlobalVariablesCheck::check(
+    const MatchFinder::MatchResult &Result) {
+
+  if (const auto *Variable =
+          Result.Nodes.getNodeAs<VarDecl>("non-const_variable")) {
+    diag(Variable->getLocation(), "variable %0 is non-const and globally "
+                                  "accessible, consider making it const")
+        << Variable; // FIXME: Add fix-it hint to Variable
+    // Don't return early, a non-const variable may also be a pointer or
+    // reference to non-const data.
+  }
+
+  if (const auto *VD =
+          Result.Nodes.getNodeAs<VarDecl>("indirection_to_non-const")) {
+    diag(VD->getLocation(),
+         "variable %0 provides global access to a non-const object; consider "
+         "making the %select{referenced|pointed-to}1 data 'const'")
+        << VD
+        << VD->getType()->isPointerType(); // FIXME: Add fix-it hint to Variable
+  }
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

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

@@ -0,0 +1,35 @@
+//===--- AvoidNonConstGlobalVariablesCheck.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_CPPCOREGUIDELINES_AVOIDNONCONSTGLOBALVARIABLESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDNONCONSTGLOBALVARIABLESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Non-const global variables hide dependencies and make the dependencies
+/// subject to unpredictable changes.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.html
+class AvoidNonConstGlobalVariablesCheck : public ClangTidyCheck {
+public:
+  AvoidNonConstGlobalVariablesCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDNONCONSTGLOBALVARIABLESCHECK_H

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

@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   AvoidGotoCheck.cpp
+  AvoidNonConstGlobalVariablesCheck.cpp
   CppCoreGuidelinesTidyModule.cpp
   InitVariablesCheck.cpp
   InterfacesGlobalInitCheck.cpp

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

@@ -15,6 +15,7 @@
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
+#include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "MacroUsageCheck.h"
@@ -48,6 +49,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
         "cppcoreguidelines-avoid-goto");
     CheckFactories.registerCheck<readability::MagicNumbersCheck>(
         "cppcoreguidelines-avoid-magic-numbers");
+    CheckFactories.registerCheck<AvoidNonConstGlobalVariablesCheck>(
+        "cppcoreguidelines-avoid-non-const-global-variables");
     CheckFactories.registerCheck<modernize::UseOverrideCheck>(
         "cppcoreguidelines-explicit-virtual-functions");
     CheckFactories.registerCheck<InitVariablesCheck>(