[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

2021-07-05 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Thanks for the fix :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

2021-07-05 Thread Steven Wan via Phabricator via cfe-commits 
stevewan added a comment.

In D105112#2858702 , @thakis wrote:

> Hi, you have to bump clang/include/clang/Serialization/ASTBitCodes.h's 
> VERSION_MAJOR after adding a LangOption, else builds with 
> LLVM_APPEND_VC_REV=NO won't invalidate their module cache correctly. See e.g. 
> https://reviews.llvm.org/rGb8b7a9dcdcbc Without that, tests are broken on 
> bots with  LLVM_APPEND_VC_REV=NO , eg 
> http://45.33.8.238/linux/50460/step_7.txt

Posted fix rGe2904c8e0fa901adeefe579297cb2ece2757fb18 
. Sorry 
for the churn.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

2021-07-05 Thread Nico Weber via Phabricator via cfe-commits 
thakis added a comment.

Hi, you have to bump clang/include/clang/Serialization/ASTBitCodes.h's 
VERSION_MAJOR after adding a LangOption, else builds with LLVM_APPEND_VC_REV=NO 
won't invalidate their module cache correctly. See e.g. 
https://reviews.llvm.org/rGb8b7a9dcdcbc Without that, tests are broken on bots 
with  LLVM_APPEND_VC_REV=NO , eg http://45.33.8.238/linux/50460/step_7.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

2021-07-05 Thread Steven Wan via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9964b0ef828b: [clang] Add -fdump-record-layouts-canonical 
option (authored by daltenty, committed by stevewan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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


Index: clang/test/Layout/dump-canonical.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-canonical.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-canonical %s | 
FileCheck %s -check-prefix CANONICAL
+
+typedef long foo_t;
+
+
+struct a {
+  foo_t x;
+} b;
+
+struct c {
+  typedef foo_t bar_t;
+  bar_t x;
+} d;
+
+// CHECK:  0 | foo_t
+// CHECK:  0 | c::bar_t
+// CANONICAL-NOT:  0 | foo_t
+// CANONICAL-NOT:  0 | c::bar_t
+// CANONICAL:  0 | long
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3577,7 +3577,10 @@
 } else {
   PrintOffset(OS, FieldOffset, IndentLevel);
 }
-OS << Field.getType().getAsString() << ' ' << Field << '\n';
+const QualType  = C.getLangOpts().DumpRecordLayoutsCanonical
+? Field.getType().getCanonicalType()
+: Field.getType();
+OS << FieldType.getAsString() << ' ' << Field << '\n';
   }
 
   // Dump virtual bases.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5405,13 +5405,16 @@
 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_canonical : Flag<["-"], 
"fdump-record-layouts-canonical">,
+  HelpText<"Dump record layout information with canonical field types">,
+  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, 
fdump_record_layouts_complete.KeyPath]>;
+  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath, 
fdump_record_layouts_complete.KeyPath, fdump_record_layouts_canonical.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
@@ -269,6 +269,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(DumpRecordLayoutsCanonical , 1, 0, "dumping the AST layout of 
records using canonical field types")
 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")


Index: clang/test/Layout/dump-canonical.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-canonical.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-canonical %s | FileCheck %s -check-prefix CANONICAL
+
+typedef long foo_t;
+
+
+struct a {
+  foo_t x;
+} b;
+
+struct c {
+  typedef foo_t bar_t;
+  bar_t x;
+} d;
+
+// CHECK:  0 | foo_t
+// CHECK:  0 | c::bar_t
+// CANONICAL-NOT:  0 | foo_t
+// CANONICAL-NOT:  0 | c::bar_t
+// CANONICAL:  0 | long
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3577,7 +3577,10 @@
 } else {
   PrintOffset(OS, FieldOffset, IndentLevel);
 }
-OS <<

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

2021-07-05 Thread Steven Wan via Phabricator via cfe-commits 
stevewan accepted this revision.
stevewan added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

2021-06-30 Thread David Tenty via Phabricator via cfe-commits 
daltenty added a comment.

In D105112#2850990 , @stevewan wrote:

> I'm not familiar with `getCanonicalType()`, can you confirm whether it sees 
> through nested typedef's?

Add a test case that hopefully will cover the case you intend


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

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

- Update test case with nested typedef


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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


Index: clang/test/Layout/dump-canonical.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-canonical.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-canonical %s | 
FileCheck %s -check-prefix CANONICAL
+
+typedef long foo_t;
+
+
+struct a {
+  foo_t x;
+} b;
+
+struct c {
+  typedef foo_t bar_t;
+  bar_t x;
+} d;
+
+// CHECK:  0 | foo_t
+// CHECK:  0 | c::bar_t
+// CANONICAL-NOT:  0 | foo_t
+// CANONICAL-NOT:  0 | c::bar_t
+// CANONICAL:  0 | long
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3577,7 +3577,10 @@
 } else {
   PrintOffset(OS, FieldOffset, IndentLevel);
 }
-OS << Field.getType().getAsString() << ' ' << Field << '\n';
+const QualType  = C.getLangOpts().DumpRecordLayoutsCanonical
+? Field.getType().getCanonicalType()
+: Field.getType();
+OS << FieldType.getAsString() << ' ' << Field << '\n';
   }
 
   // Dump virtual bases.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5381,13 +5381,16 @@
 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_canonical : Flag<["-"], 
"fdump-record-layouts-canonical">,
+  HelpText<"Dump record layout information with canonical field types">,
+  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, 
fdump_record_layouts_complete.KeyPath]>;
+  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath, 
fdump_record_layouts_complete.KeyPath, fdump_record_layouts_canonical.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(DumpRecordLayoutsCanonical , 1, 0, "dumping the AST layout of 
records using canonical field types")
 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")


Index: clang/test/Layout/dump-canonical.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-canonical.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-canonical %s | FileCheck %s -check-prefix CANONICAL
+
+typedef long foo_t;
+
+
+struct a {
+  foo_t x;
+} b;
+
+struct c {
+  typedef foo_t bar_t;
+  bar_t x;
+} d;
+
+// CHECK:  0 | foo_t
+// CHECK:  0 | c::bar_t
+// CANONICAL-NOT:  0 | foo_t
+// CANONICAL-NOT:  0 | c::bar_t
+// CANONICAL:  0 | long
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3577,7 +3577,10 @@
 } else {
   PrintOffset(OS, FieldOffset, IndentLevel);
 }
-OS << Field.getType().getAsString() << ' ' << Field << '\n';
+const QualType  = C.getLangOpts().DumpRecordLayoutsCanonical
+?

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

2021-06-30 Thread Steven Wan via Phabricator via cfe-commits 
stevewan added a comment.

I'm not familiar with `getCanonicalType()`, can you confirm whether it sees 
through nested typedef's?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105112

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

[PATCH] D105112: [clang] Add -fdump-record-layouts-canonical option

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

This option implies -fdump-record-layouts but dumps record layout information 
with canonical field types, which can be more useful in certain cases when 
comparing structure layouts.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105112

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


Index: clang/test/Layout/dump-canonical.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-canonical.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-canonical %s | 
FileCheck %s -check-prefix CANONICAL
+
+typedef long foo_t;
+
+struct a {
+  foo_t x;
+} b;
+
+// CHECK:  0 | foo_t
+// CANONICAL:  0 | long
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3577,7 +3577,10 @@
 } else {
   PrintOffset(OS, FieldOffset, IndentLevel);
 }
-OS << Field.getType().getAsString() << ' ' << Field << '\n';
+const QualType  = C.getLangOpts().DumpRecordLayoutsCanonical
+? Field.getType().getCanonicalType()
+: Field.getType();
+OS << FieldType.getAsString() << ' ' << Field << '\n';
   }
 
   // Dump virtual bases.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5381,13 +5381,16 @@
 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_canonical : Flag<["-"], 
"fdump-record-layouts-canonical">,
+  HelpText<"Dump record layout information with canonical field types">,
+  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, 
fdump_record_layouts_complete.KeyPath]>;
+  ImpliedByAnyOf<[fdump_record_layouts_simple.KeyPath, 
fdump_record_layouts_complete.KeyPath, fdump_record_layouts_canonical.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(DumpRecordLayoutsCanonical , 1, 0, "dumping the AST layout of 
records using canonical field types")
 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")


Index: clang/test/Layout/dump-canonical.cpp
===
--- /dev/null
+++ clang/test/Layout/dump-canonical.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-canonical %s | FileCheck %s -check-prefix CANONICAL
+
+typedef long foo_t;
+
+struct a {
+  foo_t x;
+} b;
+
+// CHECK:  0 | foo_t
+// CANONICAL:  0 | long
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3577,7 +3577,10 @@
 } else {
   PrintOffset(OS, FieldOffset, IndentLevel);
 }
-OS << Field.getType().getAsString() << ' ' << Field << '\n';
+const QualType  = C.getLangOpts().DumpRecordLayoutsCanonical
+? Field.getType().getCanonicalType()
+: