[PATCH] Add standard insert overloads to StringMap

[Alp Toker]

On 16/06/2014 18:04, Agustín K-ballo Bergé wrote:
> On 16/06/2014 05:01 a.m., Alp Toker wrote:
>>
>> On 16/06/2014 10:32, David Blaikie 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.
>>>
>>> Perhaps it ended up in your spam filter?
>>
>>
>> David, could you check your facts instead of firing off defensive
>> one-liners?
>>
>> The revision is private and there is no public record of the patch at 
>> all:
>>
>> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140616/thread.html 
>>
>>
>>
>> Sneaking commits into SVN without inviting public review, and blaming
>> tools when it gets noticed is so last week.
>>
>> Alp.
>>
>
> I feel offended by some of the comments and insinuations expressed on 
> this thread. All it takes is a click in the review link to see that 
> llvm-commits was in fact added as supposed to, and it would have been 
> enough to avoid all the noise that followed (while keeping the valid 
> concerns and remarks).

Hi Agustín,

I'm sorry if you felt offended. Any criticism, justified or otherwise, 
was directed at David.

> PS: Alp, I will be expecting a thorough review from you. 

Great. That's why I'm here -- we're trying to use StringMap more 
efficiently in clang's mangling code (see r210293) and this could be a 
useful facility.

So please let's break out of the silos and coordinate this on-list :-)

Alp.



>
> Regards,

-- 
http://www.nuanti.com
the browser experts