Fix printing of templated records. #86339

zahiraam · 2024-03-22T20:57:15Z

Fixed the printing of templated argument list and added test case.

github-actions · 2024-03-22T20:59:51Z

✅ With the latest revision this PR passed the C/C++ code formatter.

github-actions · 2024-03-22T20:59:51Z

✅ With the latest revision this PR passed the Python code formatter.

llvmbot · 2024-03-25T13:58:22Z

@llvm/pr-subscribers-clang

Author: Zahira Ammarguellat (zahiraam)

Changes

Fixed the printing of templated argument list and added test case.

Full diff: https://github.com/llvm/llvm-project/pull/86339.diff

2 Files Affected:

(modified) clang/lib/AST/TypePrinter.cpp (-5)
(modified) clang/unittests/AST/DeclPrinterTest.cpp (+69)

diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 7032ff2f18468c..d9504f9dcb3899 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2303,11 +2303,6 @@ printTo(raw_ostream &OS, ArrayRef<TA> Args, const PrintingPolicy &Policy,
     } else {
       if (!FirstArg)
         OS << Comma;
-      if (!Policy.SuppressTagKeyword &&
-          Argument.getKind() == TemplateArgument::Type &&
-          isa<TagType>(Argument.getAsType()))
-        OS << Argument.getAsType().getAsString();
-      else
         // Tries to print the argument with location info if exists.
         printArgument(Arg, Policy, ArgOS,
                       TemplateParameterList::shouldIncludeTypeForArgument(
diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp
index e024c41e03b484..07fa02bd96e25d 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -1386,6 +1386,75 @@ TEST(DeclPrinter, TestTemplateArgumentList16) {
   ASSERT_TRUE(PrintedDeclCXX11Matches(Code, "NT2", "int NT2 = 5"));
 }
 
+TEST(DeclPrinter, TestCXXRecordDecl17) {
+  ASSERT_TRUE(PrintedDeclCXX98Matches("template<typename T> struct Z {};"
+                                      "struct X {};"
+                                      "Z<X> A;",
+                                      "A", "Z<X> A"));
+  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
+}
+
+TEST(DeclPrinter, TestCXXRecordDecl18) {
+  ASSERT_TRUE(PrintedDeclCXX98Matches("template<typename T> struct Z {};"
+                                      "struct X {};"
+                                      "Z<X> A;"
+                                      "template <typename T1, int>"
+                                      "struct Y{};"
+                                      "Y<Z<X>, 2> B;",
+                                      "B", "Y<Z<X>, 2> B"));
+  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
+}
+
+TEST(DeclPrinter, TestCXXRecordDecl19) {
+  ASSERT_TRUE(PrintedDeclCXX98Matches("template<typename T> struct Z {};"
+                                      "struct X {};"
+                                      "Z<X> A;"
+                                      "template <typename T1, int>"
+                                      "struct Y{};"
+                                      "Y<Z<X>, 2> B;",
+                                      "B", "Y<Z<X>, 2> B"));
+  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; };
+}
+TEST(DeclPrinter, TestCXXRecordDecl20) {
+  ASSERT_TRUE(PrintedDeclCXX98Matches(
+      "template <typename T, int N> class Inner;"
+      "template <typename T, int N>"
+      "class Inner{Inner(T val){}};"
+      "template <class InnerClass, int N> class Outer {"
+      "public:"
+      "struct NestedStruct {"
+      "int nestedValue;"
+      "NestedStruct(int val) : nestedValue(val) {}"
+      "};"
+      "InnerClass innerInstance;"
+      "Outer(const InnerClass &inner) : innerInstance(inner) {}"
+      "};"
+      "Outer<Inner<int, 10>, 5>::NestedStruct nestedInstance(100);",
+      "nestedInstance",
+      "Outer<Inner<int, 10>, 5>::NestedStruct nestedInstance(100)"));
+  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
+}
+
+TEST(DeclPrinter, TestCXXRecordDecl21) {
+  ASSERT_TRUE(PrintedDeclCXX98Matches(
+      "template <typename T, int N> class Inner;"
+      "template <typename T, int N>"
+      "class Inner{Inner(T val){}};"
+      "template <class InnerClass, int N> class Outer {"
+      "public:"
+      "struct NestedStruct {"
+      "int nestedValue;"
+      "NestedStruct(int val) : nestedValue(val) {}"
+      "};"
+      "InnerClass innerInstance;"
+      "Outer(const InnerClass &inner) : innerInstance(inner) {}"
+      "};"
+      "Outer<Inner<int, 10>, 5>::NestedStruct nestedInstance(100);",
+      "nestedInstance",
+      "Outer<Inner<int, 10>, 5>::NestedStruct nestedInstance(100)"));
+  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; };
+}
+
 TEST(DeclPrinter, TestFunctionParamUglified) {
   llvm::StringLiteral Code = R"cpp(
     class __c;

zahiraam · 2024-03-26T10:47:43Z

clang/lib/AST/TypePrinter.cpp

-          Argument.getKind() == TemplateArgument::Type &&
-          isa<TagType>(Argument.getAsType()))
-        OS << Argument.getAsType().getAsString();
-      else


This code was wrongly added in #84014 and was not reached by any tests. Some of our internal testing did hit this and realized the code was wrong. Created reproducers from internal tests to show that the code is wrong.

AaronBallman

Good catch, LGTM!

zahiraam · 2024-03-26T13:06:41Z

@AaronBallman Thanks!

kazutakahirata · 2024-03-26T18:01:18Z

I'm getting errors like:

llvm-project/clang/unittests/AST/DeclPrinterTest.cpp:1394:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };

This file being a clang test, I am wondering if this is actually part of the test. Shall we put [[maybe_unused]] here?

zahiraam · 2024-03-26T18:50:59Z

I'm getting errors like:
llvm-project/clang/unittests/AST/DeclPrinterTest.cpp:1394:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
This file being a clang test, I am wondering if this is actually part of the test. Shall we put [[maybe_unused]] here?

@kazutakahirata When do you get this error?

zahiraam · 2024-03-26T19:04:15Z

Creating a PR with
[](PrintingPolicy [[maybe_unused]] & Policy) { Policy.SuppressTagKeyword = true; };
on those tests.

kazutakahirata · 2024-03-26T19:52:49Z

Creating a PR with [](PrintingPolicy [[maybe_unused]] & Policy) { Policy.SuppressTagKeyword = true; }; on those tests.

I get this error while running ninja check-clang-unit. My build tree is configured with -DCMAKE_CXX_COMPILER=/usr/bin/clang++ and -DLLVM_ENABLE_WERROR=On.

By the way:

[](PrintingPolicy [[maybe_unused]] &Policy) { Policy.SuppressTagKeyword = false; };

would result in:

clang/unittests/AST/DeclPrinterTest.cpp:1394:23: error: 'maybe_unused' attribute cannot be applied to types

[[maybe_unused]] [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };

would result in:

clang/unittests/AST/DeclPrinterTest.cpp:1394:5: error: 'maybe_unused' attribute cannot be applied to a statement

I don't know where to put [[maybe_unused]].

kazutakahirata · 2024-03-26T19:54:39Z

Looking at other examples, should the lambda expression be the last argument to PrintedDeclCXX98Matches?

amy-kwan · 2024-03-26T20:14:46Z

This also fails on PPC, as well: https://lab.llvm.org/buildbot/#/builders/36/builds/43998/steps/12/logs/stdio

zahiraam · 2024-03-26T20:22:50Z

oh! I think I know what the issue is. I have a fix for it. PR following up.
It should be:
TEST(DeclPrinter, TestCXXRecordDecl17) { ASSERT_TRUE(PrintedDeclCXX98Matches("template<typename T> struct Z {};" "struct X {};" "Z<X> A;", "A", "Z<X> A", [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; })); }
instead of:
TEST(DeclPrinter, TestCXXRecordDecl17) { ASSERT_TRUE(PrintedDeclCXX98Matches("template<typename T> struct Z {};" "struct X {};" "Z<X> A;", "A", "Z<X> A")); [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };

See the )).

mikaelholmen · 2024-03-27T06:22:38Z

@zahiraam I still get

../../clang/unittests/AST/DeclPrinterTest.cpp:1394:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1405:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1416:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1435:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1455:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5 errors generated.

This has been broken for quite a while now, can you please fix or revert?

mikaelholmen · 2024-03-27T07:32:47Z

@zahiraam I still get

../../clang/unittests/AST/DeclPrinterTest.cpp:1394:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1405:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1416:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1435:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = false; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../clang/unittests/AST/DeclPrinterTest.cpp:1455:3: error: expression result unused [-Werror,-Wunused-value]
  [](PrintingPolicy &Policy) { Policy.SuppressTagKeyword = true; };
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5 errors generated.

This has been broken for quite a while now, can you please fix or revert?

Thank you @alinas for fixing the problem in 577e0ef !

Fix printing of templated records.

ab7d280

Fixed format and added tests.

b1623ca

zahiraam marked this pull request as ready for review March 25, 2024 13:57

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 25, 2024

zahiraam requested a review from AaronBallman March 25, 2024 13:58

zahiraam commented Mar 26, 2024

View reviewed changes

AaronBallman approved these changes Mar 26, 2024

View reviewed changes

zahiraam merged commit 3a2c70b into llvm:main Mar 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix printing of templated records. #86339

Fix printing of templated records. #86339

Uh oh!

zahiraam commented Mar 22, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Mar 22, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Mar 22, 2024

Uh oh!

llvmbot commented Mar 25, 2024

Uh oh!

zahiraam Mar 26, 2024 •

edited

Loading

Uh oh!

AaronBallman left a comment

Uh oh!

zahiraam commented Mar 26, 2024

Uh oh!

kazutakahirata commented Mar 26, 2024

Uh oh!

zahiraam commented Mar 26, 2024

Uh oh!

zahiraam commented Mar 26, 2024

Uh oh!

kazutakahirata commented Mar 26, 2024

Uh oh!

kazutakahirata commented Mar 26, 2024 •

edited

Loading

Uh oh!

amy-kwan commented Mar 26, 2024

Uh oh!

zahiraam commented Mar 26, 2024 •

edited

Loading

Uh oh!

mikaelholmen commented Mar 27, 2024

Uh oh!

mikaelholmen commented Mar 27, 2024

Uh oh!

Uh oh!

Fix printing of templated records. #86339

Fix printing of templated records. #86339

Uh oh!

Conversation

zahiraam commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 22, 2024

Uh oh!

llvmbot commented Mar 25, 2024

Uh oh!

zahiraam Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

zahiraam commented Mar 26, 2024

Uh oh!

kazutakahirata commented Mar 26, 2024

Uh oh!

zahiraam commented Mar 26, 2024

Uh oh!

zahiraam commented Mar 26, 2024

Uh oh!

kazutakahirata commented Mar 26, 2024

Uh oh!

kazutakahirata commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amy-kwan commented Mar 26, 2024

Uh oh!

zahiraam commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikaelholmen commented Mar 27, 2024

Uh oh!

mikaelholmen commented Mar 27, 2024

Uh oh!

Uh oh!

zahiraam commented Mar 22, 2024 •

edited

Loading

github-actions bot commented Mar 22, 2024 •

edited

Loading

zahiraam Mar 26, 2024 •

edited

Loading

kazutakahirata commented Mar 26, 2024 •

edited

Loading

zahiraam commented Mar 26, 2024 •

edited

Loading