On Thu, Dec 6, 2012 at 8:18 PM, Jordan Rose <[email protected]> wrote:
> Nice! > > On Dec 6, 2012, at 11:09 , Benjamin Kramer <[email protected]> > wrote: > > > Author: d0k > > Date: Thu Dec 6 13:09:30 2012 > > New Revision: 169535 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=169535&view=rev > > Log: > > Add move semantics to PartialDiagnostic, which can be very expensive to > copy. > > > > Modified: > > cfe/trunk/include/clang/Basic/PartialDiagnostic.h > > > > Modified: cfe/trunk/include/clang/Basic/PartialDiagnostic.h > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PartialDiagnostic.h?rev=169535&r1=169534&r2=169535&view=diff > > > ============================================================================== > > --- cfe/trunk/include/clang/Basic/PartialDiagnostic.h (original) > > +++ cfe/trunk/include/clang/Basic/PartialDiagnostic.h Thu Dec 6 > 13:09:30 2012 > > @@ -19,6 +19,7 @@ > > #include "clang/Basic/Diagnostic.h" > > #include "clang/Basic/SourceLocation.h" > > #include "llvm/ADT/STLExtras.h" > > +#include "llvm/Support/Compiler.h" > > #include "llvm/Support/DataTypes.h" > > #include <cassert> > > > > @@ -200,6 +201,14 @@ > > } > > } > > > > +#if LLVM_HAS_RVALUE_REFERENCES > > + PartialDiagnostic(PartialDiagnostic &&Other) > > + : DiagID(Other.DiagID), DiagStorage(Other.DiagStorage), > > + Allocator(Other.Allocator) { > > + Other.DiagStorage = 0; > > + } > > +#endif > > + > > PartialDiagnostic(const PartialDiagnostic &Other, Storage *DiagStorage) > > : DiagID(Other.DiagID), DiagStorage(DiagStorage), > > Allocator(reinterpret_cast<StorageAllocator *>(~uintptr_t(0))) > > @@ -242,6 +251,23 @@ > > return *this; > > } > > > > +#if LLVM_HAS_RVALUE_REFERENCES > > + PartialDiagnostic &operator=(PartialDiagnostic &&Other) { > > + if (this != &Other) { > > Is this really necessary? If this is true, I think the move contract has > been violated anyway. > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > There was a discussion on stack overflow recently about this, and Howard chimed in: it is unnecessary. On the other hand, a `assert(this != &Other && "Illegal attempt to move toward itself")` may make sense since this can quickly become a gotcha. -- Matthieu
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
