[PATCH] D125611: DWARFVerifier: Change vector of IntervalMap to vector of unique_ptrs

[Dimitry Andric via Phabricator via llvm-commits]

dim added a subscriber: EricWF.
dim added a comment.

In D125611#3517574 <https://reviews.llvm.org/D125611#3517574>, @dblaikie wrote:

> Some more details on the specifics in the patch description would be good - I'm still not 100% clear on the reproduction case to understand what we're working around, exactly. Do you have a godbolt reproduction/small example, by chance?

Assuming there's a directory with llvm includes, this is enough to reproduce:

  #include "llvm/ADT/IntervalMap.h"
  #include <vector>
  
  void f(std::size_t size)
  {
    using namespace llvm;
  
    IntervalMap<uint32_t, uint64_t>::Allocator Alloc;
    std::vector<IntervalMap<uint32_t, uint64_t>> Sections(size, IntervalMap<uint32_t, uint64_t>(Alloc));
  }

which will (on FreeBSD at least, where libc++ is the standard C++ library) result in something like:

  $ clang++ -I llvm/include -c intervalmap.cpp
  In file included from intervalmap.cpp:1:
  In file included from llvm/include/llvm/ADT/IntervalMap.h:108:
  In file included from llvm/include/llvm/ADT/SmallVector.h:19:
  In file included from /usr/include/c++/v1/algorithm:643:
  /usr/include/c++/v1/memory:1809:31: error: call to implicitly-deleted copy constructor of 'llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>'
              ::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
                                ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /usr/include/c++/v1/memory:1687:21: note: in instantiation of function template specialization 'std::__1::allocator<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>>::construct<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>, const llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>> &>' requested here
                  __a.construct(__p, _VSTD::forward<_Args>(__args)...);
                      ^
  /usr/include/c++/v1/memory:1538:14: note: in instantiation of function template specialization 'std::__1::allocator_traits<std::__1::allocator<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>>>::__construct<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>, const llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>> &>' requested here
              {__construct(__has_construct<allocator_type, _Tp*, _Args...>(),
               ^
  /usr/include/c++/v1/vector:1066:25: note: in instantiation of function template specialization 'std::__1::allocator_traits<std::__1::allocator<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>>>::construct<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>, const llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>> &>' requested here
          __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__pos), __x);
                          ^
  /usr/include/c++/v1/vector:1159:9: note: in instantiation of member function 'std::__1::vector<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>, std::__1::allocator<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>>>::__construct_at_end' requested here
          __construct_at_end(__n, __x);
          ^
  intervalmap.cpp:9:48: note: in instantiation of member function 'std::__1::vector<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>, std::__1::allocator<llvm::IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>>>::vector' requested here
    std::vector<IntervalMap<uint32_t, uint64_t>> Sections(size, IntervalMap<uint32_t, uint64_t>(Alloc));
                                                 ^
  llvm/include/llvm/ADT/IntervalMap.h:970:14: note: copy constructor of 'IntervalMap<unsigned int, unsigned long, 12, llvm::IntervalMapInfo<unsigned int>>' is implicitly deleted because variant field 'leaf' has a non-trivial copy constructor
      RootLeaf leaf;
               ^
  1 error generated.

Ultimately, this is caused by `_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR` in `/usr/include/c++/v1/__config` (this was last modified by @EricWF, but a long time ago). If this setting is enabled (to preserve the old ABI), `std::pair` will always have a non-trivial copy constructor, and since `IntervalMap` contains a `std::pair` (via `RootLeaf` -> `IntervalMapImpl::LeafNode` -> `NodeBase<std::pair<KeyT, KeyT>, ValT, N>`), `IntervalMap` itself is also non-trivially copyable. `std::vector` in turn requires its value type to be copy-constructible (at least certainly when you use the constructor with a size).

I think this patch, using a unique_ptr sidesteps this issue by letting unique_ptr taking care of things. Another way would be to provide a hand-written copy constructor or move constructor for `IntervalMap`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125611