<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 17, 2016 at 11:25 AM Vedant Kumar <<a href="mailto:vsk@apple.com">vsk@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> If the move ctor/assign is noexcept, it should be OK as-is (without removing the copy ops).<br class="gmail_msg">
<br class="gmail_msg">
Thanks for sharing this, I did not know about this facet of 'noexcept'.<br class="gmail_msg">
<br class="gmail_msg">
> I'd probably avoid the extra complexity & leaev the code as it was before.<br class="gmail_msg">
<br class="gmail_msg">
I'd like to keep the {Input,Output}FunctionCoverageData classes consistent in<br class="gmail_msg">
whether/not they are copyable. Since I'd also prefer for the move constructor<br class="gmail_msg">
to kick in during, e.g vector::resize, do you think the cleanest solution would<br class="gmail_msg">
be to make both classes copyable but mark the move methods 'LLVM_NOEXCEPT'?<br class="gmail_msg"></blockquote><div><br>Really if you have to write them down - you're in about the same place as you are today. (though side note: you can just write the move ops as you have already, and omit the copy ops - they'll be implicitly omitted)<br><br>In theory they should be noexcept already if everything they depend on is noexcept - so the right fix is probably to make sure the underlying data structures are noexcept (maybe they can't be? - SmallVector might have to copy on move (if it's in the small mode), etc... )<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
vedant<br class="gmail_msg">
<br class="gmail_msg">
> On Oct 17, 2016, at 11:07 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> On Mon, Oct 17, 2016 at 11:02 AM Vedant Kumar <<a href="mailto:vsk@apple.com" class="gmail_msg" target="_blank">vsk@apple.com</a>> wrote:<br class="gmail_msg">
> While we didn't have any cases where we'd copy InputFunctionCoverageData<br class="gmail_msg">
> objects, I wanted to prevent these copies from ever occurring because they<br class="gmail_msg">
> would be expensive. Each object contains a SmallDenseMap<unsigned, unsigned><br class="gmail_msg">
> and a std::vector<CounterMappingRegion>.<br class="gmail_msg">
><br class="gmail_msg">
> This commit was motivated by a case where we called std::vector::resize on a<br class="gmail_msg">
> vector of OutputFunctionCoverageData. Deleting the copy constructor in that<br class="gmail_msg">
> case should have made the resize operation cheaper (caveat - I haven't actually<br class="gmail_msg">
> measured this).<br class="gmail_msg">
><br class="gmail_msg">
> 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... :/<br class="gmail_msg">
><br class="gmail_msg">
> 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.<br class="gmail_msg">
><br class="gmail_msg">
> - Dave<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> vedant<br class="gmail_msg">
><br class="gmail_msg">
> > On Oct 17, 2016, at 9:58 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<br class="gmail_msg">
> ><br class="gmail_msg">
> > Were these copies problematic in any way? If it's just a POD type, you might as well leave it copyable, etc?<br class="gmail_msg">
> ><br class="gmail_msg">
> > On Wed, Oct 12, 2016 at 3:53 PM Vedant Kumar via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg">
> > Author: vedantk<br class="gmail_msg">
> > Date: Wed Oct 12 17:44:50 2016<br class="gmail_msg">
> > New Revision: 284069<br class="gmail_msg">
> ><br class="gmail_msg">
> > URL: <a href="http://llvm.org/viewvc/llvm-project?rev=284069&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=284069&view=rev</a><br class="gmail_msg">
> > Log:<br class="gmail_msg">
> > [unittests] Delete even more copy constructors (NFC)<br class="gmail_msg">
> ><br class="gmail_msg">
> > Modified:<br class="gmail_msg">
> >     llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp<br class="gmail_msg">
> ><br class="gmail_msg">
> > Modified: llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp<br class="gmail_msg">
> > URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp?rev=284069&r1=284068&r2=284069&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp?rev=284069&r1=284068&r2=284069&view=diff</a><br class="gmail_msg">
> > ==============================================================================<br class="gmail_msg">
> > --- llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp (original)<br class="gmail_msg">
> > +++ llvm/trunk/unittests/ProfileData/CoverageMappingTest.cpp Wed Oct 12 17:44:50 2016<br class="gmail_msg">
> > @@ -106,6 +106,16 @@ struct InputFunctionCoverageData {<br class="gmail_msg">
> ><br class="gmail_msg">
> >    InputFunctionCoverageData(std::string Name, uint64_t Hash)<br class="gmail_msg">
> >        : Name(std::move(Name)), Hash(Hash) {}<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +  InputFunctionCoverageData(InputFunctionCoverageData &&IFCD)<br class="gmail_msg">
> > +      : ReverseVirtualFileMapping(std::move(IFCD.ReverseVirtualFileMapping)),<br class="gmail_msg">
> > +        Name(std::move(IFCD.Name)), Hash(IFCD.Hash),<br class="gmail_msg">
> > +        Regions(std::move(IFCD.Regions)) {}<br class="gmail_msg">
> > +<br class="gmail_msg">
> > +  InputFunctionCoverageData(const InputFunctionCoverageData &) = delete;<br class="gmail_msg">
> > +  InputFunctionCoverageData &<br class="gmail_msg">
> > +  operator=(const InputFunctionCoverageData &) = delete;<br class="gmail_msg">
> > +  InputFunctionCoverageData &operator=(InputFunctionCoverageData &&) = delete;<br class="gmail_msg">
> >  };<br class="gmail_msg">
> ><br class="gmail_msg">
> >  struct CoverageMappingTest : ::testing::Test {<br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> > _______________________________________________<br class="gmail_msg">
> > llvm-commits mailing list<br class="gmail_msg">
> > <a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a><br class="gmail_msg">
> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" class="gmail_msg" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="gmail_msg">
><br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>