[PATCH] D13344: Keep the IfStmt node even if the condition is invalid

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/Sema/SemaStmt.cpp:508-509
@@ -513,6 +507,4 @@
 
-  DiagnoseUnusedExprResult(elseStmt);
-
   return new (Context) IfStmt(Context, IfLoc, ConditionVar, ConditionExpr,
                               thenStmt, ElseLoc, elseStmt);
 }
----------------
This will create an `IfStmt` with no `ConditionExpr`. That is not a valid AST construct, and it's not reasonable to expect all the downstream consumers of the AST to be able to cope with it. It's not hard to find parts of the codebase that will crash when presented with this:

  lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:1133
  lib/Analysis/Consumed.cpp:1265
  lib/Sema/AnalysisBasedWarnings.cpp:740

... and so on.

As the comment above suggests, you could create some sort of placeholder expression node here instead (either use something like an `OpaqueValueExpr`, or add a new Expr subclass to represent an erroneous expression -- the latter would probably be useful in many cases where we hit parse errors but can still produce some useful information to tools).


http://reviews.llvm.org/D13344