[PATCH] D90083: IR: Clarify ownership of ConstantDataSequentials, NFC

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/IR/Constants.cpp:2787-2788
+  std::unique_ptr<ConstantDataSequential> *Entry = &Slot.second;
+  for (ConstantDataSequential *Node = Entry->get(); Node;
+       Entry = &Node->Next, Node = Entry->get())
     if (Node->getType() == Ty)
----------------
This becomes a bit fussy to read, what about:
```
std::unique_ptr<ConstantDataSequential> *Entry = &Slot.second;
for (; *Entry; Entry = &Node->Next)
  if ((*Entry)->getType() == Ty)
    return Entry->get();
```

Does that do the same thing? Perhaps I've missed something/broken it in some way.


================
Comment at: llvm/lib/IR/Constants.cpp:2795
+  if (isa<ArrayType>(Ty)) {
+    Entry->reset(new ConstantDataArray(Ty, Slot.first().data()));
+    return Entry->get();
----------------
I'd generally favor `*Entry = std::make_unique...` (less cognitive load - there's no suspicious raw `new` I feel compelled to double-check ("Is that reset function on a unique_ptr, or some other thing that might have more nuanced semantics on raw pointers, etc, etc")) but up to you


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

https://reviews.llvm.org/D90083