rsmith added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:12268
/// of sanity.
void Sema::ActOnInitializerError(Decl *D) {
// Our main concern here is re-establishing invariants like "a
----------------
hokein wrote:
> sammccall wrote:
> > rsmith wrote:
> > > Should we attach a `RecoveryExpr` initializer in this case too?
> > Now we're really slipping into a different set of use-cases for
> > RecoveryExpr...
> > I assume we're talking about a RecoveryExpr that has no children, at least
> > in the short term, as parsing failures don't return the likely
> > subexpressions found. So it's a pure error marker in the form of an AST
> > node. Quite a lot of ExprError()s could become these...
> >
> > Actually there's another virtue here: even without subexpressions, the
> > RecoveryExpr can capture the token range. So VarDecl::getSourceRange() will
> > include the malformed initializer, so tools can at least attribute these
> > tokens to the right part of the AST.
> yeah, I'm not sure how much benefit we can get from the recovery-expr in this
> case, if the initializer is failed to parse, we don't have any ast nodes.
What I'm thinking is that we should have some kind of marker in the AST to
track that an initializer was provided for the variable but was malformed.
Right now, we use the "invalid" bit for that, which means that downstream code
that refers to the variable will have poor recovery. If we attach an
initialization expression marked with the "contains errors" bit instead, then
we can keep the `VarDecl` valid, and improve the recovery not necessarily for
this node but for things downstream of it.
(I guess ultimately it seems reasonable to me to use the same AST
representation for "no initializer was provided and implicit default init
failed", "an initializer was provided but we couldn't parse / semantically
analyze it", and "an initializer was provided but was incompatible with the
variable's type" -- except that in the third case we can track the expression
that we formed for the initializer.)
I don't think there's any urgency to this, and having both AST models for a
while (in different cases) doesn't seem like it's going to cause much pain.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78100/new/
https://reviews.llvm.org/D78100
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
