jgorbe added a comment.

Added a test for the "capturing an array of incomplete type" case. See also 
responses to inline comments below.



================
Comment at: clang/lib/Sema/SemaExpr.cpp:14946
+  // as invalid as well.
+  if (const CXXRecordDecl *RD = FieldType->getAsCXXRecordDecl()) {
+    if (RD->isInvalidDecl()) {
----------------
rsmith wrote:
> jgorbe wrote:
> > rsmith wrote:
> > > You'll need to use `FieldType->getBaseElementTypeUnsafe()` or similar 
> > > here to handle the case where the field type is an array whose element 
> > > type is a class. (That can happen in a lambda if you capture a local 
> > > array variable by value.)
> > Done, I ended up using Context.getBaseElementType because the call to 
> > RequireCompleteType below required a QualType.
> Hah, OK, now you've switched to using `RequireCompleteType` this has become 
> redundant again (it'll walk to the base element type for you). =)
Removed call to `getBaseElementType()`, changed appearances of `EltTy` to 
`FieldType` instead.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:14952-14958
+      NamedDecl *Def;
+      EltTy->isIncompleteType(&Def);
+      if (Def && Def->isInvalidDecl()) {
+        Lambda->setInvalidDecl();
+        Field->setInvalidDecl();
+      }
+    }
----------------
rsmith wrote:
> I believe the `else` case here is redundant: if `RequireCompleteType` returns 
> `false`, the type is complete and there's nothing more you need to do.
If I just remove the `else` block, the test case included with the patch keeps 
crashing. Is there any way a `Decl` can be invalid even after 
`RequireCompleteType` returns `false`?


https://reviews.llvm.org/D54550



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

Reply via email to