hokein added a comment.
Sorry, I thought this crash is fixed as a bonus effort by improving the AST for
reference-type var-decl.
Beyond the crash cased by the dependent-type `RecoveryExpr` built in
`Sema::BuildDeclarationNameExpr`, I think there is another issue which is worth
fixing: whether we should mark the reference-type var-decl valid if it doesn't
have an initializer (either the initializer is not provided in the source code
or too broken to preserve). The current AST behavior is inconsistent:
int &a; // var-decl a is invalid without an initializer
int &b = undefine[1111]; // var-decl b is valid without an initializer
int &c = undefine; // var-decl c is valid with a recovery-expr initializer
From the AST point of view, there is no difference between `a` and `b`, they
both don't have an initializer, but their valid bits are different. IMO marking
b invalid is an improvement, the valid bit is consistent for refer-type
var-decls without initializers; it avoids the bogus `requires an initializer`
diagnostic during template instantiations; as a bonus, it seems to "fix" the
crash (at least for the original testcase).
Another option would be to mark `a` valid regardless of the initializer. Pros:
preserve more AST nodes for `a`; the valid bit are consistent among three
cases. Cons: unknown? whether it'll break some subtle invariants in clang; the
bogus `requires an initializer` diagnostic is still there.
But yeah, this is a separate issue, we should fix the crash first.
================
Comment at: clang/lib/Sema/TreeTransform.h:14708
} else if (Op == OO_Arrow) {
+ if (First->getType()->isDependentType())
+ return ExprError();
----------------
I'm not sure this is the only place triggering the crash, it looks like that
we're fixing a symptom.
While here, `First` refers to a dependent-type `RecoveryExpr` (which is built
from the code path: `TransformDeclRefExpr` -> `RebuildDeclRefExpr`->
`Sema::BuildDeclarationNameExpr`). So I think we have a high chance that the
`RecoveryExpr` will spread multiple places in the `TreeTransform` and cause
similar violations in other places.
A safe fix will be to not build the `RecoveryExpr` in
`TreeTransform::TransformDeclRefExpr` -- `Sema::BuildDeclarationNameExpr` has a
`AcceptInvalidDecl` parameter (by default it is false), we could reuse it to
control whether we should build a `RecoveryExpr`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121824/new/
https://reviews.llvm.org/D121824
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
