[PATCH] D20490: [Parser] Fix a crash on invalid where a delayed TypoExpr was corrected twice

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/Parse/ParseExpr.cpp:450-452
@@ -449,1 +449,5 @@
+
+        // In this case, ActOnBinOp performed the CorrectDelayedTyposInExpr check.
+        if (!getLangOpts().CPlusPlus)
+          continue;
       } else {
----------------
The inconsistent behavior of `ActOnBinOp` seems somewhere between an implementation detail and a bug; it doesn't seem reasonable for the parser to rely on that. I'm not particularly happy about making changes like this without some documentation of the overall design that shows whose responsibility it is to correct typos in which cases.


Before we introduced `TypoExpr`, the parser was permitted to simply discard `Expr` nodes that it didn't use (because it'd hit a parse error). Ideally, I'd like to return to that state of affairs, by removing the relevant `CorrectDelayedTyposInExpr` calls from the parser and having Sema automatically diagnose them when we get to the end of the relevant context, if we've not already done so.

Another reasonable-seeming option would be to add a `Sema::ActOnDiscardedExpr(Expr*)` that the parser can call (which calls `CorrectDelayedTyposInExpr`), and make it clear that the parser is responsible for passing each Expr that it receives from Sema to exactly one ActOn* function (unless otherwise specified) -- that way, at least the responsibilities will be clear, but it doesn't help us avoid bugs where `TypoExpr`s are accidentally discarded.


http://reviews.llvm.org/D20490