[clang][DeclPrinter] Fix AST print of delegating constructors

[strimo378]

Summary

DeclPrinter::PrintConstructorInitializers did not consider delegating initializers. As result, the output contained an "NULL TYPE" for delegating constructors.

I chose a generic test case name (ast-print-method-decl.cpp). The plan is to add there other method related test cases for fixes I have in my local branch.

Please use "Timo Stripf" and [e-mail] for commit.

[aaron.ballman]

Thank you for the fix! This generally LGTM, but I did find some extra test cases to add.

clang/test/AST/ast-print-method-decl.cpp

2

I think the file should be named ast-print-delegating-constructor.cpp, WDYT?

16

I'd also like to see test cases along the lines of:

struct B {
  template <typename Ty>
  B(Ty);
  B(int X) : B((float)X) {}
};

struct C {
  C(auto);
  C(int) : C("") {}
};

[strimo378]

clang/test/AST/ast-print-method-decl.cpp

16

I fully agree to you that these test cases should also work but that goes beyond the scope of a "simple" delegating ctor output fix.

In the current implementation all implicitly specialized templates are output and therefore the generated code is not correct C++ anymore, see https://godbolt.org/z/34xaoYW5n . I think it is not a good idea to check for incorrect C++ code in a test case...

I am currently focusing to get non-templated C++ code output correctly. For methods e.g. there are many problems with the correct position of attributes. I will address that in one of my next merge request and I want to put all method related tests into ast-print-method-decl.cpp . Therefore, I would like to keep the name.

[aaron.ballman]

clang/test/AST/ast-print-method-decl.cpp

16

I fully agree to you that these test cases should also work but that goes beyond the scope of a "simple" delegating ctor output fix.

Ah, so this is exposing an already existing issue. In that case, let's add the tests and leave a FIXME comment to improve the behavior in the future (bonus points for filing an issue) so that it's clear we've noticed there's a problem rather than looking like an oversight in test coverage. This also ensures that those forms of delegation don't cause assertions or crashes despite being wrong.

I am currently focusing to get non-templated C++ code output correctly. For methods e.g. there are many problems with the correct position of attributes. I will address that in one of my next merge request and I want to put all method related tests into ast-print-method-decl.cpp . Therefore, I would like to keep the name.

Ah, if there's more functionality to be added to the test in the near future, the current name is fine by me.

[strimo378]

Updated test case

[strimo378]

Ping

[aaron.ballman]

LGTM! Do you need me to commit this on your behalf? (If you plan to do more patches, I'm also happy to let you ask for commit privileges [https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access] and land this yourself if you'd like.)

[strimo378]

I plan around half a dozen patches only for ast-print functionality. I would love to land it myself and will asked for commit privileges.

clang/lib/AST/DeclPrinter.cpp

if (!HasInitializerList) {
       Proto.clear();
       HasInitializerList = true;
     } else
       Out << ", ";
 
     if (BMInitializer->isAnyMemberInitializer()) {
       FieldDecl *FD = BMInitializer->getAnyMember();
       Out << *FD;
+    } else if (BMInitializer->isDelegatingInitializer()) {
+      Out << CDecl->getNameAsString();
     } else {
       Out << QualType(BMInitializer->getBaseClass(), 0).getAsString(Policy);
     }
 
     Out << "(";
     if (!BMInitializer->getInit()) {
       // Nothing to print
     } else {

clang/test/AST/ast-print-method-decl.cpp

+// RUN: %clang_cc1 -ast-print %s -o - -std=c++20 | FileCheck %s
+
+// CHECK: struct A {
+struct A {
+  // CHECK-NEXT: A();
+  A();
+
+  // CHECK-NEXT: A(int) : A() {
+  A(int) : A() {
+    // CHECK-NEXT: }
+  }
+
+  // CHECK-NEXT: };
+};
+
+
+// CHECK: struct B {
+struct B {
+  // CHECK-NEXT: template <typename Ty> B(Ty);
+  template <typename Ty> B(Ty);
+
+  // FIXME: Implicitly specialized method should not be output
+  // CHECK-NEXT: template<> B<float>(float);
+
+  // CHECK-NEXT: B(int X) : B((float)X) {
+  B(int X) : B((float)X) {
+  // CHECK-NEXT: }
+  }
+
+  // CHECK-NEXT: };
+};
+
+// CHECK: struct C {
+struct C {
+  // FIXME: template <> should not be output
+  // CHECK: template <> C(auto);
+  C(auto);
+
+  // FIXME: Implicitly specialized method should not be output
+  // CHECK: template<> C<const char *>(const char *);
+
+  // CHECK: C(int) : C("") {
+  C(int) : C("") {
+  // CHECK-NEXT: }
+  }
+
+  // CHECK-NEXT: };
+};