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

[David Blaikie via cfe-dev]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150818/710af472/attachment.html>