<div dir="ltr">The SpecificBumpPtrAllocator doesn't have this problem, because it is already tied to a specific type. Although the convenience and having consistent API across the BumpPtrAllocators would be nice. Either way, such a change should be independent of this patch and we should make a separate patch for the BumpPtrAllocator changes.</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 27, 2017 at 11:09 AM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'm always a little bothered when users of a library have to write a placement new, I feel like this kind of thing should be hidden in the library as much as possible.<br><br>Would it be possible to modify BumpPtrAllocator and SpecificBumpPtrAllocator to have an AllocateAndConstruct that wraps the logic here so we can get rid of the placement new (which would have prevented this typo in the first place)<br><div class="gmail_quote"><div dir="ltr">On Thu, Jul 27, 2017 at 10:30 AM Juergen Ributzka via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ributzka marked an inline comment as done.<br>
ributzka added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/lib/Support/YAMLTraits.<wbr>cpp:357<br>
     }<br>
-    return llvm::make_unique<ScalarHNode><wbr>(N, KeyStr);<br>
+    return new (HNodeAllocator.Allocate<<wbr>ScalarNode>()) ScalarHNode(N, KeyStr);<br>
   } else if (BlockScalarNode *BSN = dyn_cast<BlockScalarNode>(N)) {<br>
----------------<br>
zturner wrote:<br>
> This is a little confusing.  Why are you calling `Allocate<ScalarNode>()` but then actually constructing a `ScalarHNode` instead of a `ScalarNode`?<br>
Oops, that is a typo. Thanks for catching this.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D35908" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D35908</a><br>
<br>
<br>
<br>
</blockquote></div>
</blockquote></div><br></div>