[PATCH] D142446: [MC] Disable copying and moving of MCInstrDescs the C++20 way

Ilya Biryukov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 24 03:13:12 PST 2023


ilya-biryukov added a comment.

Ah, I keep forgetting that the unknown attributes are fine. My experiments with MSVC suggest that it will increase the struct size: https://gcc.godbolt.org/z/offYrhn4K.
libc++ uses `msvc::no_unique_address`, which seems to work, but is MSVC-specific.

We should hide the `[[no_unique_address]]` behind a macro, we do the same for LLVM_REQUIRE_CONSTANT_INITIALIZATION <https://github.com/llvm/llvm-project/blob/89427bb7cbd104cd04be9dd75acf18f30d980e74/llvm/include/llvm/Support/Compiler.h#L276> and LLVM_FALLTHROUGH <https://github.com/llvm/llvm-project/blob/89427bb7cbd104cd04be9dd75acf18f30d980e74/llvm/include/llvm/Support/Compiler.h#L262>.
This should allow to use MSVC-specific version and avoid warnings in compilers that don't support `no_unique_address` (I'm not entirely certain, but suspect LLVM might still require building with those).



================
Comment at: llvm/include/llvm/MC/MCInstrDesc.h:193
 
+namespace {
+// An empty struct that can be used as a trailing member to prevent another
----------------
We should not use an anonymous namespace in headers.
This causes `MCInstrDesc` to have a unique declaration in each `.cpp` file it's used, leading to ODR violations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142446



More information about the llvm-commits mailing list