Merge branch 'llvm:main' into cfi-show

Author: eugeneepshteyn

clang-tools-extra/clang-tidy/.clang-tidy

@@ -5,7 +5,6 @@ Checks: >
   -bugprone-branch-clone,
   -bugprone-easily-swappable-parameters,
   -bugprone-narrowing-conversions,
-  -bugprone-suspicious-stringview-data-usage,
   -bugprone-unchecked-optional-access,
   -bugprone-unused-return-value,
   modernize-*,

clang-tools-extra/clang-tidy/ClangTidyOptions.cpp

@@ -99,6 +99,8 @@ void yamlize(IO &IO, ClangTidyOptions::OptionMap &Val, bool,
     for (auto &Option : SortedOptions) {
       bool UseDefault = false;
       void *SaveInfo = nullptr;
+      // Requires 'llvm::yaml::IO' to accept 'StringRef'
+      // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
       IO.preflightKey(Option.first.data(), true, false, UseDefault, SaveInfo);
       IO.scalarString(Option.second, needsQuotes(Option.second));
       IO.postflightKey(SaveInfo);
@@ -116,6 +118,8 @@ void yamlize(IO &IO, ClangTidyOptions::OptionMap &Val, bool,
     } else if (isa<MappingNode>(I.getCurrentNode())) {
       IO.beginMapping();
       for (StringRef Key : IO.keys()) {
+        // Requires 'llvm::yaml::IO' to accept 'StringRef'
+        // NOLINTNEXTLINE(bugprone-suspicious-stringview-data-usage)
         IO.mapRequired(Key.data(), Val[Key].Value);
       }
       IO.endMapping();

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

@@ -23,6 +23,7 @@
 #include "CopyConstructorInitCheck.h"
 #include "CrtpConstructorAccessibilityCheck.h"
 #include "DanglingHandleCheck.h"
+#include "DerivedMethodShadowingBaseMethodCheck.h"
 #include "DynamicStaticInitializersCheck.h"
 #include "EasilySwappableParametersCheck.h"
 #include "EmptyCatchCheck.h"
@@ -134,6 +135,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-copy-constructor-init");
     CheckFactories.registerCheck<DanglingHandleCheck>(
         "bugprone-dangling-handle");
+    CheckFactories.registerCheck<DerivedMethodShadowingBaseMethodCheck>(
+        "bugprone-derived-method-shadowing-base-method");
     CheckFactories.registerCheck<DynamicStaticInitializersCheck>(
         "bugprone-dynamic-static-initializers");
     CheckFactories.registerCheck<EasilySwappableParametersCheck>(

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

@@ -19,6 +19,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   CopyConstructorInitCheck.cpp
   CrtpConstructorAccessibilityCheck.cpp
   DanglingHandleCheck.cpp
+  DerivedMethodShadowingBaseMethodCheck.cpp
   DynamicStaticInitializersCheck.cpp
   EasilySwappableParametersCheck.cpp
   EmptyCatchCheck.cpp

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

@@ -0,0 +1,128 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "DerivedMethodShadowingBaseMethodCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+static bool sameBasicType(const ParmVarDecl *Lhs, const ParmVarDecl *Rhs) {
+  return Lhs && Rhs &&
+         Lhs->getType()
+                 .getCanonicalType()
+                 .getNonReferenceType()
+                 .getUnqualifiedType() == Rhs->getType()
+                                              .getCanonicalType()
+                                              .getNonReferenceType()
+                                              .getUnqualifiedType();
+}
+
+static bool namesCollide(const CXXMethodDecl &Lhs, const CXXMethodDecl &Rhs) {
+  if (Lhs.getNameAsString() != Rhs.getNameAsString())
+    return false;
+  if (Lhs.isConst() != Rhs.isConst())
+    return false;
+  if (Lhs.getNumParams() != Rhs.getNumParams())
+    return false;
+  for (unsigned int It = 0; It < Lhs.getNumParams(); ++It)
+    if (!sameBasicType(Lhs.getParamDecl(It), Rhs.getParamDecl(It)))
+      return false;
+  return true;
+}
+
+namespace {
+
+AST_MATCHER(CXXMethodDecl, nameCollidesWithMethodInBase) {
+  const CXXRecordDecl *DerivedClass = Node.getParent();
+  for (const auto &Base : DerivedClass->bases()) {
+    llvm::SmallVector<const CXXBaseSpecifier *, 8> Stack;
+    Stack.push_back(&Base);
+    while (!Stack.empty()) {
+      const CXXBaseSpecifier *CurrentBaseSpec = Stack.back();
+      Stack.pop_back();
+
+      if (CurrentBaseSpec->getAccessSpecifier() ==
+          clang::AccessSpecifier::AS_private)
+        continue;
+
+      const CXXRecordDecl *CurrentRecord =
+          CurrentBaseSpec->getType()->getAsCXXRecordDecl();
+      if (!CurrentRecord)
+        continue;
+
+      // For multiple inheritance, we ignore only the bases that come from the
+      // std:: namespace
+      if (CurrentRecord->isInStdNamespace())
+        continue;
+
+      for (const auto &BaseMethod : CurrentRecord->methods()) {
+        if (namesCollide(*BaseMethod, Node)) {
+          ast_matchers::internal::BoundNodesTreeBuilder Result(*Builder);
+          Builder->setBinding("base_method",
+                              clang::DynTypedNode::create(*BaseMethod));
+          return true;
+        }
+      }
+
+      for (const auto &SubBase : CurrentRecord->bases())
+        Stack.push_back(&SubBase);
+    }
+  }
+  return false;
+}
+
+// Same as clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp,
+// similar matchers are used elsewhere in LLVM
+AST_MATCHER(CXXMethodDecl, isOutOfLine) { return Node.isOutOfLine(); }
+
+} // namespace
+
+DerivedMethodShadowingBaseMethodCheck::DerivedMethodShadowingBaseMethodCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context) {}
+
+void DerivedMethodShadowingBaseMethodCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxMethodDecl(
+          unless(anyOf(isOutOfLine(), isStaticStorageClass(), isImplicit(),
+                       cxxConstructorDecl(), isOverride(), isPrivate(),
+                       // isFinal(), //included with isOverride,
+                       // Templates are not handled yet
+                       ast_matchers::isTemplateInstantiation(),
+                       ast_matchers::isExplicitTemplateSpecialization())),
+          ofClass(cxxRecordDecl(isDerivedFrom(cxxRecordDecl()))
+                      .bind("derived_class")),
+          nameCollidesWithMethodInBase())
+          .bind("shadowing_method"),
+      this);
+}
+
+void DerivedMethodShadowingBaseMethodCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *ShadowingMethod =
+      Result.Nodes.getNodeAs<CXXMethodDecl>("shadowing_method");
+  const auto *DerivedClass =
+      Result.Nodes.getNodeAs<CXXRecordDecl>("derived_class");
+  const auto *BaseMethod = Result.Nodes.getNodeAs<CXXMethodDecl>("base_method");
+
+  if (!ShadowingMethod || !DerivedClass || !BaseMethod)
+    llvm_unreachable("Required binding not found");
+
+  diag(ShadowingMethod->getBeginLoc(),
+       "'%0' shadows method with the same name in class %1")
+      << ShadowingMethod->getQualifiedNameAsString() << BaseMethod->getParent();
+  diag(BaseMethod->getBeginLoc(), "previous definition of %0 is here",
+       DiagnosticIDs::Note)
+      << ShadowingMethod;
+}
+
+} // namespace clang::tidy::bugprone

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

@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_DERIVEDMETHODSHADOWINGBASEMETHODCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DERIVEDMETHODSHADOWINGBASEMETHODCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Checks that a derived class does not define the same (non virtual) method as
+/// a base class
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/derived-method-shadowing-base-method.html
+class DerivedMethodShadowingBaseMethodCheck : public ClangTidyCheck {
+public:
+  DerivedMethodShadowingBaseMethodCheck(StringRef Name,
+                                        ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_DERIVEDMETHODSHADOWINGBASEMETHODCHECK_H

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

@@ -204,7 +204,7 @@ static bool bodyEmpty(const ASTContext *Context, const CompoundStmt *Body) {
       CharSourceRange::getCharRange(Body->getLBracLoc().getLocWithOffset(1),
                                     Body->getRBracLoc()),
       Context->getSourceManager(), Context->getLangOpts(), &Invalid);
-  return !Invalid && std::strspn(Text.data(), " \t\r\n") == Text.size();
+  return !Invalid && Text.ltrim(" \t\r\n").empty();
 }
 
 UseEqualsDefaultCheck::UseEqualsDefaultCheck(StringRef Name,

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

@@ -337,8 +337,7 @@ std::string IdentifierNamingCheck::HungarianNotation::getDeclTypeName(
 
     // Remove keywords
     for (StringRef Kw : Keywords) {
-      for (size_t Pos = 0;
-           (Pos = Type.find(Kw.data(), Pos)) != std::string::npos;) {
+      for (size_t Pos = 0; (Pos = Type.find(Kw, Pos)) != std::string::npos;) {
         Type.replace(Pos, Kw.size(), "");
       }
     }
@@ -373,7 +372,7 @@ std::string IdentifierNamingCheck::HungarianNotation::getDeclTypeName(
         " int", " char", " double", " long", " short"};
     bool RedundantRemoved = false;
     for (auto Kw : TailsOfMultiWordType) {
-      size_t Pos = Type.rfind(Kw.data());
+      size_t Pos = Type.rfind(Kw);
       if (Pos != std::string::npos) {
         const size_t PtrCount = getAsteriskCount(Type, ND);
         Type = Type.substr(0, Pos + Kw.size() + PtrCount);
@@ -602,9 +601,8 @@ std::string IdentifierNamingCheck::HungarianNotation::getDataTypePrefix(
   if (PtrCount > 0) {
     ModifiedTypeName = [&](std::string Str, StringRef From, StringRef To) {
       size_t StartPos = 0;
-      while ((StartPos = Str.find(From.data(), StartPos)) !=
-             std::string::npos) {
-        Str.replace(StartPos, From.size(), To.data());
+      while ((StartPos = Str.find(From, StartPos)) != std::string::npos) {
+        Str.replace(StartPos, From.size(), To);
         StartPos += To.size();
       }
       return Str;

clang-tools-extra/docs/ReleaseNotes.rst

@@ -163,6 +163,11 @@ New checks
   Detects default initialization (to 0) of variables with ``enum`` type where
   the enum has no enumerator with value of 0.
 
+- New :doc:`bugprone-derived-method-shadowing-base-method
+  <clang-tidy/checks/bugprone/derived-method-shadowing-base-method>` check.
+
+  Finds derived class methods that shadow a (non-virtual) base class method.
+
 - New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-access
   <clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-access>`
   check.

clang-tools-extra/docs/clang-tidy/checks/bugprone/derived-method-shadowing-base-method.rst

@@ -0,0 +1,30 @@
+.. title:: clang-tidy - bugprone-derived-method-shadowing-base-method
+
+bugprone-derived-method-shadowing-base-method
+=============================================
+
+Finds derived class methods that shadow a (non-virtual) base class method.
+
+In order to be considered "shadowing", methods must have the same signature
+(i.e. the same name, same number of parameters, same parameter types, etc).
+Only checks public, non-templated methods. 
+
+The below example is bugprone because consumers of the ``Derived`` class will
+expect the ``reset`` method to do the work of ``Base::reset()`` in addition to extra
+work required to reset the ``Derived`` class.  Common fixes include:
+
+- Making the ``reset`` method polymorphic
+- Re-naming ``Derived::reset`` if it's not meant to intersect with ``Base::reset``
+- Using ``using Base::reset`` to change the access specifier
+
+This is also a violation of the Liskov Substitution Principle.
+
+.. code-block:: c++
+
+  struct Base {
+    void reset() {/* reset the base class */};
+  };
+
+  struct Derived : public Base {
+    void reset() {/* reset the derived class, but not the base class */};
+  };