[llvm] r284069 - [unittests] Delete even more copy constructors (NFC)

[Vedant Kumar via llvm-commits]

> If the move ctor/assign is noexcept, it should be OK as-is (without removing the copy ops).

Thanks for sharing this, I did not know about this facet of 'noexcept'.

> I'd probably avoid the extra complexity & leaev the code as it was before.

I'd like to keep the {Input,Output}FunctionCoverageData classes consistent in
whether/not they are copyable. Since I'd also prefer for the move constructor
to kick in during, e.g vector::resize, do you think the cleanest solution would
be to make both classes copyable but mark the move methods 'LLVM_NOEXCEPT'?

vedant

> On Oct 17, 2016, at 11:07 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Mon, Oct 17, 2016 at 11:02 AM Vedant Kumar <vsk at apple.com> wrote:
> While we didn't have any cases where we'd copy InputFunctionCoverageData
> objects, I wanted to prevent these copies from ever occurring because they
> would be expensive. Each object contains a SmallDenseMap<unsigned, unsigned>
> and a std::vector<CounterMappingRegion>.
> 
> This commit was motivated by a case where we called std::vector::resize on a
> vector of OutputFunctionCoverageData. Deleting the copy constructor in that
> case should have made the resize operation cheaper (caveat - I haven't actually
> measured this).
> 
> If the move ctor/assign is noexcept, it should be OK as-is (without removing the copy ops). We (C++/Clang/etc) don't really have a good answer for this in a codebase without exceptions anyway... :/
> 
> This is pretty far outside any of the code I deal with, so do whatever works best for you, but I'd probably avoid the extra complexity & leaev the code as it was before.
> 
> - Dave
>  
> 
> vedant
> 
> > On Oct 17, 2016, at 9:58 AM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Were these copies problematic in any way? If it's just a POD type, you might as well leave it copyable, etc?
> >
> > On Wed, Oct 12, 2016 at 3:53 PM Vedant Kumar via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> > Author: vedantk
> > Date: Wed Oct 12 17:44:50 2016
> > New Revision: 284069
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=284069&view=rev
> > Log:
> > [unittests] Delete even more copy constructors (NFC)
> >
> > Modified:
> >     llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
> >
> > Modified: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp?rev=284069&r1=284068&r2=284069&view=diff
> > ==============================================================================
> > --- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp (original)
> > +++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp Wed Oct 12 17:44:50 2016
> > @@ -106,6 +106,16 @@ struct InputFunctionCoverageData {
> >
> >    InputFunctionCoverageData(std::string Name, uint64_t Hash)
> >        : Name(std::move(Name)), Hash(Hash) {}
> > +
> > +  InputFunctionCoverageData(InputFunctionCoverageData &&IFCD)
> > +      : ReverseVirtualFileMapping(std::move(IFCD.ReverseVirtualFileMapping)),
> > +        Name(std::move(IFCD.Name)), Hash(IFCD.Hash),
> > +        Regions(std::move(IFCD.Regions)) {}
> > +
> > +  InputFunctionCoverageData(const InputFunctionCoverageData &) = delete;
> > +  InputFunctionCoverageData &
> > +  operator=(const InputFunctionCoverageData &) = delete;
> > +  InputFunctionCoverageData &operator=(InputFunctionCoverageData &&) = delete;
> >  };
> >
> >  struct CoverageMappingTest : ::testing::Test {
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>