[PATCH] D99568: [clang][invocation] Fix copy constructor, add copy assignment to CompilerInvocation

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

I'm not sure a deep copy is entirely sound, due to odd ownership rules in "remapped buffers" (I forget the subclass that includes those). A few months ago I got most of the way to deleting those (lifting the logic up into the clang tooling that needed it)... maybe we need to push that work over the line before this is safe... or if this is in fact safe, can you explain why?

BTW, I may be responsible for the not-very-descriptive naming of `CompilerInvocationBase`... I could imagine new options coming along that get added to the wrong place in the future as well. Any thoughts on a good name to use instead?



================
Comment at: clang/include/clang/Frontend/CompilerInvocation.h:86
   CompilerInvocationBase(const CompilerInvocationBase &X);
-  CompilerInvocationBase &operator=(const CompilerInvocationBase &) = delete;
+  CompilerInvocationBase &operator=(CompilerInvocationBase X);
   ~CompilerInvocationBase();
----------------
I wonder if it would be better to signify this is a deep copy by naming the function, such as by calling it `clone()`. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99568