[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr

[Ding Fei via Phabricator via cfe-commits]

danix800 added inline comments.


================
Comment at: clang/lib/Sema/SemaStmt.cpp:3736
+    BlockDecl *BD = CurBlock->TheDecl;
+    if (!BD->isInvalidDecl() && RetValExp && RetValExp->containsErrors())
+      BD->setInvalidDecl();
----------------
hokein wrote:
> nit: the `isInvalidDecl` check is not need, we can inline the `RetValExpr` to the above if as well.
> 
> ```
> if (auto *CurBlock = dyn_cast<BlockScopeInfo>(CurCap); CurBlock && CurCap->HasImplicitReturnType && RetValExp && RetValExp->containsErrors()) 
>   CurBlock->TheDecl->setInvalidDecl();
> ```
Nice, look more cleaner!


================
Comment at: clang/test/SemaObjC/crash-on-val-dep-block-expr.m:1
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s
+// no crash
----------------
hokein wrote:
> can you move these tests to `llvm-project/clang/test/AST/ast-dump-recovery.c`?
No problem. But I noticed `llvm-project/clang/test/AST/ast-dump-recovery.m` also exists,
should I put them into this `.m` file instead?


Repository:
  rZORG LLVM Github Zorg

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

https://reviews.llvm.org/D155396