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

[Yaron Keren via llvm-commits]

Hans: you are right, thanks.

Duncan. this ADT-avoiding trick indeed works in principle but breaks in our
code due to the llvm::equal struct functor (in the same file). If the equal
functor is commented out then deref<llvm::equal>() breaks elsewhere. Maybe
it's possible to make it all work together.

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.


2015-08-17 23:11 GMT+03:00 Hans Wennborg <hans at chromium.org>:

> Since this discussion is still ongoing, and I expect the final patch
> will be more comprehensive than just fixing
> StructType::isLayoutIdentical, I've committed a minimal fix for 3.7 in
> r245233.
>
> Thanks,
> Hans
>
> On Mon, Aug 17, 2015 at 10:28 AM, Duncan P. N. Exon Smith via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >
> >> On 2015-Aug-16, at 23:11, Yaron Keren <yaron.keren at gmail.com> wrote:
> >>
> >> The namespace trick does not help since equal is visible to vector just
> the same it is to us. Below is the full error message. It would work
> without pulling equal out the namespace as llvm::algorithm::equal or
> llvm::alg::equal.
> >>
> >
> > I think I got the magic wrong.  Try this:
> >
> >     namespace llvm {
> >     namespace algorithm {
> >     void foo() {...}
> >     }
> >     using namespace algorithm;
> >     }
> >
> > Assuming that works, I don't really like the name `algorithm`; it should
> > be something that we wouldn't use except to break argument-dependent
> > lookup.  Maybe `algorithm_noadl` or something?
> >
> > (If it doesn't work, does anyone actually know how to fix this?)
> >
> >>
> >>
> >> 3>C:\Program Files (x86)\Microsoft Visual Studio
> 12.0\VC\include\vector(1800): error C2668: 'llvm::algorithm::equal' :
> ambiguous call to overloaded function
> (C:\llvm-clean\lib\TableGen\Record.cpp)
> >> 3>          C:\llvm-clean\include\llvm/ADT/STLExtras.h(377): could be
> 'bool
> llvm::algorithm::equal<std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>,std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>>(I1,I1,I2)' [found using argument-dependent lookup]
> >> 3>          with
> >> 3>          [
> >> 3>
> I1=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>
> >> 3>  ,
> I2=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>
> >> 3>          ]
> >> 3>          C:\Program Files (x86)\Microsoft Visual Studio
> 12.0\VC\include\xutility(2792): or       'bool
> std::equal<std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>,std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>>(_InIt1,_InIt1,_InIt2)'
> >> 3>          with
> >> 3>          [
> >> 3>
> _InIt1=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>
> >> 3>  ,
> _InIt2=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>
> >> 3>          ]
> >> 3>          while trying to match the argument list
> '(std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>,
> std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>,
> std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init
> *>>>)'
> >> 3>          C:\Program Files (x86)\Microsoft Visual Studio
> 12.0\VC\include\vector(1808) : see reference to function template
> instantiation 'bool std::operator ==<llvm::Init*,std::allocator<_Ty>>(const
> std::vector<_Ty,std::allocator<_Ty>> &,const
> std::vector<_Ty,std::allocator<_Ty>> &)' being compiled
> >> 3>          with
> >> 3>          [
> >> 3>              _Ty=llvm::Init *
> >> 3>          ]
> >> 3>          C:\llvm-clean\lib\TableGen\Record.cpp(1511) : see reference
> to function template instantiation 'bool std::operator
> !=<llvm::Init*,std::allocator<_Ty>>(const
> std::vector<_Ty,std::allocator<_Ty>> &,const
> std::vector<_Ty,std::allocator<_Ty>> &)' being compiled
> >> 3>          with
> >> 3>          [
> >> 3>              _Ty=llvm::Init *
> >> 3>          ]
> >>
> >>
> >>
> >>
> >>
> >>
> >> 2015-08-17 3:48 GMT+03:00 Duncan P. N. Exon Smith <dexonsmith at apple.com
> >:
> >>
> >> > On 2015-Aug-15, at 00:13, Justin Bogner <mail at justinbogner.com>
> wrote:
> >> >
> >> > "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:
> >> >>>
> >> >>
> >> >>> 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.
> >>
> >> Fair enough.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150817/8b2c2c75/attachment.html>