Merge branch 'main' into mlir-llvm-module-asm

Author: gitoleg

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

@@ -38,6 +38,7 @@
 #include "IncorrectRoundingsCheck.h"
 #include "InfiniteLoopCheck.h"
 #include "IntegerDivisionCheck.h"
+#include "InvalidEnumDefaultInitializationCheck.h"
 #include "LambdaFunctionNameCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
@@ -165,6 +166,8 @@ class BugproneModule : public ClangTidyModule {
     CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop");
     CheckFactories.registerCheck<IntegerDivisionCheck>(
         "bugprone-integer-division");
+    CheckFactories.registerCheck<InvalidEnumDefaultInitializationCheck>(
+        "bugprone-invalid-enum-default-initialization");
     CheckFactories.registerCheck<LambdaFunctionNameCheck>(
         "bugprone-lambda-function-name");
     CheckFactories.registerCheck<MacroParenthesesCheck>(

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

@@ -30,6 +30,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   InaccurateEraseCheck.cpp
   IncorrectEnableIfCheck.cpp
   IncorrectEnableSharedFromThisCheck.cpp
+  InvalidEnumDefaultInitializationCheck.cpp
   UnintendedCharOstreamOutputCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SuspiciousStringviewDataUsageCheck.cpp

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

@@ -0,0 +1,180 @@
+//===--- InvalidEnumDefaultInitializationCheck.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 "InvalidEnumDefaultInitializationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/TypeVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+bool isCompleteAndHasNoZeroValue(const EnumDecl *D) {
+  const EnumDecl *Definition = D->getDefinition();
+  return Definition && Definition->isComplete() &&
+         !Definition->enumerators().empty() &&
+         std::none_of(Definition->enumerator_begin(),
+                      Definition->enumerator_end(),
+                      [](const EnumConstantDecl *Value) {
+                        return Value->getInitVal().isZero();
+                      });
+}
+
+AST_MATCHER(EnumDecl, isCompleteAndHasNoZeroValue) {
+  return isCompleteAndHasNoZeroValue(&Node);
+}
+
+// Find an initialization which initializes the value (if it has enum type) to a
+// default zero value.
+AST_MATCHER(Expr, isEmptyInit) {
+  if (isa<CXXScalarValueInitExpr, ImplicitValueInitExpr>(&Node))
+    return true;
+  if (const auto *Init = dyn_cast<InitListExpr>(&Node)) {
+    if (Init->getNumInits() == 0)
+      return true;
+  }
+  return false;
+}
+
+AST_MATCHER(InitListExpr, hasArrayFiller) { return Node.hasArrayFiller(); }
+
+// Check if any type has a "child" type that is an enum without zero value.
+// The "child" type can be an array element type or member type of a record
+// type (or a recursive combination of these). In this case, if the "root" type
+// is statically initialized, the enum component is initialized to zero.
+class FindEnumMember : public TypeVisitor<FindEnumMember, bool> {
+public:
+  const EnumType *FoundEnum = nullptr;
+
+  bool VisitType(const Type *T) {
+    const Type *DesT = T->getUnqualifiedDesugaredType();
+    if (DesT != T)
+      return Visit(DesT);
+    return false;
+  }
+  bool VisitArrayType(const ArrayType *T) {
+    return Visit(T->getElementType().getTypePtr());
+  }
+  bool VisitConstantArrayType(const ConstantArrayType *T) {
+    return Visit(T->getElementType().getTypePtr());
+  }
+  bool VisitEnumType(const EnumType *T) {
+    if (isCompleteAndHasNoZeroValue(T->getDecl())) {
+      FoundEnum = T;
+      return true;
+    }
+    return false;
+  }
+  bool VisitRecordType(const RecordType *T) {
+    const RecordDecl *RD = T->getDecl();
+    if (RD->isUnion())
+      return false;
+    auto VisitField = [this](const FieldDecl *F) {
+      return Visit(F->getType().getTypePtr());
+    };
+    return llvm::any_of(RD->fields(), VisitField);
+  }
+};
+
+} // namespace
+
+InvalidEnumDefaultInitializationCheck::InvalidEnumDefaultInitializationCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {}
+
+void InvalidEnumDefaultInitializationCheck::registerMatchers(
+    MatchFinder *Finder) {
+  auto EnumWithoutZeroValue = enumType(
+      hasDeclaration(enumDecl(isCompleteAndHasNoZeroValue()).bind("enum")));
+  auto EnumOrArrayOfEnum = qualType(hasUnqualifiedDesugaredType(
+      anyOf(EnumWithoutZeroValue,
+            arrayType(hasElementType(qualType(
+                hasUnqualifiedDesugaredType(EnumWithoutZeroValue)))))));
+  Finder->addMatcher(
+      expr(isEmptyInit(), hasType(EnumOrArrayOfEnum)).bind("expr"), this);
+
+  // Array initialization can contain an "array filler" for the (syntactically)
+  // unspecified elements. This expression is not found by AST matchers and can
+  // have any type (the array's element type). This is an implicitly generated
+  // initialization, so if the type contains somewhere an enum without zero
+  // enumerator, the zero initialization applies here. We search this array
+  // element type for the specific enum type manually when this matcher matches.
+  Finder->addMatcher(initListExpr(hasArrayFiller()).bind("array_filler_expr"),
+                     this);
+}
+
+void InvalidEnumDefaultInitializationCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *InitExpr = Result.Nodes.getNodeAs<Expr>("expr");
+  const auto *Enum = Result.Nodes.getNodeAs<EnumDecl>("enum");
+  if (!InitExpr) {
+    const auto *InitList =
+        Result.Nodes.getNodeAs<InitListExpr>("array_filler_expr");
+    // Initialization of omitted array elements with array filler was found.
+    // Check the type for enum without zero value.
+    // FIXME: In this way only one enum-typed value is found, not all of these.
+    FindEnumMember Finder;
+    if (!Finder.Visit(InitList->getArrayFiller()->getType().getTypePtr()))
+      return;
+    InitExpr = InitList;
+    Enum = Finder.FoundEnum->getDecl();
+  }
+
+  if (!InitExpr || !Enum)
+    return;
+
+  ASTContext &ACtx = Enum->getASTContext();
+  SourceLocation Loc = InitExpr->getExprLoc();
+  if (Loc.isInvalid()) {
+    if (isa<ImplicitValueInitExpr, InitListExpr>(InitExpr)) {
+      DynTypedNodeList Parents = ACtx.getParents(*InitExpr);
+      if (Parents.empty())
+        return;
+
+      if (const auto *Ctor = Parents[0].get<CXXConstructorDecl>()) {
+        // Try to find member initializer with the found expression and get the
+        // source ___location from it.
+        CXXCtorInitializer *const *CtorInit = std::find_if(
+            Ctor->init_begin(), Ctor->init_end(),
+            [InitExpr](const CXXCtorInitializer *Init) {
+              return Init->isMemberInitializer() && Init->getInit() == InitExpr;
+            });
+        if (!CtorInit)
+          return;
+        Loc = (*CtorInit)->getLParenLoc();
+      } else if (const auto *InitList = Parents[0].get<InitListExpr>()) {
+        // The expression may be implicitly generated for an initialization.
+        // Search for a parent initialization list with valid source ___location.
+        while (InitList->getExprLoc().isInvalid()) {
+          DynTypedNodeList Parents = ACtx.getParents(*InitList);
+          if (Parents.empty())
+            return;
+          InitList = Parents[0].get<InitListExpr>();
+          if (!InitList)
+            return;
+        }
+        Loc = InitList->getExprLoc();
+      }
+    }
+    // If still not found a source ___location, omit the warning.
+    // Ideally all such cases (if they exist) should be handled to make the
+    // check more precise.
+    if (Loc.isInvalid())
+      return;
+  }
+  diag(Loc, "enum value of type %0 initialized with invalid value of 0, "
+            "enum doesn't have a zero-value enumerator")
+      << Enum;
+  diag(Enum->getLocation(), "enum is defined here", DiagnosticIDs::Note);
+}
+
+} // namespace clang::tidy::bugprone

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

@@ -0,0 +1,31 @@
+//===--- InvalidEnumDefaultInitializationCheck.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_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects default initialization (to 0) of variables with `enum` type where
+/// the enum has no enumerator with value of 0.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/invalid-enum-default-initialization.html
+class InvalidEnumDefaultInitializationCheck : public ClangTidyCheck {
+public:
+  InvalidEnumDefaultInitializationCheck(StringRef Name,
+                                        ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INVALIDENUMDEFAULTINITIALIZATIONCHECK_H

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

@@ -3267,6 +3267,56 @@ TEST(SignatureHelpTest, VariadicType) {
   }
 }
 
+TEST(SignatureHelpTest, SkipExplicitObjectParameter) {
+  Annotations Code(R"cpp(
+    struct A {
+      void foo(this auto&& self, int arg); 
+      void bar(this A self, int arg);
+    };
+    int main() {
+      A a {};
+      a.foo($c1^);
+      (&A::bar)($c2^);
+      (&A::foo)($c3^);
+    }
+  )cpp");
+
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs = {"-std=c++23"};
+
+  MockFS FS;
+  auto Inputs = TU.inputs(FS);
+
+  auto Preamble = TU.preamble();
+  ASSERT_TRUE(Preamble);
+
+  {
+    const auto Result = signatureHelp(testPath(TU.Filename), Code.point("c1"),
+                                      *Preamble, Inputs, MarkupKind::PlainText);
+
+    EXPECT_EQ(1U, Result.signatures.size());
+
+    EXPECT_THAT(Result.signatures[0], AllOf(sig("foo([[int arg]]) -> void")));
+  }
+  {
+    const auto Result = signatureHelp(testPath(TU.Filename), Code.point("c2"),
+                                      *Preamble, Inputs, MarkupKind::PlainText);
+
+    EXPECT_EQ(1U, Result.signatures.size());
+
+    EXPECT_THAT(Result.signatures[0], AllOf(sig("([[A]], [[int]]) -> void")));
+  }
+  {
+    // TODO: llvm/llvm-project/146649
+    const auto Result = signatureHelp(testPath(TU.Filename), Code.point("c3"),
+                                      *Preamble, Inputs, MarkupKind::PlainText);
+    // TODO: We expect 1 signature here, with this signature
+    EXPECT_EQ(0U, Result.signatures.size());
+    // EXPECT_THAT(Result.signatures[0], AllOf(sig("([[auto&&]], [[int]]) ->
+    // void")));
+  }
+}
+
 TEST(CompletionTest, IncludedCompletionKinds) {
   Annotations Test(R"cpp(#include "^)cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -4369,14 +4419,24 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
   Annotations Code(R"cpp(
     struct A {
       void foo(this auto&& self, int arg); 
+      void bar(this A self, int arg);
     };
 
     int main() {
       A a {};
-      a.^
+      a.$c1^;
+      (&A::fo$c2^;
+      (&A::ba$c3^;
     }
   )cpp");
 
+  // TODO: llvm/llvm-project/146649
+  // This is incorrect behavior. Correct Result should be a variant of,
+  // c2: signature = (auto&& self, int arg)
+  //     snippet = (${1: auto&& self}, ${2: int arg})
+  // c3: signature = (A self, int arg)
+  //     snippet = (${1: A self}, ${2: int arg})
+
   auto TU = TestTU::withCode(Code.code());
   TU.ExtraArgs = {"-std=c++23"};
 
@@ -4387,12 +4447,31 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
 
   MockFS FS;
   auto Inputs = TU.inputs(FS);
-  auto Result = codeComplete(testPath(TU.Filename), Code.point(),
-                             Preamble.get(), Inputs, Opts);
-
-  EXPECT_THAT(Result.Completions,
-              ElementsAre(AllOf(named("foo"), signature("(int arg)"),
-                                snippetSuffix("(${1:int arg})"))));
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c1"),
+                               Preamble.get(), Inputs, Opts);
+
+    EXPECT_THAT(Result.Completions,
+                UnorderedElementsAre(AllOf(named("foo"), signature("(int arg)"),
+                                           snippetSuffix("(${1:int arg})")),
+                                     AllOf(named("bar"), signature("(int arg)"),
+                                           snippetSuffix("(${1:int arg})"))));
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c2"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(
+        Result.Completions,
+        ElementsAre(AllOf(named("foo"), signature("<class self:auto>(int arg)"),
+                          snippetSuffix("<${1:class self:auto}>"))));
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c3"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(Result.Completions,
+                ElementsAre(AllOf(named("bar"), signature("(int arg)"),
+                                  snippetSuffix(""))));
+  }
 }
 } // namespace
 } // namespace clangd

clang-tools-extra/docs/ReleaseNotes.rst

@@ -108,6 +108,12 @@ Improvements to clang-tidy
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-invalid-enum-default-initialization
+  <clang-tidy/checks/bugprone/invalid-enum-default-initialization>` check.
+
+  Detects default initialization (to 0) of variables with ``enum`` type where
+  the enum has no enumerator with value of 0.
+
 - New :doc:`llvm-mlir-op-builder
   <clang-tidy/checks/llvm/use-new-mlir-op-builder>` check.