[PATCH] Add standard insert overloads to StringMap

Aaron Ballman aaron at aaronballman.com
Mon Jun 16 01:07:07 PDT 2014


On Mon, Jun 16, 2014 at 4:01 AM, Alp Toker <alp at nuanti.com> 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.

I didn't find David's email to be defensive; it was a valid suggestion
(we've had problems with mail hitting spam filters in the past). I
doubt very much that this was intended to be "snuck" into anything.
Likely just accidental pain from Phab.

~Aaron



More information about the llvm-commits mailing list