[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf1e2469edcfc: [clang-tidy] Fix false-positive in
cppcoreguidelines-slicing (authored by PiotrZSL).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144206/new/
https://reviews.llvm.org/D144206
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,25 @@
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) {}
+};
+
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -199,11 +199,15 @@
` 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
` 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
+ ` check when warning would be
+ emitted in constructor for virtual base class initialization.
+
Removed checks
^^
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -40,12 +40,15 @@
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 @@
// 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.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3
[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144206/new/ https://reviews.llvm.org/D144206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
PiotrZSL added a comment. Ack. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144206/new/ https://reviews.llvm.org/D144206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
PiotrZSL updated this revision to Diff 504377.
PiotrZSL marked an inline comment as done.
PiotrZSL added a comment.
Review comments fix
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144206/new/
https://reviews.llvm.org/D144206
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,25 @@
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) {}
+};
+
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -199,11 +199,15 @@
` 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
` 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
+ ` check when warning would be
+ emitted in constructor for virtual base class initialization.
+
Removed checks
^^
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -40,12 +40,15 @@
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 @@
// 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.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,25 @@
a = h;
// CHECK-MESSAGES: :[[@LINE-1]]:5:
[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
carlosgalvezp added a comment.
Looks good, just a small comment!
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp:102
+
+struct BaseA {
+virtual ~BaseA() {}
Please briefly document this test case, i.e. why in this case this is not
considered slicing and the check should not warn. It would be good to also
mention the Github issue for traceability.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144206/new/
https://reviews.llvm.org/D144206
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
PiotrZSL updated this revision to Diff 503287.
PiotrZSL added a comment.
Rebase due to broken baseline
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144206/new/
https://reviews.llvm.org/D144206
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
a = h;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type
'DerivedThatOverridesH' to 'Base' discards override 'h'
}
+
+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) {}
+};
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,10 @@
` check. Basic support
for bit-field and integer members as a loop variable or upper limit were
added.
+- Fixed a false positive in :doc:`cppcoreguidelines-slicing
+ ` check when warning would be
+ emitted in constructor for virtual base class initialization.
+
Removed checks
^^
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -40,12 +40,15 @@
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 @@
// 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.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
a = h;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedThatOverridesH' to 'Base' discards override 'h'
}
+
+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) {}
[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
PiotrZSL updated this revision to Diff 503082.
PiotrZSL added a comment.
Ping, Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144206/new/
https://reviews.llvm.org/D144206
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
a = h;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type
'DerivedThatOverridesH' to 'Base' discards override 'h'
}
+
+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) {}
+};
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,10 @@
` check. Basic support
for bit-field and integer members as a loop variable or upper limit were
added.
+- Fixed a false positive in :doc:`cppcoreguidelines-slicing
+ ` check when warning would be
+ emitted in constructor for virtual base class initialization.
+
Removed checks
^^
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -40,12 +40,15 @@
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 @@
// 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.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
a = h;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedThatOverridesH' to 'Base' discards override 'h'
}
+
+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) {}
+};
+
Index:
[PATCH] D144206: [clang-tidy] Fix false-positive in cppcoreguidelines-slicing
PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL published this revision for review.
PiotrZSL added a comment.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Ready for review.
When warning would be emitted in constructor for virtual base class
initialization.
Fixes: https://github.com/llvm/llvm-project/issues/31187
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D144206
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
a = h;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type
'DerivedThatOverridesH' to 'Base' discards override 'h'
}
+
+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) {}
+};
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,10 @@
Changes in existing checks
^^
+- Fixed a false positive in :doc:`cppcoreguidelines-slicing
+ ` check when warning would be
+ emitted in constructor for virtual base class initialization.
+
Removed checks
^^
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
@@ -40,12 +40,15 @@
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 @@
// 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.
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp
@@ -98,3 +98,20 @@
a = h;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedThatOverridesH' to 'Base' discards override 'h'
}
+
+struct BaseA {
+virtual ~BaseA() {}
+virtual void foo() {}
+
+int i;
+};
+
