carlosgalvezp requested changes to this revision.
carlosgalvezp added a comment.
This revision now requires changes to proceed.
Amazing, thanks a lot for fixing this long-standing issue! I have a couple
comments. I'm not familiar at all with the Sema part so someone that knows
should look at it :)
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:374
{
PositiveSelfInitialization() : PositiveSelfInitialization() {}
};
----------------
Not really sure what this test is meant to do. Why would it call the destructor
of the own class in it's own constructor? Looks very strange to me.
If anything, the constructor should call the constructor of the base:
PositiveSelfInitialization() : NegativeAggregateType()
What do you think?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:555
+// Check that a delegating constructor in a template does not trigger false
positives (PR#37902).
+template <typename>
+struct TemplateWithDelegatingCtor {
----------------
typename T?
Not sure if it makes a difference
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp:559
+ TemplateWithDelegatingCtor() : X{} {};
+ TemplateWithDelegatingCtor(int) : TemplateWithDelegatingCtor(){};
+};
----------------
Could you add another test where X is initialized via NSDMI instead of in the
constructor initializer list?
int X{};
TemplateWithDelegatingCtor() = default;
TemplateWithDelegatingCtor(int) : TemplateWithDelegatingCtor() {}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113518/new/
https://reviews.llvm.org/D113518
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
