vsapsai added inline comments.

================
Comment at: clang/lib/Sema/SemaDecl.cpp:15055
           }
+          // If it is the last field is checked elsewhere.
         }
----------------
rjmccall wrote:
> vsapsai wrote:
> > rjmccall wrote:
> > > "Whether" rather than "If", please.  You should also leave a comment 
> > > about *why* we can't check this here — I assume because you also want to 
> > > complain about the last explicit ivar if there are synthesized ivars?  I 
> > > think we could at least still check this for `@interface` ivars.
> > Will change s/If/Whether/
> > 
> > Main reason for checking elsewhere is to check after ivars are synthesized, 
> > you are right. At some point I had this check done here but for detecting 
> > ivar-after-flexible-array on interface/extension, interface/implementation 
> > border I am relying on chained ObjCIvarDecl. But here I have `ArrayRef<Decl 
> > *> Fields` so implementation will be different. I decided that it would be 
> > cleaner to perform the check only in DiagnoseVariableSizedIvars.
> Is there a reason to do any of the checking here, then?
No objective reason. I updated isIncompleteArrayType branch to avoid flexible 
array members rejected at line 15023

```lang=c++
    } else if (!FDTy->isDependentType() &&
               RequireCompleteType(FD->getLocation(), FD->getType(),
                                   diag::err_field_incomplete)) {
```

and here I've added it for consistency. Will move to DiagnoseVariableSizedIvars 
and see if it works fine, don't expect any problems.


https://reviews.llvm.org/D38773



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

Reply via email to