[PATCH] D125899: [ADT] Add copy constructor to IntervalMap

[Dimitry Andric via Phabricator via llvm-commits]

dim added a comment.

In D125899#3522946 <https://reviews.llvm.org/D125899#3522946>, @dblaikie wrote:

> In D125899#3522879 <https://reviews.llvm.org/D125899#3522879>, @kparzysz wrote:
>
>> Changed the copy to a deep one.
>
> Ah, if the type wasn't deep copying before - ie: its default copy operation wasn't actually valid/probably would've resulted in duplicate free/delete/etc and generally been broken. Then my use case was just getting lucky because it was copying empty maps.
>
> If that's the case, then maybe that does argue in favor of the unique_ptr solution/marking the broken copy ctor as deleted. (or figuring out some way to do the construction in-place in my case without using copying)
>
> I'll take a closer look.

There's a subtle difference between the two std::vector constructors, mentioned at https://en.cppreference.com/w/cpp/container/vector/vector, where variant 3 is "Constructs the container with count copies of elements with value value" (so it needs a copy constructor), and variant 4 is "Constructs the container with count default-inserted instances of T. No copies are made". E.g. the latter should not require a copy constructor, but just a default constructor.

However, I'm unsure how you can 'choose' between those two variants. :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125899/new/

https://reviews.llvm.org/D125899