[PATCH] Add standard insert overloads to StringMap

David Blaikie dblaikie at gmail.com
Mon Jun 16 00:01:25 PDT 2014


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)

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?))

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?

(though don't get me wrong, this seems like a good change to make)

http://reviews.llvm.org/D4153






More information about the llvm-commits mailing list