[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-04 Thread Vlad Serebrennikov via cfe-commits 

Endilll wrote:

> 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.

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-04 Thread via cfe-commits 

Sirraide wrote:

Apparently, buildbot is mad at me 
()?

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?

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-04 Thread via cfe-commits 

https://github.com/Sirraide closed 
https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-04 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman approved this pull request.

LGTM!

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

Sirraide wrote:

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

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

https://github.com/Sirraide updated 
https://github.com/llvm/llvm-project/pull/83688

>From 4abc148104769bd756042c73edeb7ee54397a05f Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:41:22 +0100
Subject: [PATCH 1/6] [Clang] [Sema] Do not attempt to dump the layout of
 dependent types

---
 clang/lib/AST/Decl.cpp  |  2 +-
 clang/test/Layout/dump-complete.cpp | 29 -
 2 files changed, 29 insertions(+), 2 deletions(-)

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 
+struct s {
+  int x;
+};
+
+template 
+struct ts {
+  T x;
+};
+
+void f() {
+  ts a;
+  ts b;
+}
+
+namespace gh83684 {
+template 
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d
+// CHECK-NOT:  0 | struct s
+// CHECK-NOT:  0 | struct AllocationResult

>From 3056a8414cae4648a3cc8244cacef29e6dc00564 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:45:24 +0100
Subject: [PATCH 2/6] [Clang] Add release note

---
 clang/docs/ReleaseNotes.rst | 4 
 1 file changed, 4 insertions(+)

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 `_).
+
 OpenACC Specific Changes
 
 

>From cdf14b7cef9e08d932b649dc2724362aeea56a40 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sun, 3 Mar 2024 10:44:38 +0100
Subject: [PATCH 3/6] [Clang] Add comment and test case for explicit
 specialisation

---
 clang/lib/AST/Decl.cpp  | 3 +++
 clang/test/Layout/dump-complete.cpp | 8 
 2 files changed, 11 insertions(+)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index d069cd65732310..a3e4e13ffdc74d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5042,6 +5042,9 @@ 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 the type is dependent, then we can't compute its layout because there
+  // is no way for us to know the size or alignment of a dependent type.
   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 0a91d3329e6921..7fed145084dccb 100644
--- a/clang/test/Layout/dump-complete.cpp
+++ b/clang/test/Layout/dump-complete.cpp
@@ -22,9 +22,15 @@ struct ts {
   T x;
 };
 
+template <>
+struct ts {
+  float f;
+};
+
 void f() {
   ts a;
   ts b;
+  ts c;
 }
 
 namespace gh83684 {
@@ -38,6 +44,8 @@ struct AllocationResult {
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK-NEXT: 0 |   float
 // CHECK:  0 | struct ts
 // CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d

>From ac9b16aa4a26a9926e221937fc48150640acd09e Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sun, 3 Mar 2024 11:16:04 +0100
Subject: [PATCH 4/6] [Clang] Don't try to print the layout of an invalid decl

---
 clang/lib/AST/Decl.cpp  | 7 +--
 clang/test/Layout/dump-complete-invalid.cpp | 6 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Layout/dump-complete-invalid.cpp

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

Sirraide wrote:

> This might also fix #83671

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

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

https://github.com/Sirraide updated 
https://github.com/llvm/llvm-project/pull/83688

>From 4abc148104769bd756042c73edeb7ee54397a05f Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:41:22 +0100
Subject: [PATCH 1/5] [Clang] [Sema] Do not attempt to dump the layout of
 dependent types

---
 clang/lib/AST/Decl.cpp  |  2 +-
 clang/test/Layout/dump-complete.cpp | 29 -
 2 files changed, 29 insertions(+), 2 deletions(-)

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 
+struct s {
+  int x;
+};
+
+template 
+struct ts {
+  T x;
+};
+
+void f() {
+  ts a;
+  ts b;
+}
+
+namespace gh83684 {
+template 
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d
+// CHECK-NOT:  0 | struct s
+// CHECK-NOT:  0 | struct AllocationResult

>From 3056a8414cae4648a3cc8244cacef29e6dc00564 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:45:24 +0100
Subject: [PATCH 2/5] [Clang] Add release note

---
 clang/docs/ReleaseNotes.rst | 4 
 1 file changed, 4 insertions(+)

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 `_).
+
 OpenACC Specific Changes
 
 

>From cdf14b7cef9e08d932b649dc2724362aeea56a40 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sun, 3 Mar 2024 10:44:38 +0100
Subject: [PATCH 3/5] [Clang] Add comment and test case for explicit
 specialisation

---
 clang/lib/AST/Decl.cpp  | 3 +++
 clang/test/Layout/dump-complete.cpp | 8 
 2 files changed, 11 insertions(+)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index d069cd65732310..a3e4e13ffdc74d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5042,6 +5042,9 @@ 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 the type is dependent, then we can't compute its layout because there
+  // is no way for us to know the size or alignment of a dependent type.
   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 0a91d3329e6921..7fed145084dccb 100644
--- a/clang/test/Layout/dump-complete.cpp
+++ b/clang/test/Layout/dump-complete.cpp
@@ -22,9 +22,15 @@ struct ts {
   T x;
 };
 
+template <>
+struct ts {
+  float f;
+};
+
 void f() {
   ts a;
   ts b;
+  ts c;
 }
 
 namespace gh83684 {
@@ -38,6 +44,8 @@ struct AllocationResult {
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK-NEXT: 0 |   float
 // CHECK:  0 | struct ts
 // CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d

>From ac9b16aa4a26a9926e221937fc48150640acd09e Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sun, 3 Mar 2024 11:16:04 +0100
Subject: [PATCH 4/5] [Clang] Don't try to print the layout of an invalid decl

---
 clang/lib/AST/Decl.cpp  | 7 +--
 clang/test/Layout/dump-complete-invalid.cpp | 6 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Layout/dump-complete-invalid.cpp

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

https://github.com/Sirraide updated 
https://github.com/llvm/llvm-project/pull/83688

>From 4abc148104769bd756042c73edeb7ee54397a05f Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:41:22 +0100
Subject: [PATCH 1/4] [Clang] [Sema] Do not attempt to dump the layout of
 dependent types

---
 clang/lib/AST/Decl.cpp  |  2 +-
 clang/test/Layout/dump-complete.cpp | 29 -
 2 files changed, 29 insertions(+), 2 deletions(-)

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 
+struct s {
+  int x;
+};
+
+template 
+struct ts {
+  T x;
+};
+
+void f() {
+  ts a;
+  ts b;
+}
+
+namespace gh83684 {
+template 
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d
+// CHECK-NOT:  0 | struct s
+// CHECK-NOT:  0 | struct AllocationResult

>From 3056a8414cae4648a3cc8244cacef29e6dc00564 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:45:24 +0100
Subject: [PATCH 2/4] [Clang] Add release note

---
 clang/docs/ReleaseNotes.rst | 4 
 1 file changed, 4 insertions(+)

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 `_).
+
 OpenACC Specific Changes
 
 

>From cdf14b7cef9e08d932b649dc2724362aeea56a40 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sun, 3 Mar 2024 10:44:38 +0100
Subject: [PATCH 3/4] [Clang] Add comment and test case for explicit
 specialisation

---
 clang/lib/AST/Decl.cpp  | 3 +++
 clang/test/Layout/dump-complete.cpp | 8 
 2 files changed, 11 insertions(+)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index d069cd65732310..a3e4e13ffdc74d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5042,6 +5042,9 @@ 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 the type is dependent, then we can't compute its layout because there
+  // is no way for us to know the size or alignment of a dependent type.
   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 0a91d3329e6921..7fed145084dccb 100644
--- a/clang/test/Layout/dump-complete.cpp
+++ b/clang/test/Layout/dump-complete.cpp
@@ -22,9 +22,15 @@ struct ts {
   T x;
 };
 
+template <>
+struct ts {
+  float f;
+};
+
 void f() {
   ts a;
   ts b;
+  ts c;
 }
 
 namespace gh83684 {
@@ -38,6 +44,8 @@ struct AllocationResult {
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK-NEXT: 0 |   float
 // CHECK:  0 | struct ts
 // CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d

>From ac9b16aa4a26a9926e221937fc48150640acd09e Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sun, 3 Mar 2024 11:16:04 +0100
Subject: [PATCH 4/4] [Clang] Don't try to print the layout of an invalid decl

---
 clang/lib/AST/Decl.cpp  | 7 +--
 clang/test/Layout/dump-complete-invalid.cpp | 6 ++
 2 files changed, 11 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Layout/dump-complete-invalid.cpp

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread Jack Ren via cfe-commits 

bjrjk wrote:

> > 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.

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

Sirraide wrote:

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

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

Sirraide wrote:

> 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.

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread Vlad Serebrennikov via cfe-commits 

Endilll wrote:

This might also fix #83671

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 

https://github.com/Sirraide updated 
https://github.com/llvm/llvm-project/pull/83688

>From 4abc148104769bd756042c73edeb7ee54397a05f Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:41:22 +0100
Subject: [PATCH 1/3] [Clang] [Sema] Do not attempt to dump the layout of
 dependent types

---
 clang/lib/AST/Decl.cpp  |  2 +-
 clang/test/Layout/dump-complete.cpp | 29 -
 2 files changed, 29 insertions(+), 2 deletions(-)

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 
+struct s {
+  int x;
+};
+
+template 
+struct ts {
+  T x;
+};
+
+void f() {
+  ts a;
+  ts b;
+}
+
+namespace gh83684 {
+template 
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d
+// CHECK-NOT:  0 | struct s
+// CHECK-NOT:  0 | struct AllocationResult

>From 3056a8414cae4648a3cc8244cacef29e6dc00564 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:45:24 +0100
Subject: [PATCH 2/3] [Clang] Add release note

---
 clang/docs/ReleaseNotes.rst | 4 
 1 file changed, 4 insertions(+)

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 `_).
+
 OpenACC Specific Changes
 
 

>From cdf14b7cef9e08d932b649dc2724362aeea56a40 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sun, 3 Mar 2024 10:44:38 +0100
Subject: [PATCH 3/3] [Clang] Add comment and test case for explicit
 specialisation

---
 clang/lib/AST/Decl.cpp  | 3 +++
 clang/test/Layout/dump-complete.cpp | 8 
 2 files changed, 11 insertions(+)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index d069cd65732310..a3e4e13ffdc74d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5042,6 +5042,9 @@ 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 the type is dependent, then we can't compute its layout because there
+  // is no way for us to know the size or alignment of a dependent type.
   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 0a91d3329e6921..7fed145084dccb 100644
--- a/clang/test/Layout/dump-complete.cpp
+++ b/clang/test/Layout/dump-complete.cpp
@@ -22,9 +22,15 @@ struct ts {
   T x;
 };
 
+template <>
+struct ts {
+  float f;
+};
+
 void f() {
   ts a;
   ts b;
+  ts c;
 }
 
 namespace gh83684 {
@@ -38,6 +44,8 @@ struct AllocationResult {
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK-NEXT: 0 |   float
 // CHECK:  0 | struct ts
 // CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-03 Thread via cfe-commits 


@@ -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())

Sirraide wrote:

Sure; I was going to do that but I felt it wasn’t really ‘necessary’, but it 
can’t hurt.

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread Shafik Yaghmour via cfe-commits 


@@ -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())

shafik wrote:

Can you update the comment above to reflect this change.

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread Shafik Yaghmour via cfe-commits 

https://github.com/shafik commented:

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

https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread Shafik Yaghmour via cfe-commits 

https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread via cfe-commits 

llvmbot wrote:




@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
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 `_).
+
 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 
+struct s {
+  int x;
+};
+
+template 
+struct ts {
+  T x;
+};
+
+void f() {
+  ts a;
+  ts b;
+}
+
+namespace gh83684 {
+template 
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d
+// CHECK-NOT:  0 | struct s
+// CHECK-NOT:  0 | struct AllocationResult

``




https://github.com/llvm/llvm-project/pull/83688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

2024-03-02 Thread via cfe-commits 

https://github.com/Sirraide created 
https://github.com/llvm/llvm-project/pull/83688

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

This fixes #83684.

>From 4abc148104769bd756042c73edeb7ee54397a05f Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:41:22 +0100
Subject: [PATCH 1/2] [Clang] [Sema] Do not attempt to dump the layout of
 dependent types

---
 clang/lib/AST/Decl.cpp  |  2 +-
 clang/test/Layout/dump-complete.cpp | 29 -
 2 files changed, 29 insertions(+), 2 deletions(-)

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 
+struct s {
+  int x;
+};
+
+template 
+struct ts {
+  T x;
+};
+
+void f() {
+  ts a;
+  ts b;
+}
+
+namespace gh83684 {
+template 
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:  0 | struct a
 // CHECK:  0 | struct b
 // CHECK:  0 | class c
+// CHECK:  0 | struct ts
+// CHECK:  0 | struct ts
 // CHECK-NOT:  0 | class d
+// CHECK-NOT:  0 | struct s
+// CHECK-NOT:  0 | struct AllocationResult

>From 3056a8414cae4648a3cc8244cacef29e6dc00564 Mon Sep 17 00:00:00 2001
From: Sirraide 
Date: Sat, 2 Mar 2024 20:45:24 +0100
Subject: [PATCH 2/2] [Clang] Add release note

---
 clang/docs/ReleaseNotes.rst | 4 
 1 file changed, 4 insertions(+)

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 `_).
+
 OpenACC Specific Changes
 
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits