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
> still a lot to do. But the goal is to keep as much as possible of it.
Thank you for the explanation -- I like the goal, but am definitely concerned
about the assumptions it might break by doing this (in general, not specific to
this patch). We try to recover gracefully whenever possible, but still, a whole
lot of frontend code relies on knowing when something is invalid to prevent the
compiler from doing even worse things. I'm not certain the best balance to
strike with that, but suspect it's going to adversely impact tools like
clang-tidy which tend to assume that AST matchers match *valid* code and not
invalid code.
================
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, which
- // will have no place to connect back with the switch.
- if (Tok.is(tok::l_brace)) {
- ConsumeBrace();
- SkipUntil(tok::r_brace);
- } else
- SkipUntil(tok::semi);
- return Switch;
- }
+ assert(!Switch.isInvalid() && "ActOnStartOfSwitchStmt always succeed");
----------------
succeed -> succeeds
Are the concerns pointed out in the FIXME addressed by code not in this patch?
================
Comment at: lib/Sema/SemaStmt.cpp:669
if (Cond.isInvalid())
- return StmtError();
+ Cond = ConditionResult(
+ *this, nullptr,
----------------
This makes the condition result valid when it isn't. Users of this condition
result may expect a valid condition result to return nonnull values when
calling `get()`, which makes me uncomfortable.
https://reviews.llvm.org/D26350
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
