rsmith added a comment.
I expect we'll want a `ContainsErrors` bit on `Stmt`, propagated similarly to
the existing `InstantiationDependent` / `ContainsUnexpandedParameterPack` / ...
bits. For example, the constant evaluator will want to quickly bail out of
evaluating expressions and statements containing errors, and the error recovery
`TreeTransform` that we perform to fix typos will want that too (and maybe
could be able to fix other kinds of errors for which we build these error nodes
eventually?).
================
Comment at: include/clang/AST/BuiltinTypes.def:265
+// a template.
+BUILTIN_TYPE(Recovery, RecoveryTy)
+
----------------
erik.pilkington wrote:
> Why are you creating a new type as opposed to just using DependentTy (sorta
> like TypoExpr does)? It seems like if you want to recycle all the
> dependence-propagating code in Sema, then you need to fall back to
> DependentTy anyways, i.e. `1 + <recovery-expr>` will have dependent type with
> this patch, right?
>
>
Using `DependentTy` for `TypoExpr` is a mistake; it leads to all sorts of bad
follow-on diagnostics incorrectly claiming that constructs have dependent types.
Rather than calling this `RecoveryTy`, I'd prefer to call it something a bit
more general like `ErrorTy`, with the intent being that we eventually use it
for `TypoExpr` too.
================
Comment at: include/clang/AST/Expr.h:5801-5809
+/// RecoveryExpr - a broken expression that we couldn't construct an AST for,
+/// e.g. because overload resolution failed.
+/// We capture:
+/// - the kind of expression that would have been produced
+/// - the valid subexpressions
+/// - the type, if known. If unknown, it is the built-in RecoveryTy, which
+/// is dependent (and so generally suppresses further diagnostics etc).
----------------
Do we need this at all, if we have a properly-propagated `ErrorTy` anyway?
Instead of using this, it would seem that we could model an ill-formed
expression as the corresponding AST node but with its type set to `ErrorTy`.
Presumably the latter is what we'll do when we find a subexpression containing
an error, rather than creating a `RecoveryExpr` at every enclosing syntactic
level, so AST clients will need to deal with that anyway.
================
Comment at: include/clang/AST/Type.h:2423-2424
+ : Type(Builtin, QualType(),
+ /*Dependent=*/(K == Dependent || K == Recovery),
+ /*InstantiationDependent=*/(K == Dependent || K == Recovery),
/*VariablyModified=*/false,
----------------
Experience with `TypoExpr` suggests that treating non-dependent constructs as
dependent is a mistake in the long term.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61722/new/
https://reviews.llvm.org/D61722
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
