[PATCH] D129531: [clang][C++20] P0960R3: Allow initializing aggregates from a parenthesized list of values

[Sheng via Phabricator via cfe-commits]

0x59616e added a comment.

I can only nitpick some of the peripheral issues since I have no knowledge in most of the part of clang. Perhaps implementing the new standard feature is too arduous for a tyro like me. It's great to see the real expert to complete this.



================
Comment at: clang/lib/AST/ExprConstant.cpp:9959-9962
+        if (!FD->isUnnamedBitfield()) {
+          Field = FD;
+          break;
+        }
----------------
nit: Maybe use early exit to reduce indentation for better readability ?


================
Comment at: clang/lib/AST/StmtPrinter.cpp:2465-2471
+  unsigned i = 0;
+  for (Expr *E : Node->getInitExprs()) {
+    if (i)
+      OS << ", ";
+    PrintExpr(E);
+    i++;
+  }
----------------
nit: Is it possible to use `llvm::interleaveComma` [0] here without any bad effect (e.g. increase in compiling time, since STLExtras.h is not a trivial header) ?

[0] https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/STLExtras.h#L1902


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531