schittir marked 2 inline comments as done. schittir added inline comments.
================ Comment at: clang/include/clang/AST/Stmt.h:2606-2607 enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR }; - Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt. + Stmt *SubExprs[END_EXPR] = { + nullptr}; // SubExprs[INIT] is an expression or declstmt. SourceLocation LParenLoc, RParenLoc; ---------------- tahonermann wrote: > aaron.ballman wrote: > > I don't think this initialization is necessary. The constructor for > > `ForStmt` initializes all of the valid elements: > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024 > > > > The `EmptyShell` constructor does not initialize anything but that's > > because it is piecemeal initialized by the AST importer, but all of its > > fields are also initialized: > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296 > Declarations like this are common in the AST. I'm curious while static > analysis flagged this one in particular. Perhaps it identified a path where > one or more of the elements don't get initialized? If so, this would be a > good find and a fix should be applied to that path. I will follow-up with this one more and open a different PR if necessary. Removing this change. ================ Comment at: clang/include/clang/AST/Stmt.h:2606-2607 enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR }; - Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt. + Stmt *SubExprs[END_EXPR] = { + nullptr}; // SubExprs[INIT] is an expression or declstmt. SourceLocation LParenLoc, RParenLoc; ---------------- schittir wrote: > tahonermann wrote: > > aaron.ballman wrote: > > > I don't think this initialization is necessary. The constructor for > > > `ForStmt` initializes all of the valid elements: > > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/AST/Stmt.cpp#L1024 > > > > > > The `EmptyShell` constructor does not initialize anything but that's > > > because it is piecemeal initialized by the AST importer, but all of its > > > fields are also initialized: > > > https://github.com/llvm/llvm-project/blob/87c5a3e203ae3643bc13c9a13917b92a657f4358/clang/lib/Serialization/ASTReaderStmt.cpp#L296 > > Declarations like this are common in the AST. I'm curious while static > > analysis flagged this one in particular. Perhaps it identified a path where > > one or more of the elements don't get initialized? If so, this would be a > > good find and a fix should be applied to that path. > I will follow-up with this one more and open a different PR if necessary. > Removing this change. Thank you for this analysis! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158488/new/ https://reviews.llvm.org/D158488 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits