r245352 - Workaround -Wdeprecated on SemDiagnosticConsumer's tricksy copy ctor.

[David Blaikie via cfe-commits]

On Tue, Aug 18, 2015 at 4:46 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> On Tue, Aug 18, 2015 at 2:03 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> Richard, do you think there's anything we could do better here?
>>
>> It seems difficult to support proper move semantic behavior for
>> [Sema]DiagnosticBuilder across the two common use cases:
>>
>>   DiagnosticBuilder D;
>>   D << x;
>>   D << y;
>>   return D;
>>
>> and
>>
>>   return DiagnosticBuilder() << x << y;
>>
>
> Do we actually use this form to return a DiagnosticBuilder, or just to
> construct an ActionResult? Can we eliminate uses of this form?
>

We don't use it very often outside of "return S.Diag(...) << x << y" - 3
cases (each in disparate parts of the codebase) in Clang, 5 in Clang-Tidy.

For the Sema case, there seemed to be enough uses that I've experimented
with making Sema::Diag variadic and taking the extra parameters: "return
S.Diag(..., x, y);" which seems to work pretty well. Sent for review in
http://reviews.llvm.org/D12131


>
>
>> The only thing I can imagine is if every one of those op<< were function
>> templates using universal references (I forget, is that the right name for
>> them?) and matching their return value (so in the first case, passed a
>> non-const lvalue ref, they return by non-const lvalue ref, and in the
>> second case, passed an rvalue, they return the same). But that seems
>> painful.
>>
>
> It seems tricky to form an unambiguous overload set for this that
> preserves the value category, without duplicating all the operator<<s.
>

Yep :/


>
>
>> On Tue, Aug 18, 2015 at 1:54 PM, David Blaikie via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: dblaikie
>>> Date: Tue Aug 18 15:54:26 2015
>>> New Revision: 245352
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=245352&view=rev
>>> Log:
>>> Workaround -Wdeprecated on SemDiagnosticConsumer's tricksy copy ctor.
>>>
>>> Modified:
>>>     cfe/trunk/include/clang/Sema/Sema.h
>>>
>>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=245352&r1=245351&r2=245352&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue Aug 18 15:54:26 2015
>>> @@ -1054,6 +1054,14 @@ public:
>>>      SemaDiagnosticBuilder(DiagnosticBuilder &DB, Sema &SemaRef,
>>> unsigned DiagID)
>>>        : DiagnosticBuilder(DB), SemaRef(SemaRef), DiagID(DiagID) { }
>>>
>>> +    // This is a cunning lie. DiagnosticBuilder actually performs move
>>> +    // construction in its copy constructor (but due to varied uses,
>>> it's not
>>> +    // possible to conveniently express this as actual move
>>> construction). So
>>> +    // the default copy ctor here is fine, because the base class
>>> disables the
>>> +    // source anyway, so the user-defined ~SemaDiagnosticBuilder is a
>>> safe no-op
>>> +    // in that case anwyay.
>>> +    SemaDiagnosticBuilder(const SemaDiagnosticBuilder&) = default;
>>> +
>>>      ~SemaDiagnosticBuilder() {
>>>        // If we aren't active, there is nothing to do.
>>>        if (!isActive()) return;
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150818/b35b2669/attachment.html>