[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