[PATCH] D65591: [AST] Add a flag indicating if any subexpression had errors

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I would actually try to counter the type proposal and defend the `containsErrors` bit. Here's my thinking.

Not knowing that `int(<something-that-contains-errors>)` had errors inside can lead to situations with bad diagnostics.
We won't be able to suppress any non-type-related error, e.g. `int *a = int(<something-that-contains-errors>)` will probably complain that you can't assign an `int` to a pointer. This diagnostics is spurious, because `<something-that-contains-error>` might have actually evaluated to `0` and the users are better-off fixing the original error first and showing the other diagnostic is just noise.
The example might look a little contrived, but having an easy-to-use mechanism to suppress those errors is useful.

Expressions are not the only construct where we loose information. Something like `containsErrors` could generalize to types, template arguments, name qualifiers and other constructs that can potentially have valid sub-components, even if the compiler chooses to not produce them now due to semantic errors. This is forward-looking and I don't think we have concrete plans to make this happen, but it would be sad to go with a design that can't be extended to anything beyond expressions even theoretically.

Adding a new type, properly propagating it and checking for it in all places where we suppress diagnostics is a lot of work. `containsErrors` is a good way to incrementally find places that need to be tweaked and fix them to avoid producing extra diagnostics and running function that require well-formed ASTs. Having a new type seems like an all-or-nothing effort, i.e. one would have to spend considerable time building it and fixing everything that used to produce invalid expressions.
As for the lamda case: can we actually propagate `containsErrors` through lambdas? That probably means lifting this bit to the statement level, but that could definitely work. Knowing that lambda (or any other function, for that matter) produced an error somewhere inside its body actually looks like useful information and propagating it is not a lot of effort (in fact, I did try this and, while being more complicated than expressions, propagating this flag for statements seems doable).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65591/new/

https://reviews.llvm.org/D65591