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

[Aaron Ballman via llvm-commits]

On Mon, Aug 17, 2015 at 6:49 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>
>> On 2015-Aug-17, at 15:27, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Mon, Aug 17, 2015 at 6:18 PM, Yaron Keren <yaron.keren at gmail.com> wrote:
>>> +aaron, reid
>>>
>>> Here is a patch disabling the iterator null pointer checking in Visual C++
>>> debug mode. This makes std::equal behave as expected instead of asserting on
>>> std::equal(nullpt,r nullptr, nullptr), saving an extra check just for Visual
>>> C++ debug mode here and there.
>>>
>>> It should not cause any practical problem debugging as if a null iterator is
>>> actually dereferenced (as opposed to compared) the debugger will stop on
>>> location.
>>>
>>> What do you think?
>>
>> Mildly terrified. :-) This also disables really useful debugging tools
>> in other parts of the STL that have definitely found bugs for us in
>> the past. A lot of <algorithm>, <istream>, <ostream>, <locale>,
>> <memory>, <string>, and <utility> make use of _DEBUG_POINTER, which
>> this patch would disable.
>>
>> I would *strongly* prefer to not disable that debugging functionality,
>> especially since this runtime-level debugging has found bugs our own
>> tests have not.
>>
>> ~Aaron
>
> This _DEBUG_POINTER macro seems to violate the standard in ways that
> require weird workarounds for LLVM's uses of the STL.

In what way is it violating the standard? (I apologize if this has
been answered before, but I didn't see anyone citing the standard,
just some reasonable guesses.) If you believe there is a bug with
MSVC, has it been reported to Microsoft, and have they weighed in on
the topic?

~Aaron

> It seems better to me to make MSVC's STL standards-compliant (when
> building LLVM) than to duplicate all the STL algorithms inside
> `namespace llvm`, which IMO is the best alternative.
>
> It would be awesome if there were fewer "we don't follow the standard"
> gotchas that most of us only find, post-commit, in MSVC buildbots.
>
>>> 2015-08-17 23:45 GMT+03:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:
>>>>
>>>>
>>>>> On 2015-Aug-17, at 13:43, Yaron Keren <yaron.keren at gmail.com> wrote:
>>>>>
>>>>> I'm trying another approach. The non-null validation goes through a
>>>>> macro defined to a function which possibly may be predefined to nothing.
>>>>> This will disable all null checking on iterators in debug mode which is
>>>>> relevant to additional functions beyond std::equal. In practice it should
>>>>> not be a problem since if a null iterator is actually dereferenced (not only
>>>>> compared) then the debugger stops with invalid access anyhow.
>>>>
>>>> Well, this would be ideal :).
>>>
>>>
>