[clang-tools-extra] f1e2469 - [clang-tidy] Fix false-positive in cppcoreguidelines-slicing

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-03-11T17:36:21Z
New Revision: f1e2469edcfc160867e4ef73b2dcc259974c9d6a

URL: https://github.com/llvm/llvm-project/commit/f1e2469edcfc160867e4ef73b2dcc259974c9d6a
DIFF: https://github.com/llvm/llvm-project/commit/f1e2469edcfc160867e4ef73b2dcc259974c9d6a.diff

LOG: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing

When warning would be emitted in constructor for virtual base class
initialization.

Fixes: https://github.com/llvm/llvm-project/issues/31187

Reviewed By: carlosgalvezp

Differential Revision: https://reviews.llvm.org/D144206

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
index bc094a2f028d..76754394de76 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -40,12 +40,15 @@ void SlicingCheck::registerMatchers(MatchFinder *Finder) {
   const auto HasTypeDerivedFromBaseDecl =
       anyOf(hasType(IsDerivedFromBaseDecl),
             hasType(references(IsDerivedFromBaseDecl)));
-  const auto IsWithinDerivedCtor =
-      hasParent(cxxConstructorDecl(ofClass(equalsBoundNode("DerivedDecl"))));
+  const auto IsCallToBaseClass = hasParent(cxxConstructorDecl(
+      ofClass(isSameOrDerivedFrom(equalsBoundNode("DerivedDecl"))),
+      hasAnyConstructorInitializer(allOf(
+          isBaseInitializer(), withInitializer(equalsBoundNode("Call"))))));
 
   // Assignment slicing: "a = b;" and "a = std::move(b);" variants.
   const auto SlicesObjectInAssignment =
-      callExpr(callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
+      callExpr(expr().bind("Call"),
+               callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
                                           isMoveAssignmentOperator()),
                                     OfBaseClass)),
                hasArgument(1, HasTypeDerivedFromBaseDecl));
@@ -53,17 +56,17 @@ void SlicingCheck::registerMatchers(MatchFinder *Finder) {
   // Construction slicing: "A a{b};" and "f(b);" variants. Note that in case of
   // slicing the letter will create a temporary and therefore call a ctor.
   const auto SlicesObjectInCtor = cxxConstructExpr(
+      expr().bind("Call"),
       hasDeclaration(cxxConstructorDecl(
           anyOf(isCopyConstructor(), isMoveConstructor()), OfBaseClass)),
       hasArgument(0, HasTypeDerivedFromBaseDecl),
       // We need to disable matching on the call to the base copy/move
       // constructor in DerivedDecl's constructors.
-      unless(IsWithinDerivedCtor));
+      unless(IsCallToBaseClass));
 
-  Finder->addMatcher(traverse(TK_AsIs, expr(anyOf(SlicesObjectInAssignment,
-                                                  SlicesObjectInCtor))
-                                           .bind("Call")),
-                     this);
+  Finder->addMatcher(
+      traverse(TK_AsIs, expr(SlicesObjectInAssignment).bind("Call")), this);
+  Finder->addMatcher(traverse(TK_AsIs, SlicesObjectInCtor), this);
 }
 
 /// Warns on methods overridden in DerivedDecl with respect to BaseDecl.

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index bda6168f3aac..4e861dcd38cf 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -199,11 +199,15 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/too-small-loop-variable>` check. Basic support
   for bit-field and integer members as a loop variable or upper limit were added.
 
-- Improved :doc:`readability-magic-numbers 
+- Improved :doc:`readability-magic-numbers
   <clang-tidy/checks/readability/magic-numbers>` check, now allows for
   magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
   the new ``IgnoreTypeAliases`` option is set to true.
 
+- Fixed a false positive in :doc:`cppcoreguidelines-slicing
+  <clang-tidy/checks/cppcoreguidelines/slicing>` check when warning would be
+  emitted in constructor for virtual base class initialization.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
index 6856f52f7061..58ca644f18d4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,25 @@ void negatives() {
   a = h;
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedThatOverridesH' to 'Base' discards override 'h'
 }
+
+namespace PR31187 {
+// Don't warn when calling constructor of base virtual class, from
+// initialization list of derived class constructor.
+
+struct BaseA {
+virtual ~BaseA() {}
+virtual void foo() {}
+
+int i;
+};
+
+struct BaseB : virtual BaseA {
+virtual void foo() {}
+};
+
+struct ClassWithVirtualBases : BaseB {
+  ClassWithVirtualBases(const BaseB& other) : BaseA(other), BaseB(other) {}
+  ClassWithVirtualBases(const ClassWithVirtualBases& other) : BaseA(other), BaseB(other) {}
+};
+
+}