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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to