Skip to content

Commit f1e2469

Browse files
committed
[clang-tidy] Fix false-positive in cppcoreguidelines-slicing
When warning would be emitted in constructor for virtual base class initialization. Fixes: #31187 Reviewed By: carlosgalvezp Differential Revision: https://reviews.llvm.org/D144206
1 parent 9b8d944 commit f1e2469

File tree

3 files changed

+38
-9
lines changed

3 files changed

+38
-9
lines changed

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

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,30 +40,33 @@ void SlicingCheck::registerMatchers(MatchFinder *Finder) {
4040
const auto HasTypeDerivedFromBaseDecl =
4141
anyOf(hasType(IsDerivedFromBaseDecl),
4242
hasType(references(IsDerivedFromBaseDecl)));
43-
const auto IsWithinDerivedCtor =
44-
hasParent(cxxConstructorDecl(ofClass(equalsBoundNode("DerivedDecl"))));
43+
const auto IsCallToBaseClass = hasParent(cxxConstructorDecl(
44+
ofClass(isSameOrDerivedFrom(equalsBoundNode("DerivedDecl"))),
45+
hasAnyConstructorInitializer(allOf(
46+
isBaseInitializer(), withInitializer(equalsBoundNode("Call"))))));
4547

4648
// Assignment slicing: "a = b;" and "a = std::move(b);" variants.
4749
const auto SlicesObjectInAssignment =
48-
callExpr(callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
50+
callExpr(expr().bind("Call"),
51+
callee(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
4952
isMoveAssignmentOperator()),
5053
OfBaseClass)),
5154
hasArgument(1, HasTypeDerivedFromBaseDecl));
5255

5356
// Construction slicing: "A a{b};" and "f(b);" variants. Note that in case of
5457
// slicing the letter will create a temporary and therefore call a ctor.
5558
const auto SlicesObjectInCtor = cxxConstructExpr(
59+
expr().bind("Call"),
5660
hasDeclaration(cxxConstructorDecl(
5761
anyOf(isCopyConstructor(), isMoveConstructor()), OfBaseClass)),
5862
hasArgument(0, HasTypeDerivedFromBaseDecl),
5963
// We need to disable matching on the call to the base copy/move
6064
// constructor in DerivedDecl's constructors.
61-
unless(IsWithinDerivedCtor));
65+
unless(IsCallToBaseClass));
6266

63-
Finder->addMatcher(traverse(TK_AsIs, expr(anyOf(SlicesObjectInAssignment,
64-
SlicesObjectInCtor))
65-
.bind("Call")),
66-
this);
67+
Finder->addMatcher(
68+
traverse(TK_AsIs, expr(SlicesObjectInAssignment).bind("Call")), this);
69+
Finder->addMatcher(traverse(TK_AsIs, SlicesObjectInCtor), this);
6770
}
6871

6972
/// Warns on methods overridden in DerivedDecl with respect to BaseDecl.

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,15 @@ Changes in existing checks
199199
<clang-tidy/checks/bugprone/too-small-loop-variable>` check. Basic support
200200
for bit-field and integer members as a loop variable or upper limit were added.
201201

202-
- Improved :doc:`readability-magic-numbers
202+
- Improved :doc:`readability-magic-numbers
203203
<clang-tidy/checks/readability/magic-numbers>` check, now allows for
204204
magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
205205
the new ``IgnoreTypeAliases`` option is set to true.
206206

207+
- Fixed a false positive in :doc:`cppcoreguidelines-slicing
208+
<clang-tidy/checks/cppcoreguidelines/slicing>` check when warning would be
209+
emitted in constructor for virtual base class initialization.
210+
207211
Removed checks
208212
^^^^^^^^^^^^^^
209213

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/slicing.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,25 @@ void negatives() {
9898
a = h;
9999
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: slicing object from type 'DerivedThatOverridesH' to 'Base' discards override 'h'
100100
}
101+
102+
namespace PR31187 {
103+
// Don't warn when calling constructor of base virtual class, from
104+
// initialization list of derived class constructor.
105+
106+
struct BaseA {
107+
virtual ~BaseA() {}
108+
virtual void foo() {}
109+
110+
int i;
111+
};
112+
113+
struct BaseB : virtual BaseA {
114+
virtual void foo() {}
115+
};
116+
117+
struct ClassWithVirtualBases : BaseB {
118+
ClassWithVirtualBases(const BaseB& other) : BaseA(other), BaseB(other) {}
119+
ClassWithVirtualBases(const ClassWithVirtualBases& other) : BaseA(other), BaseB(other) {}
120+
};
121+
122+
}

0 commit comments

Comments
 (0)