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

[Yaron Keren via llvm-commits]

So, there are three options

1) Go _DEBUG_POINTER_IMPL route for VC2013 only. This will also disable the
null checks in the other headers where they are beneficial and may miss
real bugs.

2) Replace std::equal with llvm::isEqual or stl::equal or ...

3) Solve the llvm::equal ambiguity using namespace trick while making peace
with the llvm::equal functor.



2015-08-18 16:13 GMT+03:00 Aaron Ballman <aaron at aaronballman.com>:

> On Mon, Aug 17, 2015 at 8:28 PM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
> >
> >> On 2015-Aug-17, at 16:38, Aaron Ballman <aaron at aaronballman.com> wrote:
> >>
> >> On Mon, Aug 17, 2015 at 7:23 PM, Yaron Keren <yaron.keren at gmail.com>
> wrote:
> >>> An  issue that was patched several times in LLVM is std::equal(nullptr,
> >>> nullptr, nullptr) where VC asserts on the third nullptr. Earlier in the
> >>> thread Marshal wrote this should return true and not assert.
> >>
> >> Yes, I've fixed some myself, IIRC. Marshall's logic makes sense to me,
> >> and from my reading of the standard, it also seems to fit.
> >
> > 25.2.11 is pretty clear:
> >
> >> Returns: true if for every iterator i in the range [first1,last1) the
> following corresponding condi- tions hold: *i == *(first2 + (i - first1)),
> pred(*i, *(first2 + (i - first1))) != false. Otherwise, returns false.
> >
> >
> > Unless you're suggesting that `[nullptr, nullptr)` is an invalid
> > iterator range, or that `nullptr` is an invalid iterator?  But
> > `[nullptr, nullptr)` supports all the necessary operations for an
> > empty iterator range, so how could that be?  (Maybe I'm guessing
> > wrong and this is a straw man... in which case, how could this be
> > a conforming extension?)
>
> I took longer to read more of the standard (specifically, the parts
> about library requirements in [res.on.arguments] and [iterators]), and
> I don't think this is a conforming extension. Then I fired up MSVC
> 2015 and did some experiments, and realized the behavior is different
> in 2015, so I'm not even certain the behavior was intentional in the
> first place.
>
> In MSVC 2013, equal would check the range and then check the last
> pointer for null, and this check would trigger the assertion.
> In MSVC 2015, equal would check the range, and then check the last
> pointer for null only if the range is not empty, and so no assertion
> is triggered.
>
> (There is still a degenerate case bug where std::equal(nullptr,
> nullptr, nullptr) refuses to compile that I've reported.)
>
> >> However,
> >> I'm not convinced this isn't a valid extension that prevents UB from
> >> calling std::memcmp() with a null pointer value.
> >
> > `std::equal` isn't defined in terms of `std::memcmp()`, it's defined
> > as above.  How could it be valid to return anything but `true`?  The
> > wording is clear.
> >
> > But if MSVC is calling `std::memcmp()` directly then we probably
> > don't have a choice but to reimplement `std::equal()` ourselves,
> > correctly :(.
>
> I think that's true in MSVC 2013, unfortunately. However, I also think
> that speaks to a hackish solution limited to just MSVC 2013 being
> somewhat more acceptable because we will be getting rid of 2013
> sometime in the future (next few years).
>
> ~Aaron
>
> >
> >> I think LWG would
> >> need to weigh in on the matter, but I don't know of any issues that
> >> are open on this topic that we could point to for a definitive answer.
> >> Perhaps Marshall is aware of something, however.
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150820/7e846af2/attachment.html>