Re: [boost] [yap] Review part 2: implementation

Subject: Re: [boost] [yap] Review part 2: implementation
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2018-02-15 16:11:49

AMDG

On 02/14/2018 10:29 PM, Zach Laine via Boost wrote:
> On Wed, Feb 14, 2018 at 4:45 PM, Steven Watanabe via Boost <
> boost_at_[hidden]> wrote:
> 
>> On 02/14/2018 03:11 PM, Zach Laine via Boost wrote:
>>> On Tue, Feb 13, 2018 at 4:51 PM, Steven Watanabe via Boost <
>>> boost_at_[hidden]> wrote:
>>>
>>>> algorithm.hpp:
>>>>
>>>> 221: template <long long I, typename Expr>
>>>>      decltype(auto) get (Expr && expr, hana::llong<I> i);
>>>>    Does this need to be restricted to hana::llong, rather
>>>>    than, say, std::integral_constant?
>>>>
>>>
>>> It's the lack of nice syntax for integral_constant literals that made me
>>> choose this.  I write get(expr, N_c) a lot, and I expect users to as
>> well.
>>>
>>
>>   Supporting an IntegralConstant doesn't mean that
>> you can't pass 0_c.
> 
> 
> Is the change from std::integral_constant to IntegralConstant significant?
> That is, are you asking that I accept models of some concept, or just
> std::integral_constant?
> 

  I meant hana::IntegralConstant in the first place.
std::integral_constant was just an example of another
IntegralConstant, which hana::at can accept.

> 
>>> <snip>
>>>> expression_free_operators.hpp/expression_if_else.hpp:
>>>>
>>>> - Why is this not part of expression.hpp?
>>>>
>>>
>>> For the same reason that expression_free_operators.hpp is not.  Those
>> three
>>> headers are separate pieces that you may want some or all of, so I kept
>>> them separate.
>>>
>>
>>   expression_if_else makes a certain amount of sense as
>> being separate.  Making expression_free_operators separate
>> comes with a high risk of ODR problems in any code that
>> has different behavior depending on whether a given operator
>> is defined.  It's extra strange because expression_free_operators.hpp
>> only has half the operator overloads, with the other half
>> being defined as members.
> 
> 
> I've read this a few times and don't get it.  Without including
> expression_free_operators.hpp, where might the other ODR-colliding
> operators come from?  Do you mean if the user defines her own operators for
> expression<>, and then includes expression_free_operators.hpp in another
> TU, or something else?
> 

  I was talking about the one definition rule as applied
to some function that uses the operators.
As an example, consider something that you yourself
have written:

boost/yap/print.hpp:

template <typename T, typename = void_t<>>
struct printer
{
    std::ostream & operator() (std::ostream & os, T const &)
    { return os << "<<unprintable-value>>"; }
};

template <typename T>
struct printer<
    T,
    void_t<decltype(
        std::declval<std::ostream &>() << std::declval<T const &>()
    )> >
{
    std::ostream & operator() (std::ostream & os, T const & x)
    { return os << x; }
};

Now consider a class that defines a stream operator separately
from the class:

// x.hpp
class X {};

// x_print.hpp
std::ostream& operator<<(std::ostream&, const X&);

Next, I have two separate translation units:

//a.cpp
#include <x.hpp>
#include <x_print.hpp>
yap::detail::print_value(cout, X{});

// b.cpp
#include <x.hpp>
yap::detail::print_value(cout, X{});

printer chooses a different specialization in a.cpp
than it does in b.cpp, which results in undefined
behavior.

  As a result of this problem, I believe that any
free functions (including operators) that are intended
to be found by ADL *must* be declared alongside the
class definition.

> [snip]
> 
> Let me bring operator||/operator&& short circuiting back up.  After
> implementing it, I don't think it's appropriate.  Consider the current
> non-short-circuiting implementation on the boost_review branch:
> 
> https://github.com/tzlaine/yap/blob/boost_review/include/boost/yap/detail/default_eval.hpp#L136
> 
> #define BOOST_YAP_BINARY_OPERATOR_CASE(op, op_name)                     \
>         template <>                                                     \
>         struct default_eval_expr_impl<expr_kind:: op_name>              \
>         {                                                               \
>             template <typename Expr, typename ...T>                     \
>             decltype(auto) operator() (Expr && expr, T && ... args)     \
>             {                                                           \
>                 using namespace hana::literals;                         \
>                 return                                                  \
>                     default_eval_expr(static_cast<Expr
> &&>(expr).elements[0_c], static_cast<T &&>(args)...) op \
>                     default_eval_expr(static_cast<Expr
> &&>(expr).elements[1_c], static_cast<T &&>(args)...);
>                                 \
>             }                                                           \
>         };
> 

  This implementation is fine.  The problem was the original
implementation that called eval_ ## op_name, bypassing built-in
short-circuiting.

> If the "&&" in "default_eval_expr(...) && default_eval_expr(...)" resolves
> to a built-in operator&&(), short-circuiting will happen as always, right?
> But if the "&&" resolves to a user-defined operator&&(), let's say this one:
> 
> struct my_type {
>     // ...
>     std::shared_ptr<int> operator&&(my_type const & rhs) {/*...*/}
> };
> 
> then I really don't want to artificially impose the short-circuiting.  The
> whole point of evaluate() is that it evaluates to whatever your Yap
> expression would have if it were a plain ol' non-Yap expression.
> 

  I agree with you.  You don't need to add your own
short-circuiting.  You just need to not get in the
way of the language's short-circuiting.

In Christ,
Steven Watanabe