[PATCH] Add standard insert overloads to StringMap

[Aaron Ballman]

On Mon, Jun 16, 2014 at 3:32 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Jun 16, 2014 at 12:28 AM, Alp Toker <alp at nuanti.com> wrote:
>>
>> On 16/06/2014 10:01, David Blaikie 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)
>>>
>>> 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)
>>
>>
>> I'm sorry David, that's not your decision to make. This is taking the
>> private review idea way too far.
>>
>> The patch should be posted to llvm-commits for public review.
>
> I'm not sure what you're referring to, but I guess it's a Phab
> bug/mail delivery issue, because I replied to the on-list mail with an
> on-list reply. This is a code review occurring on llvm-commits.

Your initial comments on the patch was the first thing that came into
my inbox on the topic, not the patch itself. It's definitely not in my
spam filter.

~Aaron