shafik added a comment.
I am going to do another pass but this is my initial set of comments.
================
Comment at: clang/include/clang-c/Index.h:1537
+ * initializer.
+ *
+ */
----------------
nit: extra line
================
Comment at: clang/include/clang/AST/ExprCXX.h:4728-4729
+/// void foo() {
+/// A a1(0); // legal in C++20
+/// A a2(1.5, 1.0); // legal in C++20
+/// }
----------------
================
Comment at: clang/include/clang/AST/ExprCXX.h:4741-4742
+/// void foo() {
+/// A a(1.5); // legal in C++20
+/// A b{1.5}; // illegal !
+/// }
----------------
================
Comment at: clang/include/clang/AST/ExprCXX.h:4779
+ const ArrayRef<Expr *> getInitExprs() const {
+ return {getTrailingObjects<Expr *>(), NumExprs};
+ }
----------------
Should we prefer `makeArrayRef`?
================
Comment at: clang/lib/AST/Expr.cpp:3699
+ // FIXME: unimplemented
+ llvm_unreachable("unimplemented");
}
----------------
cor3ntin wrote:
> You can probably just fall through, a list of expression only has side
> effects if the individual expressions do, which the code just below is doing.
On Discourse, the suggestion was made to have use `InitListExpr` as a base
class and in that case I believe we should do the same that that we do for
`InitListExpr`.
================
Comment at: clang/lib/AST/ExprClassification.cpp:446
+
+ case Expr::CXXParenListInitExprClass:
+ // FIXME: unimplemented
----------------
This looks like we should handle this similar to `InitListExpr` but I am not
sure if single element if it is bound to a reference can be an lvalue holds as
well here as well CC @erichkeane
================
Comment at: clang/lib/AST/StmtPrinter.cpp:2459
+void StmtPrinter::VisitCXXParenListInitExpr(CXXParenListInitExpr *Node) {
+ // FIXME: unimplemented
+ llvm_unreachable("unimplemented");
----------------
Assuming we go w/ `InitListExpr` as a base class then you should do something
similar to ` StmtPrinter::VisitInitListExpr(...)` except with parens.
================
Comment at: clang/lib/CodeGen/CGExprAgg.cpp:1618
+ Dest.getAddress(), RD, BaseRD,
+ /*isBaseVirtual*/ false);
+ AggValueSlot AggSlot = AggValueSlot::forAddr(
----------------
================
Comment at: clang/lib/Sema/SemaInit.cpp:5263
+static void TryOrBuildParenListInitialization(
+ Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind,
----------------
I feel like this function could use a lot of comments, I found it hard to
follow the flow of logic and I think I get it mostly now but comments would
have helped a lot.
================
Comment at: clang/lib/Sema/SemaInit.cpp:5273
+ for (InitializedEntity SubEntity : Range) {
+ if (Index == 1 && Entity.getType()->isUnionType())
+ // Unions should only have one initializer expression.
----------------
Or some other clarifying comment.
================
Comment at: clang/lib/Sema/SemaInit.cpp:5310
+ InitializationKind SubKind = InitializationKind::CreateForInit(
+ E->getExprLoc(), /*isDirectInit*/ false, E);
+ InitializationSequence Seq(S, SubEntity, SubKind, E);
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129531/new/
https://reviews.llvm.org/D129531
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
