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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/lib/AST/Decl.cpp:4585
+
+  ASTContext &Ctx=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 &Ctx=getASTContext();
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete) {
+    const ASTRecordLayout &RL = 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 &RL = 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