[PATCH] D74693: [IRBuilder] Delete copy constructor

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 16 12:54:24 PST 2020


nikic marked an inline comment as done.
nikic added inline comments.


================
Comment at: llvm/lib/IR/DIBuilder.cpp:900-903
+class IRBuilderForDbg : public IRBuilder<> {
+public:
+  IRBuilderForDbg(const DILocation *DL, BasicBlock *InsertBB,
+                  Instruction *InsertBefore)
----------------
Meinersbur wrote:
> Subclassing does not feel like the right approach for this (we might think about marking `IRBuilder` as final). It's just preconfiguring an IRBuilder. Standard technique would be to return a `unique_ptr<IRBuilder>` instead.
> 
> However, subclassing is more pragmatic here. Other opinions?
Agree that the subclassing here was pretty odd. I've changed this to instead split up IRBuilder construction and the setting of insert-point/debug-loc. It's one more line of code at the use-site, but this seems cleaner.


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

https://reviews.llvm.org/D74693





More information about the llvm-commits mailing list