[PATCH] Add standard insert overloads to StringMap

Agustín Bergé kaballo86 at hotmail.com
Mon Jun 16 12:15:08 PDT 2014


>>! In D4153#4, @dblaikie wrote:
> The test case looks a bit thin - it doesn't test the return value of
> insert, only tests one of the two insert functions the patch
> introduces, and doesn't test positive and negative cases of insert
> (pre-existing element and new insertion)

I will add those.

> Does this function have some/all redundancy with existing interface?
> (should the other insert-like APIs be refactored to call insert (&
> perhaps eventually removed in favor of just using insert?))

The new overloads behave exactly like `GetOrCreateValue`, except that the parameters and return types are different. I'll reimplement it in terms of `insert`.
 
> In the other code review you mention StringMap has an insert that
> returns bool? Yet you're not modifying that version here? What
> arguments does it take that make it distinct from the two functions
> you're adding?

The existing `insert` is defined as `bool insert(MapEntryTy *KeyValue)`, it takes a pointer to an already created `MapEntryTy` and acquires ownership of it. AFAIK, this is the only piece of functionality that would not be covered by a standard associative sequence interface.

http://reviews.llvm.org/D4153






More information about the llvm-commits mailing list