NoQ added inline comments.
================
Comment at: lib/Analysis/CFG.cpp:3899
+ if (auto *CE = const_cast<CXXConstructExpr *>(NE->getConstructExpr()))
+ CurrentConstructionContext = {/*Constructor=*/CE, /*Trigger=*/NE};
+
----------------
NoQ wrote:
> dcoughlin wrote:
> > Is it possible that there is already a CurrentConstructionContext? Will
> > this overwrite it?
> >
> > Can you write an assertion to make sure that this doesn't happen?
> >
> > Generally I would expect a context to have a well-defined start and end. I
> > think we should be explicit about where we expect it to start and end so we
> > can find violations of our expectations.
> Yes, for now it'd be overwritten every time we meet a new constructor (this
> wasn't the case in the first version of this revision). However, the
> additional check (which should be eventually replaced with an assertion, but
> is for now needed) that we're picking the right context in `CXXConstructExpr`
> visit, together with only visiting every constructor once during CFG
> construction during normal visits (because weird stuff like
> `CXXDefaultArgExpr` isn't yet supported), guarantees that we're not doing
> anything wrong.
>
> I should have cleaned this up, but i don't want to invest attention into that
> because subsequent patches will clearly make things way more complex than
> that, whenever we start dealing with a multitude of construction contexts or
> with partially-constructed contexts.
>
> So for now it's a trivial kinda-safe solution.
>
> I should document that though, for sure.
Fixed now - added the `VisitForConstructionContext()` wrapper which contains
the assertion that checks that we're not overwriting any existing context.
The context is being cleaned up after it's consumed in `appendConstructor()`.
https://reviews.llvm.org/D42672
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
