[PATCH] D26350: Keep invalid Switch in the AST

[Aaron Ballman via Phabricator via cfe-commits]

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