<div dir="rtl"><div dir="ltr">Hans: you are right, thanks.</div><div dir="ltr"><br></div><div dir="ltr">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.</div><div dir="ltr"><br></div><div dir="ltr">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. </div><div dir="ltr"><br></div><div class="gmail_extra"><br><div class="gmail_quote"><div dir="ltr">2015-08-17 23:11 GMT+03:00 Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span>:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Since this discussion is still ongoing, and I expect the final patch<br>
will be more comprehensive than just fixing<br>
StructType::isLayoutIdentical, I've committed a minimal fix for 3.7 in<br>
r245233.<br>
<br>
Thanks,<br>
Hans<br>
<div><div><br>
On Mon, Aug 17, 2015 at 10:28 AM, Duncan P. N. Exon Smith via<br>
llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
><br>
>> On 2015-Aug-16, at 23:11, Yaron Keren <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>> wrote:<br>
>><br>
>> 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.<br>
>><br>
><br>
> I think I got the magic wrong. Try this:<br>
><br>
> namespace llvm {<br>
> namespace algorithm {<br>
> void foo() {...}<br>
> }<br>
> using namespace algorithm;<br>
> }<br>
><br>
> Assuming that works, I don't really like the name `algorithm`; it should<br>
> be something that we wouldn't use except to break argument-dependent<br>
> lookup. Maybe `algorithm_noadl` or something?<br>
><br>
> (If it doesn't work, does anyone actually know how to fix this?)<br>
><br>
>><br>
>><br>
>> 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)<br>
>> 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]<br>
>> 3> with<br>
>> 3> [<br>
>> 3> I1=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init *>>><br>
>> 3> , I2=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init *>>><br>
>> 3> ]<br>
>> 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)'<br>
>> 3> with<br>
>> 3> [<br>
>> 3> _InIt1=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init *>>><br>
>> 3> , _InIt2=std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<llvm::Init *>>><br>
>> 3> ]<br>
>> 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 *>>>)'<br>
>> 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<br>
>> 3> with<br>
>> 3> [<br>
>> 3> _Ty=llvm::Init *<br>
>> 3> ]<br>
>> 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<br>
>> 3> with<br>
>> 3> [<br>
>> 3> _Ty=llvm::Init *<br>
>> 3> ]<br>
>><br>
>><br>
>><br>
>><br>
>><br>
>><br>
>> 2015-08-17 3:48 GMT+03:00 Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>>:<br>
>><br>
>> > On 2015-Aug-15, at 00:13, Justin Bogner <<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>> wrote:<br>
>> ><br>
>> > "Duncan P. N. Exon Smith" <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>> writes:<br>
>> >>> On 2015-Aug-12, at 12:22, Yaron Keren <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>> wrote:<br>
>> >>><br>
>> >><br>
>> >>> 3) Should it replace the other uses of std:;equal in LLVM and clang,<br>
>> >>> although a nullptr is not possible in some (most?) of the code?<br>
>> >><br>
>> >> I'd say that's up to the MSVC folks (like you). I wouldn't bother<br>
>> >> for iterators that aren't nullable, but for the others, it seems<br>
>> >> like it would be nice to have testcases to motivate the change?<br>
>> >> Possibly better in follow-up commits.<br>
>> ><br>
>> > If we're adding an llvm::equal, we should probably just forbid<br>
>> > std::equal with the justification that it's broken on MSVC - having two<br>
>> > ways to do the same thing that are *usually* equivalent will invariably<br>
>> > result in people using the wrong one in the case where it matters.<br>
>><br>
>> Fair enough.<br>
</div></div></blockquote></div><br></div></div>