aaron.ballman added inline comments.
Comment at: lib/Sema/SemaStmt.cpp:823
+ CondExpr->isValueDependent() ||
+ isa(CondExpr);
+ unsigned CondWidth =
rsmith wrote:
> It's fragile to assume that the
rsmith added inline comments.
Comment at: lib/Sema/SemaStmt.cpp:823
+ CondExpr->isValueDependent() ||
+ isa(CondExpr);
+ unsigned CondWidth =
It's fragile to assume that the only way you can see a
ogoffart updated this revision to Diff 119142.
ogoffart added a comment.
Updated the patch so that ActOnStartOfSwitchStmt returns void, and
ActOnFinishSwitchStmt will skip some checks in case of error
https://reviews.llvm.org/D26350
Files:
include/clang/Sema/Sema.h
lib/Parse/ParseStmt.cpp
rsmith added inline comments.
Comment at: lib/Parse/ParseStmt.cpp:1297
- if (Switch.isInvalid()) {
-// Skip the switch body.
-// FIXME: This is not optimal recovery, but parsing the body is more
-// dangerous due to the presence of case and default statements, whic
aaron.ballman added a comment.
This looks reasonable to me, but you should wait for @rsmith to sign off before
committing.
Comment at: lib/Sema/SemaStmt.cpp:669
if (Cond.isInvalid())
-return StmtError();
+Cond = ConditionResult(
+*this, nullptr,
ogoffart added a comment.
Thanks for your review and i'll try to address the concerns.
I believe tools that really need valid code relies on the diagnostics and bail
out on error. On the other hand, tools that may work on code containing error
do a best effort to work on the remaining AST .
And
aaron.ballman added a comment.
In https://reviews.llvm.org/D26350#809577, @ogoffart wrote:
> The problem i'm trying to solve is precisely to keep as much as possible of
> the valid AST in the main AST, despite errors.
> I've already done some work with r249982, r272962 and more, and there is
>
ogoffart added a comment.
The problem i'm trying to solve is precisely to keep as much as possible of the
valid AST in the main AST, despite errors.
I've already done some work with r249982, r272962 and more, and there is still
a lot to do. But the goal is to keep as much as possible of it.
The
aaron.ballman added a comment.
You've explained how you are accomplishing this but not why. I don't think
Clang typically keeps erroneous AST nodes in the tree. What kind of problem is
this intended to solve?
https://reviews.llvm.org/D26350
___
cf
ogoffart added a comment.
re-ping
https://reviews.llvm.org/D26350
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ogoffart added a comment.
Ping
https://reviews.llvm.org/D26350
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ogoffart added a comment.
ping2
https://reviews.llvm.org/D26350
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ogoffart added a comment.
ping?
https://reviews.llvm.org/D26350
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ogoffart added a comment.
I believe only the change in ActOnFinishSwitchStmt might be controversial.
Is it breaking an invariant than having switches kept in the AST?
https://reviews.llvm.org/D26350
___
cfe-commits mailing list
cfe-commits@lists.ll
ogoffart created this revision.
ogoffart added reviewers: rsmith, cfe-commits.
- When the condition is invalid, replace it by an OpaqueValueExpr
- When parsing an invalid CaseStmt, don't drop the sub statement, just return
it instead.
- In Sema::ActOnStartOfSwitchStmt, always keep the SwitchSt
15 matches
Mail list logo
