vsavchenko accepted this revision.
vsavchenko added a comment.
This revision is now accepted and ready to land.
Great! Thanks for addressing all of the comments!
================
Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:185-186
+ return FD->getType()->isReferenceType();
+ } else {
+ assert(false && "Unknown captured variable");
+ }
----------------
AbbasSabra wrote:
> vsavchenko wrote:
> > AbbasSabra wrote:
> > > vsavchenko wrote:
> > > > But actually, it's a bit different with this one. I don't know exact
> > > > details and rules how clang sema fills in the class for lambda.
> > > > According to [[ https://en.cppreference.com/w/cpp/language/lambda |
> > > > cppreference ]]:
> > > > > For the entities that are captured by reference (with the default
> > > > > capture [&] or when using the character &, e.g. [&a, &b, &c]), it is
> > > > > unspecified if additional data members are declared in the closure
> > > > > type
> > > >
> > > > It can be pretty much specified in clang, that's true, but it looks
> > > > like in `DeadStoreChecker` we have a very similar situation and we do
> > > > not assume that captured variable always have a corresponding field.
> > > Yes, I based this on the fact that DeadStoreChecker considers it
> > > possible, but I have never faced a case where it does not have a
> > > corresponding field.
> > It still would be good to have some proof that it is indeed like this or
> > simply fallback into returning true (which you already do when in doubt).
> As I said, I believe it cannot happen but I assumed based on the other
> checker that there is something I don't know.
> I think based on !!getCaptureFields!! the implementation we are iterating
> over all captures. For that algorithm to work number of captures should be
> equal to number of fields
> ```
> void CXXRecordDecl::getCaptureFields(
> llvm::DenseMap<const VarDecl *, FieldDecl *> &Captures,
> FieldDecl *&ThisCapture) const {
> Captures.clear();
> ThisCapture = nullptr;
>
> LambdaDefinitionData &Lambda = getLambdaData();
> RecordDecl::field_iterator Field = field_begin();
> for (const LambdaCapture *C = Lambda.Captures, *CEnd = C +
> Lambda.NumCaptures;
> C != CEnd; ++C, ++Field) {
> if (C->capturesThis())
> ThisCapture = *Field;
> else if (C->capturesVariable())
> Captures[C->getCapturedVar()] = *Field;
> }
> assert(Field == field_end());
> }
> ```
>
> I dropped the defensive code
OK, sounds reasonable!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102273/new/
https://reviews.llvm.org/D102273
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
