aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM though there may be some additional changes worth considering; I don't 
insist on changes though.



================
Comment at: clang/lib/AST/StmtPrinter.cpp:178
 void StmtPrinter::PrintRawCompoundStmt(CompoundStmt *Node) {
+  assert(Node && "Compound statement cannot be null");
   OS << "{" << NL;
----------------
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<>`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157118/new/

https://reviews.llvm.org/D157118

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to