hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.
lgtm, just some nits.
================
Comment at: clang/lib/Analysis/CFG.cpp:485
- // This can point either to a try or a __try block. The frontend forbids
- // mixing both kinds in one function, so having one for both is enough.
+ // This can point to either to a C++ try, an Objective-C @try, or a SEH
__try.
+ // try and @try can be mixed and generally work the same.
----------------
One "to" too much in "to either to a ...".
Should probably be *an* SEH __try (at least in my head, that S is pronounced
"ess").
================
Comment at: clang/lib/Analysis/CFG.cpp:3888
+
+ // We set Block to NULL to allow lazy creation of a new block (if necessary)
+ Block = nullptr;
----------------
ultra nit: period.
================
Comment at: clang/lib/Analysis/CFG.cpp:3895
CFGBlock *CFGBuilder::VisitObjCAtThrowStmt(ObjCAtThrowStmt *S) {
// FIXME: This isn't complete. We basically treat @throw like a return
// statement.
----------------
does the fixme still stand?
================
Comment at: clang/lib/Analysis/CFG.cpp:3941
+ bool HasCatchAll = false;
+ for (unsigned h = 0; h < Terminator->getNumCatchStmts(); ++h) {
+ // The code after the try is the implicit successor.
----------------
Why h? Could this be a range for instead? Otherwise for (i = .., e = ..., i !=
e) is the classic llvm style.
================
Comment at: clang/lib/Analysis/CFG.cpp:5790
+ OS << "@catch (";
+ if (CS->getCatchParamDecl())
+ CS->getCatchParamDecl()->print(OS,
PrintingPolicy(Helper.getLangOpts()),
----------------
maybe
```
if (const VarDecl *PD = CS->getCatchParamDecl())
PD->print(...)
```
================
Comment at: clang/test/Sema/warn-unreachable.m:1
+// RUN: %clang_cc1 %s -fsyntax-only -fobjc-exceptions -verify
-Wunreachable-code
+
----------------
Not important, but I think it's much more common to have %s at the end. Same
for the other test file.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112287/new/
https://reviews.llvm.org/D112287
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
