[PATCH] D17920: Query the StringMap only once when creating MDString (NFC)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 22 09:17:58 PDT 2016


On Tue, Mar 22, 2016 at 9:02 AM, Mehdi AMINI via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> joker.eph added a comment.
>
> > std::unordered_map::emplace returns the classic <iterator, bool> pair.
> Perhaps emplace_second should do the same? Rather than relying on being
> able to detect the invalid default constructed state of the key?
>
>
> I don't understand what you mean: `std::pair<iterator, bool>
> emplace_second(StringRef Key, ArgsTy &&... Args) `
>

Right, to match/parallel with:

template< class... Args >
std::pair <http://en.cppreference.com/w/cpp/utility/pair><iterator,bool>
 emplace( Args&&... args )


>
>
> ================
> Comment at: include/llvm/IR/Metadata.h:595
> @@ -594,3 +594,3 @@
>    MDString() : Metadata(MDStringKind, Uniqued), Entry(nullptr) {}
> -  MDString(MDString &&) : Metadata(MDStringKind, Uniqued) {}
> +  MDString(MDString &&R) = default;
>
> ----------------
> dblaikie wrote:
> > Why do we need a move ctor at all if we're supporting emplace? Can we
> just delete it? (even if we omit it, it should be removed implicitly - same
> with the move assignment above, I think)
> StringMap requires movable types (for std::pair I think).
>

If we're supporting emplace, it's usually because the identity of the
object is important from the moment of construction - so perhaps we should
be making sure that identity is actually preserved, same as the standard
emplace.


>
>
>
> http://reviews.llvm.org/D17920
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160322/f944fdc5/attachment.html>


More information about the llvm-commits mailing list