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

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 17 13:11:45 PDT 2015


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.


More information about the llvm-commits mailing list