This is quite useful for IDE's or other tools which would like to recover as
much as possible even if there are a few errors in the code, and discarding
the full 'if' bodies is unfortunate.
Is there still an error reported for the invalid condition?
Anyhow, from my POV this is excellent and should fix a bunch of issues I've seen when using clang-c in KDevelop. I never got around to investigating it, but it always was related to conditionals. I'm pretty sure this patch fixes it then.
@ogoffart: For Manuel to see the difference, could you share the before/after screenshots of your code browser? That makes the impact of this patch pretty clear.
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).
Please add a comment here saying that we're creating this node for error recovery. Also, the type of the expression should be BoolTy, not VoidTy. Other than that, this looks fine to me.