[Clang] [Sema] Do not attempt to dump the layout of dependent types when -fdump-record-layouts-complete is passed

[Sirraide]

We were crashing on trying to print the layout of an uninstantiated template, which obviously doesn’t make sense.

This fixes #83684.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (Sirraide)

Changes

We were crashing on trying to print the layout of an uninstantiated template, which obviously doesn’t make sense.

This fixes #83684.

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+4)
• (modified) clang/lib/AST/Decl.cpp (+1-1)
• (modified) clang/test/Layout/dump-complete.cpp (+28-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f44fef28b9f17f..69cf0cb643e6aa 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -308,6 +308,10 @@ Miscellaneous Bug Fixes
 Miscellaneous Clang Crashes Fixed
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Do not attempt to dump the layout of dependent types when ``-fdump-record-layouts-complete``
+  is passed.
+  Fixes (`#83684 <https://github.com/llvm/llvm-project/issues/83684>`_).
+
 OpenACC Specific Changes
 ------------------------
 
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 5d6bb72a208a1a..d069cd65732310 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5042,7 +5042,7 @@ void RecordDecl::completeDefinition() {
 
   // Layouts are dumped when computed, so if we are dumping for all complete
   // types, we need to force usage to get types that wouldn't be used elsewhere.
-  if (Ctx.getLangOpts().DumpRecordLayoutsComplete)
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete && !isDependentType())
     (void)Ctx.getASTRecordLayout(this);
 }
 
diff --git a/clang/test/Layout/dump-complete.cpp b/clang/test/Layout/dump-complete.cpp
index 9ccbf477c7052e..0a91d3329e6921 100644
--- a/clang/test/Layout/dump-complete.cpp
+++ b/clang/test/Layout/dump-complete.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fdump-record-layouts-complete %s | FileCheck %s
 
 struct a {
   int x;
@@ -12,7 +12,34 @@ class c {};
 
 class d;
 
+template <typename>
+struct s {
+  int x;
+};
+
+template <typename T>
+struct ts {
+  T x;
+};
+
+void f() {
+  ts<int> a;
+  ts<double> b;
+}
+
+namespace gh83684 {
+template <class Pointer>
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:          0 | struct a
 // CHECK:          0 | struct b
 // CHECK:          0 | class c
+// CHECK:          0 | struct ts<int>
+// CHECK:          0 | struct ts<double>
 // CHECK-NOT:      0 | class d
+// CHECK-NOT:      0 | struct s
+// CHECK-NOT:      0 | struct AllocationResult

[shafik]

I think this makes sense but I would like another set of eyes.

[Endilll]

This might also fix #83671

[Sirraide]

This might also fix #83671

From what I saw, that’s a different problem, and there’s a different pr for that already, but I’ll double-check.

[Sirraide]

Oh, apparently, there’s yet another problem here; it seems we should also check for isInvalidDecl().

[bjrjk]

This might also fix #83671

From what I saw, that’s a different problem, and there’s a different pr for that already, but I’ll double-check.

Seems not same problem, root cause are different.

[Sirraide]

This might also fix #83671

Actually, it seems to fix your reduced test case, hmm...

[Sirraide]

So yeah, this pr indeed also seems to fix #83671 as well.

[AaronBallman]

LGTM!

[Sirraide]

Apparently, buildbot is mad at me (https://lab.llvm.org/buildbot/#/builders/165/builds/49741)?

I’m not entirely sure how a fix that affects only -fdump-record-layouts-complete could have caused this. Is this a spurious failure or due to something unrelated, or is it worth investigating this?

[Endilll]

Apparently, buildbot is mad at me (https://lab.llvm.org/buildbot/#/builders/165/builds/49741)?

I’m not entirely sure how a fix that affects only -fdump-record-layouts-complete could have caused this. Is this a spurious failure or due to something unrelated, or is it worth investigating this?

That builder seems happy testing main after your commit: https://lab.llvm.org/buildbot/#/builders/165, so I'd write it off as spurious.