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