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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Summary of an off-line discussion with Ilya:

- The current situation where Clang can do delayed typo correction for C++ but not for C is not a good long-term position. The reason for that situation is that we use a dependent type to represent a type with errors. There are two natural paths out of this situation: (1) use something other than a dependent type, such as an error type, for this purpose, and (2) support dependence in C. Of these, (1) is pretty much strictly more expensive than (2): whatever work we need to do for the C-only parts of Clang in (2), we will need to do the same work in the C-only parts of Clang for (1), and much more work in addition. It's also worth noting that we will in fact want all the different flavors of dependence for error tracking too: we will ideally want to distinguish between "this expression can't be evaluated because its value depends on an error" and "this expression doesn't have a meaningful type because its type depends on an error". Conclusions: **1. we should support delayed typo-correction in C by supporting dependence in C**, and **2. we should generalize what we mean by "dependent" to mean "depends on a template parameter or an error**.
- Computing 4 (with this patch, 5) bits of status summarizing properties of subexpressions is highly undesirable: it's error-prone, repetitive, and expensive. It would be better if we packed these five bits into an enum and had a single consistent mechanism for computing them based on the corresponding properties of subexpressions, and if we had some way to do that that didn't require complexity in every constructor for every AST node. (We could, for example, set the bits from the `FooExpr::Create` functions by calling one centralized function that walks the children and accumulates properties from them -- with all exceptions to that general pattern in a single place.) Conclusions: **3. it's time to refactor these bits; the proof that we've done this right will be that adding the fifth such bit is easy**.
- A transitive "does this or anything within it contain errors" flag has value. It may not be exactly the right information for any particular query, but finer-grained information (eg, "do we know the type of this expression?") can be obtained using the various dependence bits. Example: this bit lets us determine whether dependence means "depends on a template parameter" (bit is false) or whether it means "don't know what this depends on, suppress follow-on errors" (bit is true). Eg, we could stop producing a bogus diagnostic for a missing `template` keyword in `typo.function<int>()`. Example: this bit will permit us to distinguish between "function has a body and as far as we know it's valid" and "function has a body that contained errors", and suppress errors for (eg) missing return statements in a constexpr function, and likewise suppress control-flow-based warnings on function granularity rather than whole-compilation granularity. (These examples are all cases where we have a significant diagnostic quality problem today.)

Here's the plan we came up with:

1. (Notionally) generalize what we mean by "dependent" to mean "depends on a template parameter or on an error".
2. Refactor and centralize the computation and propagation of the dependence and "contains pack" bits.
3. Add the `containsErrors` bit (after (2) this should be easy) to all statements (not only expressions)
4. (Asynchronously) add support for dependent constructs to Clang's C-only codepaths and start doing delayed typo-correction (and preserving invalid subexpressions) even in C.

@ilya-biryukov Did I forget anything?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65591