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

[Justin Bogner via llvm-commits]

Aaron Ballman via llvm-commits <llvm-commits at lists.llvm.org> writes:
> 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.

What kinds of tools are we talking about here? The incorrect checking in
std::equals case causes us to fail only at runtime, in debug mode, and
on MSVC, which means it's pretty likely for developers to commit code
that breaks it, and workarounds to implement llvm:: versions of standard
functions to work around broken environments have a fairly high cost in
terms of maintenance and cognitive load. Also, as evidenced by this
thread, developer time.

Are the checks we're gaining by having this macro defined worth the pain
we have to endure to get them?

> 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
>
>>
>>
>> 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 :).
>>
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits