[PATCH] D121824: [clang] Do not crash on arrow operator on dependent type.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 17 04:43:52 PDT 2022


hokein added a comment.

Thanks for looking at this. It seems an issue caused by https://reviews.llvm.org/D120812 (sorry), where we build a recovery-expr to model declrefexpr pointing to an invalid decl.



================
Comment at: clang/lib/Sema/TreeTransform.h:14709
     // -> is never a builtin operation.
+    if (First->getType()->isDependentType())
+      return ExprError();
----------------
It looks like we are somehow doing a transformation on a dependent-type expression (I think it is the recoveryExpression) during the template instantiation, which leads to the crash.



================
Comment at: clang/test/SemaCXX/arrow-operator.cpp:79
+void foo() {
+  // x is dependent.
+  A<int>& x = blah[7];  // expected-error {{use of undeclared identifier 'blah'}} \
----------------
The AST nodes looks weird and inconsistent in the primary template and instantiated template.

VarDecl `x` AST node in the primary template look like (with a Valid bit!)

```
`-DeclStmt 
   `-VarDecl  x 'A<int> &'
```

while in the instantiated template (with an **Invalid** bit!):
```
 `-DeclStmt
    `-VarDecl invalid x 'A<int> &'
```

The difference of valid bit results in two different forms of the expression `x->call()`  (a normal CXXMemberCallExpr vs. a dependent-type CallExpr wrapping a RecoveryExpr), I think this is likely the root cause of the crash.

If we invalidate the VarDecl x in the primary template for this case, the issue should be gone. This seems a reasonable fix to me -- a reference-type VarDecl is invalid, when it doesn't have an initializer (either 1. it is not written in the source code or 2. it is too malformed to preserve ).  Clang AST already does 1.  We're missing 2, I think it is in `Sema::ActOnInitializerError`.




================
Comment at: clang/test/SemaCXX/arrow-operator.cpp:81
+  A<int>& x = blah[7];  // expected-error {{use of undeclared identifier 'blah'}} \
+                        // expected-error {{declaration of reference variable 'x' requires an initializer}}
+  x->call();
----------------
if we mark vardecl x invalid, I think the bogus `requires an initializer` diagnostic will be gone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121824



More information about the cfe-commits mailing list