[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
danix800 updated this revision to Diff 542950.
danix800 added a comment.
1. Remove unnecessary condition & cleanup code
2. Move testcase into correct place.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155396/new/
https://reviews.llvm.org/D155396
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/AST/ast-dump-recovery.m
Index: clang/test/AST/ast-dump-recovery.m
===
--- clang/test/AST/ast-dump-recovery.m
+++ clang/test/AST/ast-dump-recovery.m
@@ -1,4 +1,4 @@
-// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -frecovery-ast
-frecovery-ast-type -ast-dump %s | FileCheck -strict-whitespace %s
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -frecovery-ast
-frecovery-ast-type -fblocks -ast-dump %s | FileCheck -strict-whitespace %s
@interface Foo
- (void)method:(int)n;
@@ -16,3 +16,11 @@
// CHECK-NEXT: `-DeclRefExpr {{.*}} 'foo'
foo.undef;
}
+
+// CHECK: |-VarDecl {{.*}} 'int (^)()' cinit
+// CHECK: | `-RecoveryExpr {{.*}} ' (^)(void)' contains-errors
lvalue
+// CHECK: | `-BlockExpr {{.*}} ' (^)(void)'
+// CHECK: | `-BlockDecl {{.*}} invalid
+int (^a)() = ^() {
+ return c;
+};
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3730,6 +3730,11 @@
if (FunctionScopes.back()->FirstReturnLoc.isInvalid())
FunctionScopes.back()->FirstReturnLoc = ReturnLoc;
+ if (auto *CurBlock = dyn_cast(CurCap);
+ CurBlock && CurCap->HasImplicitReturnType && RetValExp &&
+ RetValExp->containsErrors())
+CurBlock->TheDecl->setInvalidDecl();
+
return Result;
}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17159,6 +17159,9 @@
if (getCurFunction())
getCurFunction()->addBlock(BD);
+ if (BD->isInvalidDecl())
+return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
+ {Result}, Result->getType());
return Result;
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -663,6 +663,10 @@
- Correcly diagnose jumps into statement expressions.
This ensures the behavior of Clang is consistent with GCC.
(`#63682 `_)
+- Invalidate BlockDecl with implicit return type, in case any of the return
+ value exprs is invalid. Propagating the error info up by replacing BlockExpr
+ with a RecoveryExpr. This fixes:
+ (`#63863 _`)
Bug Fixes to Compiler Builtins
^^
Index: clang/test/AST/ast-dump-recovery.m
===
--- clang/test/AST/ast-dump-recovery.m
+++ clang/test/AST/ast-dump-recovery.m
@@ -1,4 +1,4 @@
-// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -frecovery-ast -frecovery-ast-type -ast-dump %s | FileCheck -strict-whitespace %s
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -frecovery-ast -frecovery-ast-type -fblocks -ast-dump %s | FileCheck -strict-whitespace %s
@interface Foo
- (void)method:(int)n;
@@ -16,3 +16,11 @@
// CHECK-NEXT: `-DeclRefExpr {{.*}} 'foo'
foo.undef;
}
+
+// CHECK: |-VarDecl {{.*}} 'int (^)()' cinit
+// CHECK: | `-RecoveryExpr {{.*}} ' (^)(void)' contains-errors lvalue
+// CHECK: | `-BlockExpr {{.*}} ' (^)(void)'
+// CHECK: | `-BlockDecl {{.*}} invalid
+int (^a)() = ^() {
+ return c;
+};
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3730,6 +3730,11 @@
if (FunctionScopes.back()->FirstReturnLoc.isInvalid())
FunctionScopes.back()->FirstReturnLoc = ReturnLoc;
+ if (auto *CurBlock = dyn_cast(CurCap);
+ CurBlock && CurCap->HasImplicitReturnType && RetValExp &&
+ RetValExp->containsErrors())
+CurBlock->TheDecl->setInvalidDecl();
+
return Result;
}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17159,6 +17159,9 @@
if (getCurFunction())
getCurFunction()->addBlock(BD);
+ if (BD->isInvalidDecl())
+return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
+ {Result}, Result->getType());
return Result;
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -663,6 +663,10 @@
- Correcly diagnose jumps into
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
danix800 added inline comments. Comment at: clang/test/SemaObjC/crash-on-val-dep-block-expr.m:1 +// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s +// no crash danix800 wrote: > 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? I'd prefer `ast-dump-recovery.m` since this file is much shorter. Either file needs appending `-fblocks`. Repository: rZORG LLVM Github Zorg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155396/new/ https://reviews.llvm.org/D155396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
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(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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
hokein added a comment. thanks! Comment at: clang/lib/Sema/SemaStmt.cpp:3736 +BlockDecl *BD = CurBlock->TheDecl; +if (!BD->isInvalidDecl() && RetValExp && RetValExp->containsErrors()) + BD->setInvalidDecl(); nit: the `isInvalidDecl` check is not need, we can inline the `RetValExpr` to the above if as well. ``` if (auto *CurBlock = dyn_cast(CurCap); CurBlock && CurCap->HasImplicitReturnType && RetValExp && RetValExp->containsErrors()) CurBlock->TheDecl->setInvalidDecl(); ``` Comment at: clang/test/SemaObjC/crash-on-val-dep-block-expr.m:1 +// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s +// no crash can you move these tests to `llvm-project/clang/test/AST/ast-dump-recovery.c`? Repository: rZORG LLVM Github Zorg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155396/new/ https://reviews.llvm.org/D155396 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
danix800 updated this revision to Diff 542735.
danix800 edited the summary of this revision.
danix800 added a comment.
Herald added a reviewer: gkistanova.
Update clang/docs/ReleaseNotes.rst to reflect this fix.
Repository:
rZORG LLVM Github Zorg
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155396/new/
https://reviews.llvm.org/D155396
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaObjC/crash-on-val-dep-block-expr.m
Index: clang/test/SemaObjC/crash-on-val-dep-block-expr.m
===
--- /dev/null
+++ clang/test/SemaObjC/crash-on-val-dep-block-expr.m
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s
+// no crash
+
+int (^a)() = ^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+};
+
+int (^b)() = (^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+});
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3730,6 +3730,13 @@
if (FunctionScopes.back()->FirstReturnLoc.isInvalid())
FunctionScopes.back()->FirstReturnLoc = ReturnLoc;
+ if (auto *CurBlock = dyn_cast(CurCap);
+ CurBlock && CurCap->HasImplicitReturnType) {
+BlockDecl *BD = CurBlock->TheDecl;
+if (!BD->isInvalidDecl() && RetValExp && RetValExp->containsErrors())
+ BD->setInvalidDecl();
+ }
+
return Result;
}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17159,6 +17159,9 @@
if (getCurFunction())
getCurFunction()->addBlock(BD);
+ if (BD->isInvalidDecl())
+return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
+ {Result}, Result->getType());
return Result;
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -657,6 +657,10 @@
(`#63169 _`)
- Fix crash when casting an object to an array type.
(`#63758 _`)
+- Invalidate BlockDecl with implicit return type, in case any of the return
+ value exprs is invalid. Propagating the error info up by replacing BlockExpr
+ with a RecoveryExpr. This fixes:
+ (`#63863 _`)
Bug Fixes to Compiler Builtins
^^
Index: clang/test/SemaObjC/crash-on-val-dep-block-expr.m
===
--- /dev/null
+++ clang/test/SemaObjC/crash-on-val-dep-block-expr.m
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s
+// no crash
+
+int (^a)() = ^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+};
+
+int (^b)() = (^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+});
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3730,6 +3730,13 @@
if (FunctionScopes.back()->FirstReturnLoc.isInvalid())
FunctionScopes.back()->FirstReturnLoc = ReturnLoc;
+ if (auto *CurBlock = dyn_cast(CurCap);
+ CurBlock && CurCap->HasImplicitReturnType) {
+BlockDecl *BD = CurBlock->TheDecl;
+if (!BD->isInvalidDecl() && RetValExp && RetValExp->containsErrors())
+ BD->setInvalidDecl();
+ }
+
return Result;
}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17159,6 +17159,9 @@
if (getCurFunction())
getCurFunction()->addBlock(BD);
+ if (BD->isInvalidDecl())
+return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
+ {Result}, Result->getType());
return Result;
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -657,6 +657,10 @@
(`#63169 _`)
- Fix crash when casting an object to an array type.
(`#63758 _`)
+- Invalidate BlockDecl with implicit return type, in case any of the return
+ value exprs is invalid. Propagating the error info up by replacing BlockExpr
+ with a RecoveryExpr. This fixes:
+ (`#63863 _`)
Bug Fixes to Compiler Builtins
^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
aaron.ballman added a comment.
This looks pretty good to me. Can you also add a release note for the fix to
`clang/docs/ReleaseNotes.rst`?
Comment at: clang/lib/Sema/SemaStmt.cpp:3733-3734
+ BlockScopeInfo *CurBlock = dyn_cast(CurCap);
+ if (CurBlock && CurCap->HasImplicitReturnType) {
+BlockDecl *BD = CurBlock->TheDecl;
if (auto *CurBlock = dyn_cast(CurCap); CurBlock &&
CurCap->HasImplicitReturnType) {
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155396/new/
https://reviews.llvm.org/D155396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
danix800 updated this revision to Diff 542559.
danix800 edited the summary of this revision.
danix800 added a comment.
Invalidate `BlockDecl` with implicit return type, in case any of the return
value exprs is invalid.
Propagating the error info up by replacing `BlockExpr` with a `RecoveryExpr`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155396/new/
https://reviews.llvm.org/D155396
Files:
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaObjC/crash-on-val-dep-block-expr.m
Index: clang/test/SemaObjC/crash-on-val-dep-block-expr.m
===
--- /dev/null
+++ clang/test/SemaObjC/crash-on-val-dep-block-expr.m
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s
+// no crash
+
+int (^a)() = ^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+};
+
+int (^b)() = (^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+});
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3730,6 +3730,13 @@
if (FunctionScopes.back()->FirstReturnLoc.isInvalid())
FunctionScopes.back()->FirstReturnLoc = ReturnLoc;
+ BlockScopeInfo *CurBlock = dyn_cast(CurCap);
+ if (CurBlock && CurCap->HasImplicitReturnType) {
+BlockDecl *BD = CurBlock->TheDecl;
+if (!BD->isInvalidDecl() && RetValExp && RetValExp->containsErrors())
+ BD->setInvalidDecl();
+ }
+
return Result;
}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17159,6 +17159,9 @@
if (getCurFunction())
getCurFunction()->addBlock(BD);
+ if (BD->isInvalidDecl())
+return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
+{Result}, Result->getType());
return Result;
}
Index: clang/test/SemaObjC/crash-on-val-dep-block-expr.m
===
--- /dev/null
+++ clang/test/SemaObjC/crash-on-val-dep-block-expr.m
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s
+// no crash
+
+int (^a)() = ^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+};
+
+int (^b)() = (^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+});
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3730,6 +3730,13 @@
if (FunctionScopes.back()->FirstReturnLoc.isInvalid())
FunctionScopes.back()->FirstReturnLoc = ReturnLoc;
+ BlockScopeInfo *CurBlock = dyn_cast(CurCap);
+ if (CurBlock && CurCap->HasImplicitReturnType) {
+BlockDecl *BD = CurBlock->TheDecl;
+if (!BD->isInvalidDecl() && RetValExp && RetValExp->containsErrors())
+ BD->setInvalidDecl();
+ }
+
return Result;
}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17159,6 +17159,9 @@
if (getCurFunction())
getCurFunction()->addBlock(BD);
+ if (BD->isInvalidDecl())
+return CreateRecoveryExpr(Result->getBeginLoc(), Result->getEndLoc(),
+{Result}, Result->getType());
return Result;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
danix800 added inline comments.
Comment at: clang/lib/Sema/SemaExpr.cpp:16749-16753
+ bool ContainsError = llvm::any_of(BSI->Returns, [](const ReturnStmt *Return)
{
+const auto *RetValExpr = Return->getRetValue();
+return RetValExpr && RetValExpr->containsErrors();
+ });
+ BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy, ContainsError);
hokein wrote:
> aaron.ballman wrote:
> > Hmmm -- is the block *expression* what contains the errors in this case or
> > is it the block declaration? I would have expected this to be an issue for
> > the block declaration created for the block expression. CC @rjmccall
> I think a reasonable model is to follow how we handle `FunctionDecl`, as
> `BlockDecl` and `FunctionDecl` are similar function-decl concepts.
>
> For the crash case like `int (^a)() = ^() { return undefined; }`, we should:
>
> - invalidate the `BlockDecl` as its returned type can not be deduced because
> of the error return stmt (similar to `FunctionDecl`, we invalidate it for
> `auto func() { return undefined; }`)
> - for an invalid `BlockDecl`, we should not build a `BlockExpr` that refers
> to it (we don't build `DeclRefExpr` for invalid `FunctionDecl`). For error
> recovery, we should use `RecoveryExpr`.
>
> So I think the reasonable fix is to invalidate the BlockDecl (calling
> `Decl->setInvalidDecl()`) if its body has any error stmt, and return
> `ExprError()` if the BlockDecl is invalid.
>
Thanks for sharing and pointing out the right direction.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155396/new/
https://reviews.llvm.org/D155396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
hokein added a comment.
Comment at: clang/lib/Sema/SemaExpr.cpp:16749-16753
+ bool ContainsError = llvm::any_of(BSI->Returns, [](const ReturnStmt *Return)
{
+const auto *RetValExpr = Return->getRetValue();
+return RetValExpr && RetValExpr->containsErrors();
+ });
+ BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy, ContainsError);
aaron.ballman wrote:
> Hmmm -- is the block *expression* what contains the errors in this case or
> is it the block declaration? I would have expected this to be an issue for
> the block declaration created for the block expression. CC @rjmccall
I think a reasonable model is to follow how we handle `FunctionDecl`, as
`BlockDecl` and `FunctionDecl` are similar function-decl concepts.
For the crash case like `int (^a)() = ^() { return undefined; }`, we should:
- invalidate the `BlockDecl` as its returned type can not be deduced because of
the error return stmt (similar to `FunctionDecl`, we invalidate it for `auto
func() { return undefined; }`)
- for an invalid `BlockDecl`, we should not build a `BlockExpr` that refers to
it (we don't build `DeclRefExpr` for invalid `FunctionDecl`). For error
recovery, we should use `RecoveryExpr`.
So I think the reasonable fix is to invalidate the BlockDecl (calling
`Decl->setInvalidDecl()`) if its body has any error stmt, and return
`ExprError()` if the BlockDecl is invalid.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155396/new/
https://reviews.llvm.org/D155396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
aaron.ballman added a reviewer: hokein.
aaron.ballman added a subscriber: rjmccall.
aaron.ballman added inline comments.
Comment at: clang/include/clang/AST/ComputeDependence.h:134
ExprDependence computeDependence(ExtVectorElementExpr *E);
-ExprDependence computeDependence(BlockExpr *E);
+ExprDependence computeDependence(BlockExpr *E, bool ContainsError);
ExprDependence computeDependence(AsTypeExpr *E);
This doesn't seem like the correct fix, to me -- I would have expected the
expression itself to track whether it contains errors, so you wouldn't need to
pass in extra information about that.
Comment at: clang/lib/Sema/SemaExpr.cpp:16749-16753
+ bool ContainsError = llvm::any_of(BSI->Returns, [](const ReturnStmt *Return)
{
+const auto *RetValExpr = Return->getRetValue();
+return RetValExpr && RetValExpr->containsErrors();
+ });
+ BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy, ContainsError);
Hmmm -- is the block *expression* what contains the errors in this case or is
it the block declaration? I would have expected this to be an issue for the
block declaration created for the block expression. CC @rjmccall
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155396/new/
https://reviews.llvm.org/D155396
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D155396: [Sema][ObjC] Propagating value-dependent errors into BlockExpr
danix800 created this revision.
danix800 added a reviewer: aaron.ballman.
danix800 added a project: clang.
Herald added a project: All.
danix800 requested review of this revision.
Herald added a subscriber: cfe-commits.
Fix https://github.com/llvm/llvm-project/issues/63863
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D155396
Files:
clang/include/clang/AST/ComputeDependence.h
clang/include/clang/AST/Expr.h
clang/lib/AST/ComputeDependence.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaObjC/crash-on-val-dep-block-expr.m
Index: clang/test/SemaObjC/crash-on-val-dep-block-expr.m
===
--- /dev/null
+++ clang/test/SemaObjC/crash-on-val-dep-block-expr.m
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s
+// no crash
+
+int (^a)() = ^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+};
+
+int (^b)() = (^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+});
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16746,7 +16746,11 @@
AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(, BD, BlockTy);
- BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy);
+ bool ContainsError = llvm::any_of(BSI->Returns, [](const ReturnStmt *Return)
{
+const auto *RetValExpr = Return->getRetValue();
+return RetValExpr && RetValExpr->containsErrors();
+ });
+ BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy, ContainsError);
// If the block isn't obviously global, i.e. it captures anything at
// all, then we need to do a few things in the surrounding context:
Index: clang/lib/AST/ComputeDependence.cpp
===
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -252,10 +252,12 @@
return E->getBase()->getDependence();
}
-ExprDependence clang::computeDependence(BlockExpr *E) {
+ExprDependence clang::computeDependence(BlockExpr *E, bool ContainsError) {
auto D = toExprDependenceForImpliedType(E->getType()->getDependence());
if (E->getBlockDecl()->isDependentContext())
D |= ExprDependence::Instantiation;
+ if ((D & ExprDependence::Value) && ContainsError)
+D |= ExprDependence::Error;
return D;
}
Index: clang/include/clang/AST/Expr.h
===
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -5974,9 +5974,9 @@
protected:
BlockDecl *TheBlock;
public:
- BlockExpr(BlockDecl *BD, QualType ty)
+ BlockExpr(BlockDecl *BD, QualType ty, bool ContainsError = false)
: Expr(BlockExprClass, ty, VK_PRValue, OK_Ordinary), TheBlock(BD) {
-setDependence(computeDependence(this));
+setDependence(computeDependence(this, ContainsError));
}
/// Build an empty block expression.
Index: clang/include/clang/AST/ComputeDependence.h
===
--- clang/include/clang/AST/ComputeDependence.h
+++ clang/include/clang/AST/ComputeDependence.h
@@ -131,7 +131,7 @@
ExprDependence computeDependence(ImplicitValueInitExpr *E);
ExprDependence computeDependence(InitListExpr *E);
ExprDependence computeDependence(ExtVectorElementExpr *E);
-ExprDependence computeDependence(BlockExpr *E);
+ExprDependence computeDependence(BlockExpr *E, bool ContainsError);
ExprDependence computeDependence(AsTypeExpr *E);
ExprDependence computeDependence(DeclRefExpr *E, const ASTContext );
ExprDependence computeDependence(RecoveryExpr *E);
Index: clang/test/SemaObjC/crash-on-val-dep-block-expr.m
===
--- /dev/null
+++ clang/test/SemaObjC/crash-on-val-dep-block-expr.m
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -verify %s
+// no crash
+
+int (^a)() = ^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+};
+
+int (^b)() = (^() {
+ return c; // expected-error {{use of undeclared identifier 'c'}}
+});
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16746,7 +16746,11 @@
AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(, BD, BlockTy);
- BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy);
+ bool ContainsError = llvm::any_of(BSI->Returns, [](const ReturnStmt *Return) {
+const auto *RetValExpr = Return->getRetValue();
+return RetValExpr && RetValExpr->containsErrors();
+ });
+ BlockExpr *Result = new (Context) BlockExpr(BD, BlockTy, ContainsError);
// If the block isn't obviously global, i.e. it captures
