[PATCH] D157118: [NFC][Clang] Fix static analyzer concerns
Tom Honermann via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 8 09:10:54 PDT 2023
tahonermann accepted this revision.
tahonermann added a comment.
I think this change is good as is, but I added some additional thoughts to Aaron's earlier comment.
================
Comment at: clang/lib/AST/StmtPrinter.cpp:178
void StmtPrinter::PrintRawCompoundStmt(CompoundStmt *Node) {
+ assert(Node && "Compound statement cannot be null");
OS << "{" << NL;
----------------
aaron.ballman wrote:
> Hmmm, I think this is what we're effectively already hoping is the case today, but I don't think that's a safe assertion by itself. Consider: https://github.com/llvm/llvm-project/blob/cd29ebb862b5c7a81c9f39c8c493f9246d6e5f0b/clang/lib/AST/StmtPrinter.cpp#L602
>
> It might be worth looking at all the calls to `PrintRawCompoundStmt()` to see which ones potentially can pass null in, and decide if there are additional changes to make. e.g., should that `dyn_cast` be a `cast` instead so it cannot return nullptr and we get the assertion a little bit earlier when calling `cast<>`?
Another possibility would be to modify `ObjCAtFinallyStmt` to declare `AtFinallyStmt` as `CompoundStmt*` and to modify `getFinallyBody()` and `setFinallyBody()` accordingly. That would remove the need for that `dyn_cast` in `StmtPrinter::VisitObjCAtTryStmt()`. This would then require additional changes such as updates to `ASTStmtReader::VisitObjCAtFinallyStmt()` to perform a cast as is done in `ASTStmtReader::VisitStmtExpr()`. But none of that actually addresses the fact that this function has a precondition that `Node` is not null.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157118/new/
https://reviews.llvm.org/D157118
More information about the cfe-commits
mailing list