[mlir] `mlir::DialectRegistry` can't be copied

[makslevental]

Recall

class DialectRegistry {
  ...
private:
  llvm::MapVector<TypeID, std::unique_ptr<DialectExtensionBase>> extensions;
}

then

template <typename T> void wrap_copy(void *dst, const void *src) {
  new ((T *) dst) T(*(const T *) src);
}

foo() {
  (void)wrap_copy<mlir::DialectRegistry>;
}

will produce

/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algobase.h:385:18: error: object of type 'std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>>' cannot be assigned because its copy assignment operator is implicitly deleted
  385 |               *__result = *__first;
      |                         ^

on

(eudsl) mlevental@Shark17:~/dev_projects/eudsl/projects/eudsl-py/src$ clang-20 --version
Ubuntu clang version 20.0.0 (++20241108053735+b613a54075c6-1~exp1~20241108053914.2037)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/lib/llvm-20/bin

The copy offending copy comes from SmallVectorImpl::operator=(const SmallVectorImpl<T> &RHS):

llvm-project/llvm/include/llvm/ADT/SmallVector.h

Line 1017 in 9d85ba5

NewEnd = std::copy(RHS.begin(), RHS.begin()+RHSSize, this->begin());

I don't know what the correct solution is (should SmallVector be smarter?) but doing

DialectRegistry(const DialectRegistry&) = delete;

fixes (in the sense that wrap_copy reports a call to a deleted ctor, and/or std::is_copy_constructible<mlir::DialectRegistry>::value == false).

Full compile error stack follows.

---

In file included from /home/mlevental/dev_projects/eudsl/projects/eudsl-py/src/eudsl_ext.cpp:1:
In file included from /home/mlevental/miniconda3/envs/eudsl/lib/python3.11/site-packages/nanobind/include/nanobind/nanobind.h:32:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/stdexcept:39:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/string:53:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:39:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/ext/alloc_traits.h:34:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/alloc_traits.h:33:
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_construct.h:119:25: error: call to implicitly-deleted copy constructor of 'std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>>'
  119 |       ::new((void*)__p) _Tp(std::forward<_Args>(__args)...);
      |                         ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_uninitialized.h:120:11: note: in instantiation of function template specialization 'std::_Construct<std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>>, const std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> &>' requested here
  120 |             std::_Construct(std::__addressof(*__cur), *__first);
      |                  ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_uninitialized.h:137:16: note: in instantiation of function template specialization 'std::__do_uninit_copy<const std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *, std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *>' requested here
  137 |         { return std::__do_uninit_copy(__first, __last, __result); }
      |                       ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_uninitialized.h:185:2: note: in instantiation of function template specialization 'std::__uninitialized_copy<false>::__uninit_copy<const std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *, std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *>' requested here
  185 |         __uninit_copy(__first, __last, __result);
      |         ^
/home/mlevental/dev_projects/eudsl/llvm-install/include/llvm/ADT/SmallVector.h:356:10: note: in instantiation of function template specialization 'std::uninitialized_copy<const std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *, std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *>' requested here
  356 |     std::uninitialized_copy(I, E, Dest);
      |          ^
/home/mlevental/dev_projects/eudsl/llvm-install/include/llvm/ADT/SmallVector.h:1043:9: note: in instantiation of function template specialization 'llvm::SmallVectorTemplateBase<std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>>>::uninitialized_copy<const std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *, std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>> *>' requested here
 1043 |   this->uninitialized_copy(RHS.begin()+CurSize, RHS.end(),
      |         ^
/home/mlevental/dev_projects/eudsl/llvm-install/include/llvm/ADT/SmallVector.h:1238:27: note: in instantiation of member function 'llvm::SmallVectorImpl<std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>>>::operator=' requested here
 1238 |       SmallVectorImpl<T>::operator=(RHS);
      |                           ^
/home/mlevental/dev_projects/eudsl/llvm-install/include/llvm/ADT/MapVector.h:36:7: note: in instantiation of member function 'llvm::SmallVector<std::pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>>, 0>::SmallVector' requested here
   36 | class MapVector {
      |       ^
/home/mlevental/dev_projects/eudsl/projects/eudsl-py/src/eudsl_ext.cpp:282:9: note: in instantiation of function template specialization 'wrap_copy<mlir::DialectRegistry>' requested here
  282 |   (void)wrap_copy<mlir::DialectRegistry>;
      |         ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_pair.h:195:17: note: explicitly defaulted function was implicitly deleted here
  195 |       constexpr pair(const pair&) = default;    ///< Copy constructor
      |                 ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_pair.h:192:11: note: copy constructor of 'pair<mlir::TypeID, std::unique_ptr<mlir::DialectExtensionBase>>' is implicitly deleted because field 'second' has a deleted copy constructor
  192 |       _T2 second;                ///< The second member
      |           ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/unique_ptr.h:514:7: note: 'unique_ptr' has been explicitly marked deleted here
  514 |       unique_ptr(const unique_ptr&) = delete;

[0xddom]

This issue happens with OperationState as well.

Reduced example in Compiler Explorer

Error trace is from MLIR 18 tho I checked HEAD and the unique_ptr is still being used.

  cargo:warning=/opt/homebrew/opt/llvm@18/include/c++/v1/__memory/uninitialized_algorithms.h:67:38: error: call to implicitly-deleted copy constructor of 'std::unique_ptr<mlir::Region>'
  cargo:warning=   67 |       ::new (std::__voidify(*__idx)) _ValueType(*__ifirst);
  cargo:warning=      |                                      ^          ~~~~~~~~~
  cargo:warning=/opt/homebrew/opt/llvm@18/include/c++/v1/__memory/uninitialized_algorithms.h:82:24: note: in instantiation of function template specialization 'std::__uninitialized_copy<std::unique_ptr<mlir::Region>, const std::unique_ptr<mlir::Region> *, const std::unique_ptr<mlir::Region> *, std::unique_ptr<mlir::Region> *, std::__always_false>' requested here
  cargo:warning=   82 |   auto __result = std::__uninitialized_copy<_ValueType>(
  cargo:warning=      |                        ^
  cargo:warning=/opt/homebrew/opt/llvm@18/include/llvm/ADT/SmallVector.h:369:10: note: in instantiation of function template specialization 'std::uninitialized_copy<const std::unique_ptr<mlir::Region> *, std::unique_ptr<mlir::Region> *>' requested here
  cargo:warning=  369 |     std::uninitialized_copy(I, E, Dest);
  cargo:warning=      |          ^
  cargo:warning=/opt/homebrew/opt/llvm@18/include/llvm/ADT/SmallVector.h:1056:9: note: in instantiation of function template specialization 'llvm::SmallVectorTemplateBase<std::unique_ptr<mlir::Region>>::uninitialized_copy<const std::unique_ptr<mlir::Region> *, std::unique_ptr<mlir::Region> *>' requested here
  cargo:warning= 1056 |   this->uninitialized_copy(RHS.begin()+CurSize, RHS.end(),
  cargo:warning=      |         ^
  cargo:warning=/opt/homebrew/opt/llvm@18/include/llvm/ADT/SmallVector.h:1251:27: note: in instantiation of member function 'llvm::SmallVectorImpl<std::unique_ptr<mlir::Region>>::operator=' requested here
  cargo:warning= 1251 |       SmallVectorImpl<T>::operator=(RHS);
  cargo:warning=      |                           ^
  cargo:warning=/opt/homebrew/opt/llvm@18/include/mlir/IR/OperationSupport.h:984:3: note: in instantiation of member function 'llvm::SmallVector<std::unique_ptr<mlir::Region>, 1>::SmallVector' requested here
  cargo:warning=  984 |   OperationState(const OperationState &other) = default;
  cargo:warning=      |   ^
  cargo:warning=/opt/homebrew/opt/llvm@18/include/c++/v1/__memory/unique_ptr.h:190:55: note: copy constructor is implicitly deleted because 'unique_ptr<mlir::Region>' has a user-declared move constructor
  cargo:warning=  190 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 unique_ptr(unique_ptr&& __u) _NOEXCEPT
  cargo:warning=      |                                                       ^
  cargo:warning=/opt/homebrew/opt/llvm@18/include/c++/v1/__memory/unique_ptr.h:597:30: error: no viable conversion from 'llvm::iterator_range<mlir::ValueUserIterator<mlir::ResultRange::UseIterator, mlir::OpOperand>>' to 'int'
  cargo:warning=  597 |   return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...));
  cargo:warning=      |                              ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~

[makslevental]

Yes it's true I saw this with OperationState as well. I can send a PR for both that deletes the copy ctor but someone wiser re cpp will have to weigh in on whether that's the correct solution.

[0xddom]

Personally, I would follow the rule of 0 and remove both the copy and the move constructors in OperationState. Afaik the lifetime of the data it handles is already managed by something else so there is no need for having them. Don’t remember if it defines the destructor but there’s a chance it’s not needed as well.

Not sure about DialectRegistry because I’m not familiar with its internals.

Edit: typo