[PATCH] D142214: [MC] Do not copy MCInstrDescs. NFC.

Fri Jan 20 10:21:45 PST 2023

dblaikie added inline comments.

================
Comment at: llvm/include/llvm/MC/MCInstrDesc.h:200
 public:
+  MCInstrDesc(const MCInstrDesc &) = delete;
+
----------------
foad wrote:
> @dblaikie or any other C++ experts - does this seem reasonable? I was not sure if I should be following some kind of rule-of-3/5/7/11/13/... here. All I want to do is prevent users from //accidentally// writing `MCInstrDesc MCID = ...` instead of `const MCInstrDesc &MCID = ...`.
Generally I'd suggest that APIs shouldn't make awkward ergonomic choices to reduce the chance of accidents - there's lots of things that can be expensive to copy in C++ that still have the usual copy ctor/assignment operator.

So if the thing is copyable - forcing users to do that copy in some uncommon way because the usual way is too error prone is, imho, not the right way to go - though I realize trying to teach people that this thing is expensive to copy isn't easy either. Hopefully updating a bunch of places that were copying will help set the right expectations.

If it's really non-copyable in any way, then, yes, the rule of 5 would apply. Implement the copy ctor, copy assignment, move ctor, move assignment, and destructor.

Sounds like deleting all the copy/move operations is what you want? Leave the dtor defaulted, maybe default the default (no-args) ctor? since that seems to be used in a test? (honestly, how are these objects constructed otherwise? I guess with aggregate init?)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142214