[PATCH] D35908: [YAML] Use {Specific}BumpPtrAllocators for HNodes.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 27 11:09:57 PDT 2017


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.

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)
On Thu, Jul 27, 2017 at 10:30 AM Juergen Ributzka via Phabricator <
reviews at reviews.llvm.org> wrote:

> ributzka marked an inline comment as done.
> ributzka added inline comments.
>
>
> ================
> Comment at: llvm/lib/Support/YAMLTraits.cpp:357
>      }
> -    return llvm::make_unique<ScalarHNode>(N, KeyStr);
> +    return new (HNodeAllocator.Allocate<ScalarNode>()) ScalarHNode(N,
> KeyStr);
>    } else if (BlockScalarNode *BSN = dyn_cast<BlockScalarNode>(N)) {
> ----------------
> zturner wrote:
> > This is a little confusing.  Why are you calling
> `Allocate<ScalarNode>()` but then actually constructing a `ScalarHNode`
> instead of a `ScalarNode`?
> Oops, that is a typo. Thanks for catching this.
>
>
> https://reviews.llvm.org/D35908
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170727/beefccdd/attachment.html>


More information about the llvm-commits mailing list