[PATCH] D43067: Implement equal_range for the DWARF v5 accelerator table

[David Blaikie via llvm-commits]

On Fri, Feb 9, 2018 at 3:15 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> labath added inline comments.
>
>
> ================
> Comment at: include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h:290-295
> +    Entry(const Entry &RHS) : NameIdx(RHS.NameIdx), Abbr(RHS.Abbr) {
> +      Values = RHS.Values;
> +    }
> +    Entry(Entry &&RHS) : NameIdx(RHS.NameIdx), Abbr(RHS.Abbr) {
> +      Values = std::move(RHS.Values);
> +    }
> ----------------
> dblaikie wrote:
> > Could these be defaulted (maybe even implicitly defaulted)?
> These ones are a bit tricky. They could be made default, but then I'd have
> to un-delete the super class implementations (which I deleted as a standard
> precaution for polymorphic classes). Maybe I could make the super class
> functions protected to make sure they're not invoked accidentally ?
>

Yeah, if you want derived classes to be copy/movable in any way, I'd
suggest making the base class operations protected and also make the
derived classes final (not their operations - let those be implicit). That
way someone can't introduce a class derived from the derived classes that
can accidentally be sliced by the intermediate class's copy/move ops.


>
> (The reason I need these constructors in the first place is so I can
> return the object as a value from the parsing function without resorting to
> unique_ptrs and such. I did feel it a bit weird while doing it as this
> isn't standard practice for polymorphic classes, but it seemed like a nice
> optimization.)
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D43067
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180209/e6f5a077/attachment.html>