cor3ntin added a comment.
I added a few more comments, mostly nitpicks
================
Comment at: clang/lib/AST/ExprConstant.cpp:9952-9953
+ const FieldDecl *Field;
+ if (isa<InitListExpr>(E)) {
+ Field = cast<InitListExpr>(E)->getInitializedFieldInUnion();
+ } else if (isa<CXXParenListInitExpr>(E)) {
----------------
================
Comment at: clang/lib/AST/ExprConstant.cpp:9956
+ assert(InitExprs.size() == 1 &&
+ "Number of expressions in C++ paren list is not 1");
+
----------------
Maybe that message could be a bit more clear.
"Union should have exactly 1 initializer in in C++ paren list" or something
like that.
================
Comment at: clang/lib/CodeGen/CGExprAgg.cpp:1595
+void AggExprEmitter::VisitCXXParenListInitExpr(CXXParenListInitExpr *E) {
+ ArrayRef<Expr *> InitExprs = E->getInitExprs();
----------------
There are some overlaps with `VisitInitListExpr`, I wonder if we should try to
factor out some of the code, especially the base class initialization
================
Comment at: clang/lib/CodeGen/CGExprAgg.cpp:1600
+ if (const ArrayType *AT = dyn_cast<ArrayType>(E->getType())) {
+ for (auto Pair : llvm::enumerate(InitExprs)) {
+ // Initialization for every element of the array.
----------------
Maybe it would be cleaner to use a structured binding here.
================
Comment at: clang/lib/CodeGen/CGExprAgg.cpp:1616
+ const CXXRecordDecl *BaseRD = Base.getType()->getAsCXXRecordDecl();
+ Address V = CGF.GetAddressOfDirectBaseInCompleteClass(
+ Dest.getAddress(), RD, BaseRD,
----------------
Should we assert that the base is not virtual?
================
Comment at: clang/lib/Sema/SemaInit.cpp:5281
+
+ // Unnamed bitfields should not be initialized at all,either with an arg
+ // or by default.
----------------
================
Comment at: clang/lib/Sema/TreeTransform.h:13930
+ InitExprs.size(), true, TransformedInits,
+ &ArgumentChanged))
+ return ExprError();
----------------
`ArgumentChanged` is not used, and is defaulted by the function, so you should
be able to remove it
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129531/new/
https://reviews.llvm.org/D129531
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
