[llvm] r243996 - Avoid passing nullptr to std::equal.

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 15 00:13:37 PDT 2015


"Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
>> On 2015-Aug-12, at 12:22, Yaron Keren <yaron.keren at gmail.com> wrote:
>> 
>> Better do it right on the first time. Some questions about llvm::equal :
>> 
>> 1) Would STLExtras be good place for llvm::equal ?
>
> Yes, I think that's where I've seen stuff like this.
>
>> 2) Should its contents be #ifdefed to _MSC_VER, something like
>> 
>>  template <class I1, class I2>
>>     bool equal(I1 First1, I1 Last1, I2 First2) {
>> #ifdef _MSC_VER
>>       for (; First1 != Last1; ++First1, ++First2)
>>         if (!(*First1 == *First2))
>>           return false;
>>       return true;
>> #else
>>      return std::equal(First1, Last1, First2);
>> #endif
>>     }
>
> This SGTM (although I don't really know whether that's the right
> macro to check).  This probably doesn't cover the case of using
> a different STL when compiling with MSVC, but maybe that's not
> important?
>
> I'd probably add a couple of simple unit tests, too, and update
> the CodingStandards to use `llvm::equal`.
>
>> 3) Should it replace the other uses of std:;equal in LLVM and clang,
>> although a nullptr is not possible in some (most?) of the code?
>
> I'd say that's up to the MSVC folks (like you).  I wouldn't bother
> for iterators that aren't nullable, but for the others, it seems
> like it would be nice to have testcases to motivate the change?
> Possibly better in follow-up commits.

If we're adding an llvm::equal, we should probably just forbid
std::equal with the justification that it's broken on MSVC - having two
ways to do the same thing that are *usually* equivalent will invariably
result in people using the wrong one in the case where it matters.


More information about the llvm-commits mailing list