[cfe-dev] Fwd: Code carefully copying LookupResults which aren't exactly copyable

Kaelyn Takata via cfe-dev cfe-dev at lists.llvm.org
Wed Aug 19 17:19:38 PDT 2015


As I recall, the capture of the LookupResult (which was believed to be
safe) was a convenient way of capturing the lookup-parameter state of the
LookupResult--the identifier being looked up, it's location, the lookup
kind, etc--instead of breaking them out into a bunch of separate local
variables to be captured by the lambda and used to rebuild a LookupResult
suitable for passing into SemaRef.BuildMemberReferenceExpr (which IIRC
depends on the LookupResult telling it whether overload resolution is
necessary).

On Tue, Aug 18, 2015 at 8:52 PM, David Blaikie via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> Perhaps I should remove the old lists from my contacts...
>
> ---------- Forwarded message ----------
> From: David Blaikie <dblaikie at gmail.com>
> Date: Tue, Aug 18, 2015 at 8:39 PM
> Subject: Code carefully copying LookupResults which aren't exactly copyable
> To: Richard Smith <richard at metafoo.co.uk>, Kaelyn Takata <rikka at google.com>,
> cfe-dev Developers <cfe-dev at cs.uiuc.edu>
>
>
> More -Wdeprecated cleanup.
>
> Turns out LookupResults are copied in at least one place:
>
> SemaExprMember.cpp:651
>
> Essentially this function takes a LookupResult, captures it by value into
> a lambda, within the body of the lambda it suppresses diagnostics on the
> LookupResult and then does some other stuff.
>
> LookupResult's dtor does a couple of things - it emits diagnostics, if
> needed. And it destroys the CXXBasePaths in the LookupResult. As
> luck/design would have it, the LookupResult that is copied here never has a
> non-null CXXBasePaths and, as mentioned, always disables diagnostics (if
> the lambda is actually called - if it is not, maybe bad things could
> happen? It could emit diagnostics unintentionally?).
>
> Having LookupResult be copyable in general seems problematic - if extra
> care is not taken it could easily double delete and/or emit excess
> diagnostics.
>
> Any ideas how this might be improved?
>
> Honestly I seem to recall being involved in the code review, and maybe I
> even suggested this approach, not knowing what monsters lurked beneath.
> Maybe the original code ferried just the right LookupResult parameters
> across the capture boundary? That would be safe & then we could make
> LookupResult non-copyable again. (& can unique_ptr the CXXBasePaths (or
> even Optional<CXXBasePaths>) eventually)
>
> - Dave
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150819/ce914130/attachment.html>


More information about the cfe-dev mailing list