[cfe-dev] Fwd: [libc++] possible bug?

[Ovanes Markaryan]

Sorry, don't know why this email went directly to Matthieu instead of the
list...

---------- Forwarded message ----------
From: Ovanes Markaryan <om_clang at keywallet.com>
Date: Mon, Apr 23, 2012 at 10:30 PM
Subject: Re: [cfe-dev] [libc++] possible bug?
To: Matthieu Monrocq <matthieu.monrocq at gmail.com>


Hello Matthieu,

please see my answers below.

On Mon, Apr 23, 2012 at 6:53 PM, Matthieu Monrocq <
matthieu.monrocq at gmail.com> wrote:

>
>
> Le 22 avril 2012 23:58, Ovanes Markaryan <om_clang at keywallet.com> a écrit
> :
>
>>
>>
>> On Sun, Apr 22, 2012 at 11:32 PM, Marc Glisse <marc.glisse at inria.fr>wrote:
>>
>>> On Sun, 22 Apr 2012, Ovanes Markaryan wrote:
>>>
>>>  Hello *,
>>>> just tried to compile SYMPHONY-5.4.4 with clang 3.0 + libc++ (tag 31).
>>>> While
>>>> compiling it I receive this error:
>>>>
>>>> include/c++/v1/algorithm:643:**97: error: invalid operands to binary
>>>> expression
>>>>       ('const reducedCost' and 'const reducedCost')
>>>>     _LIBCPP_INLINE_VISIBILITY bool operator()(const _T1& __x, const _T1&
>>>> __y) const {return __x < __y;}
>>>>
>>>
>>> [...]
>>>
>>>
>>>      bool operator<(const reducedCost & other)
>>>>
>>>
>>> Missing const here. It is often preferable to avoid making operators
>>> member functions when you can avoid it.
>>>
>>>      {
>>>>         return (value>other.value);
>>>>     }
>>>>
>>>
>>> --
>>> Marc Glisse
>>>
>>
>> Hello Marc,
>>
>> you are definitely right. I overlooked it, since as already wrote this is
>> not my code (just copy pasted it from the lib)... But in my tests I
>> commented out this operator< and provided the specialization of  std::less,
>> to just test it and it failed as well. I did not test with both enable
>> (operator< and less-specialization). Actually this code is somehow really
>> bad, because less is actually not less but greater ;) This code compiles
>> fine with gcc and clang + gcc's version of stdlib. As far as I remember the
>> standard requires that std::less is specialized to allow the less
>> comparison or the operator< is implicitly used through the generalized
>> less. In my tests less specialization was not considered by make_heap. This
>> is my point.
>>
>>
>> Two questions:
>
> 1. Does it work if you actually specify that `operator<` is `const` ?
>
 yes.

>
> 2. Did you were careful about putting the specialization of `less` (for
> this class) before its first use ? (it is not clear from your example)
>
Yes. Please the attached file.

>
> If you could, for example, provide a simple attachment that reproduces the
> issue it would be slightly simpler to investigate.
>

Here we go:

#include <algorithm>
#include <iterator>
#include <functional>

struct reducedCost
{
    /** To avoid computing two times the same row direction will have
strange
      values, direction is -1 or 1 if for only one of the two direction
     rc is <0 and is -2 or 2 if the two direction have one <0 rc with the
sign
     indicating which one of the two directions is the best.<br>
     Note that by theory only one reduced cost (for u_i, or v_i)
     maybe negative for each direction.
     */
    int direction;
    /** gammSign is the sign of gamma (corresponding to taking rc for u_i
or v_i)
     for the best of the two rc for this row.*/
    int gammaSign;
    /** gammaSign2 is the sign of gamma for the worst of the two rc for
this row.*/
    int gammaSign2;
    /** if both reduced costs are <0 value is the smallest of the two.*/
    double value;
    /** greatest of the two reduced costs */
    double value2;
    /** index of the row.*/
    int row;
#ifdef TEST_WITH_OPLESS_CONST
    bool operator<(const reducedCost & other)const
    {
        return (value>other.value);
    }
#endif
};

#ifdef TEST_WITH_LESS_SPEC
namespace std
{
    template<>
    struct less<reducedCost>
        : ::std::binary_function<reducedCost, reducedCost, bool>
    {
        bool operator()(const reducedCost & lhs, const reducedCost &
rhs)const
        {
            return lhs.value>rhs.value;
        }
    };
}
#endif

int main()
{
  reducedCost r[100]={};

  std::make_heap(&r[0], &r[0]+100);
  return 0;
};


I compiled it with clang 3.0 and libc++ taged with release_31. Here the
both command lines:
This will fail
$ ~/toolsets/clang-3.0/bin/clang++ -DTEST_WITH_LESS_SPEC=1 -stdlib=libc++
-I/home/ovanes/toolsets/clang-3.0/include/c++/v1
 -L/home/ovanes/toolsets/clang-3.0/lib make_heap_test.cpp
This works:
$ ~/toolsets/clang-3.0/bin/clang++ -DTEST_WITH_OPLESS_CONST=1
-stdlib=libc++ -I/home/ovanes/toolsets/clang-3.0/include/c++/v1
 -L/home/ovanes/toolsets/clang-3.0/lib make_heap_test.cpp

 Regards,
Ovanes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120423/1efd8f78/attachment.html>