[PATCH] D158488: [NFC] Initialize member pointers to nullptr.
Sindhu Chittireddy via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 22 08:21:27 PDT 2023
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
More information about the llvm-commits
mailing list