On Tue, Aug 18, 2015 at 4:46 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Tue, Aug 18, 2015 at 2:03 PM, David Blaikie <dblai...@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@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@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
