[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-22 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

a8bf33ad36837b398d3dabfd5fff1142660a1dca 
 fixed 
this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104484/new/

https://reviews.llvm.org/D104484

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

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-22 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Probably the "need to bump AST serialization Version after adding a langopt" 
thing again?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104484/new/

https://reviews.llvm.org/D104484

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

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-22 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

This breaks check-clang: http://45.33.8.238/linux/49502/step_7.txt

Please take a look and revert for now if it takes a while to fix. It's been a 
pretty rough day for the tree.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104484/new/

https://reviews.llvm.org/D104484

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

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-22 Thread David Tenty via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7942ebdf01b3: [clang] Add cc1 option for dumping layout for 
all complete types (authored by daltenty).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104484/new/

https://reviews.llvm.org/D104484

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Decl.cpp
  clang/test/Layout/dump-complete.cpp


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | 
FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,13 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext  = getASTContext();
+
+  // 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)
+(void)Ctx.getASTRecordLayout(this);
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5381,10 +5381,13 @@
 def fdump_record_layouts_simple : Flag<["-"], "fdump-record-layouts-simple">,
   HelpText<"Dump record layout information in a simple form used for testing">,
   MarshallingInfoFlag>;
+def fdump_record_layouts_complete : Flag<["-"], 
"fdump-record-layouts-complete">,
+  HelpText<"Dump record layout information for all complete types">,
+  MarshallingInfoFlag>;
 def fdump_record_layouts : Flag<["-"], "fdump-record-layouts">,
   HelpText<"Dump record layout information">,
   MarshallingInfoFlag>,
-  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath]>;
+  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath, 
fdump_record_layouts_complete.KeyPath]>;
 def fix_what_you_can : Flag<["-"], "fix-what-you-can">,
   HelpText<"Apply fix-it advice even in the presence of unfixable errors">,
   MarshallingInfoFlag>;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -265,6 +265,7 @@
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
 BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd 
records")
 BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd 
records in a simple form")
+BENIGN_LANGOPT(DumpRecordLayoutsComplete , 1, 0, "dumping the AST layout of 
all complete records")
 BENIGN_LANGOPT(DumpVTableLayouts , 1, 0, "dumping the layouts of emitted 
vtables")
 LANGOPT(NoConstantCFStrings , 1, 0, "no constant CoreFoundation strings")
 BENIGN_LANGOPT(InlineVisibilityHidden , 1, 0, "hidden visibility for inline 
C++ methods")


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,13 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext  = getASTContext();
+
+  // 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)
+(void)Ctx.getASTRecordLayout(this);
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5381,10 +5381,13 @@
 def fdump_record_layouts_simple : Flag<["-"],

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104484/new/

https://reviews.llvm.org/D104484

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

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-22 Thread David Tenty via Phabricator via cfe-commits 
daltenty updated this revision to Diff 353636.
daltenty added a comment.

- clang-format
- Address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104484/new/

https://reviews.llvm.org/D104484

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Decl.cpp
  clang/test/Layout/dump-complete.cpp


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | 
FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,13 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext  = getASTContext();
+
+  // 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)
+(void)Ctx.getASTRecordLayout(this);
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5371,10 +5371,13 @@
 def fdump_record_layouts_simple : Flag<["-"], "fdump-record-layouts-simple">,
   HelpText<"Dump record layout information in a simple form used for testing">,
   MarshallingInfoFlag>;
+def fdump_record_layouts_complete : Flag<["-"], 
"fdump-record-layouts-complete">,
+  HelpText<"Dump record layout information for all complete types">,
+  MarshallingInfoFlag>;
 def fdump_record_layouts : Flag<["-"], "fdump-record-layouts">,
   HelpText<"Dump record layout information">,
   MarshallingInfoFlag>,
-  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath]>;
+  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath, 
fdump_record_layouts_complete.KeyPath]>;
 def fix_what_you_can : Flag<["-"], "fix-what-you-can">,
   HelpText<"Apply fix-it advice even in the presence of unfixable errors">,
   MarshallingInfoFlag>;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -266,6 +266,7 @@
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
 BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd 
records")
 BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd 
records in a simple form")
+BENIGN_LANGOPT(DumpRecordLayoutsComplete , 1, 0, "dumping the AST layout of 
all complete records")
 BENIGN_LANGOPT(DumpVTableLayouts , 1, 0, "dumping the layouts of emitted 
vtables")
 LANGOPT(NoConstantCFStrings , 1, 0, "no constant CoreFoundation strings")
 BENIGN_LANGOPT(InlineVisibilityHidden , 1, 0, "hidden visibility for inline 
C++ methods")


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,13 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext  = getASTContext();
+
+  // 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)
+(void)Ctx.getASTRecordLayout(this);
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5371,10 +5371,13 @@
 def fdump_record_layouts_simple : Flag<["-"], "fdump-record-layouts-simple">,
   HelpText<"Dump record layout information in a simple form

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-17 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith added inline comments.



Comment at: clang/lib/AST/Decl.cpp:4585
+
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {

Nit: please add whitespace around `=`. (Have you run git-clang-format? That 
should fix this.)



Comment at: clang/lib/AST/Decl.cpp:4586
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);

Probably this deserves a comment, explaining that computing the record layout 
has a side effect of dumping it.

Nit: I think we usually skip braces for one-line `if` statements.



Comment at: clang/lib/AST/Decl.cpp:4587
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);
+  }

Might be reasonable to cast to `(void)` rather than assigning to an ignored 
variable.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104484/new/

https://reviews.llvm.org/D104484

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

[PATCH] D104484: [clang] Add cc1 option for dumping layout for all complete types

2021-06-17 Thread David Tenty via Phabricator via cfe-commits 
daltenty created this revision.
Herald added subscribers: dexonsmith, dang.
daltenty requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change adds an option which, in addition to dumping the record
layout as is done by -fdump-record-layouts, causes us to compute the
layout for all complete record types (rather than the as-needed basis
which is usually done by clang), so that we will dump them as well.
This is useful if we are looking for layout differences across large
code bases without needing to instantiate every type we are interested in.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104484

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Decl.cpp
  clang/test/Layout/dump-complete.cpp


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | 
FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,11 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);
+  }
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5371,10 +5371,13 @@
 def fdump_record_layouts_simple : Flag<["-"], "fdump-record-layouts-simple">,
   HelpText<"Dump record layout information in a simple form used for testing">,
   MarshallingInfoFlag>;
+def fdump_record_layouts_complete : Flag<["-"], 
"fdump-record-layouts-complete">,
+  HelpText<"Dump record layout information for all complete types">,
+  MarshallingInfoFlag>;
 def fdump_record_layouts : Flag<["-"], "fdump-record-layouts">,
   HelpText<"Dump record layout information">,
   MarshallingInfoFlag>,
-  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath]>;
+  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath, 
fdump_record_layouts_complete.KeyPath]>;
 def fix_what_you_can : Flag<["-"], "fix-what-you-can">,
   HelpText<"Apply fix-it advice even in the presence of unfixable errors">,
   MarshallingInfoFlag>;
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -266,6 +266,7 @@
 BENIGN_LANGOPT(ElideConstructors , 1, 1, "C++ copy constructor elision")
 BENIGN_LANGOPT(DumpRecordLayouts , 1, 0, "dumping the layout of IRgen'd 
records")
 BENIGN_LANGOPT(DumpRecordLayoutsSimple , 1, 0, "dumping the layout of IRgen'd 
records in a simple form")
+BENIGN_LANGOPT(DumpRecordLayoutsComplete , 1, 0, "dumping the AST layout of 
all complete records")
 BENIGN_LANGOPT(DumpVTableLayouts , 1, 0, "dumping the layouts of emitted 
vtables")
 LANGOPT(NoConstantCFStrings , 1, 0, "no constant CoreFoundation strings")
 BENIGN_LANGOPT(InlineVisibilityHidden , 1, 0, "hidden visibility for inline 
C++ methods")


Index: clang/test/Layout/dump-complete.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-complete.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | FileCheck %s
+
+struct a {
+  int x;
+};
+
+struct b {
+  char y;
+} foo;
+
+class c {};
+
+class d;
+
+// CHECK:  0 | struct a
+// CHECK:  0 | struct b
+// CHECK:  0 | class c
+// CHECK-NOT:  0 | class d
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4581,6 +4581,11 @@
 void RecordDecl::completeDefinition() {
   assert(!isCompleteDefinition() && "Cannot redefine record!");
   TagDecl::completeDefinition();
+
+  ASTContext =getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+const ASTRecordLayout  = Ctx.getASTRecordLayout(this);
+  }
 }
 
 /// isMsStruct - Get whether or not this record uses ms_struct layout.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@