[PATCH] D113518: [clang] Create delegating constructors even in templates

[Carlos Galvez via Phabricator via cfe-commits]

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