Re: [boost] [egg] some comments

From: Giovanni Piero Deretta (gpderetta_at_[hidden])
Date: 2008-04-09 11:55:42

On Wed, Apr 9, 2008 at 11:16 AM, shunsuke <pstade.mb_at_[hidden]> wrote:
>
> Giovanni Piero Deretta wrote:
>  > I'm attaching some comments after a not-so-brief reading of the egg
>  > documentations.
>  >
>
>  I'm attaching the answers to your egg.txt.
>

And here are my replies to some of your answers.

>
> > === Introduction
>  >
>  > - "Egg what for?" (shouldn't it be "What is Egg for?"): why do I need to
>  >   use the ugly BOOST_EGG_PIPABLE syntax? why doesn't the following
>  >   syntax work?:
>  >
>  >   result_of_pipable<T_plus>::type const my_plus = {};
>
>  (As you pointed,) Those macros are unneeded in the case of stateless functions.
>  So, I propose a new `static_` form:
>
>      The adapting form in review version:
>          result_of_curry2<F>::type const c = BOOST_EGG_CURRY2({}); // stateless
>      is changed to a new `static_` form *without macros*:
>          static_< result_of<T_curry2(F)> >::type const c = {{}};
>
>  How do you think about this new form?
>

Much better, but I do not particulary like the static_ name
(expecially because of the trailing '_'). Statically_initialized is
too long, so if you can't come up with anything better and shorter, I
can live with static.

Btw, can you explain me (again, please :P) why

  result_of<T_curry2(F)> >::type const c = {{{}};

isn't enough? I guess I should see static_ implementation.

What about collapsing static_ and result_of in a single class, and
allowing arbitrary composition:

  apply<compose, apply<compose, my_fun1, my_fun2>, my_fun3>::type c = {{}};

The apply name is already taken by MPL, but maybe something of the sort.

>  > - Notation:
>  >
>  >   Please get rid of this notation. The double underscore syntax hurts
>  >   my eyes :). Also it is very hard to have to always refer to this
>  >   section when reading the documentation. The last four entries are
>  >   obvious enough, and so is decltype. For the rest you should find a
>  >   different, self explicative notation.
>
>  Because the document of FunctionAdaptor section is going to
>  be simplified, I think I can remove most of them.

Great!

>
>  >   3. "Unintentional ADL". It is claimed (in the solution) that egg
>  >      solves the problem of unintended adl by making make_filtered an
>  >      object.
>
>  > Be aware that gcc performs ADL even on function objects.
>
>  Which version of gcc?

AFAIK all of them:

namespace a {

    struct foo_t {};

    struct bar_t {
        void operator()(foo_t){};
    } bar;
};

int main() {
    a::foo_t foo;

    bar(foo);
}

This compiles at least with gcc-4.1.3 and gcc-3.3.6 (it doesn't with
comeau online)
The biggest problem is of course is that gcc fails to compile this:

namespace a {

    struct foo_t {};

    struct bar {
    };
};

void bar(...);

int main() {
    a::foo_t foo;

    bar(foo);
}

I.e. ADL finds even non callable entities! Comeau online compiles it cleanly.

>
>  > - "Binding functions"
>  >
>  >   "boost::result_of isn't used for now". Why?
>
>  Because of a bug described in the `egg::result_of_` section.

Ah, now understand why I'm missing some documentation details! the
documentation doesn't always clearly show which words are links. for
example in my browser result_of_ is not visually distinguished from
the rest of the paragraph. I do not know if it is a problem of my
browser of you need to fix your CSS.

>  >   The description of lazy supposes that the reader is familiar with
>  >   boost lambda which is a non trivial library. Of course egg is not
>  >   exactly a beginner library, so it might be expected.
>
>  >   The visual distinction between make_filtered and make_Filtered might
>  >   not be caught immediately. May be change the name?
>  >    Also, I do not like this convention of having the lazy variant
>  >   having an uppercase letter in the middle. Is this just a
>  >   random example or is meant to be a useful convention? (BTW, I think
>  >   that egg documentation should have a good section about function
>  >   object naming conventions).
>
>  I don't find a convertion for lazy and unlazy functions yet.

I do not either, other than puting them in a 'lazy' namespace. This is
one of the reasons I like optionally lazy function objects  :).

>
>  >   Also I do not like the T_* convention to name function types. I
>  >   liked best the old op_* convention.
>
>  LexicallyTypedObject concept is not restricted to FunctionObjects.
>  "T_" is short of "typeof".
>  This concept is a workaround for "missig decltype" in C++03.
>

What about using the prefix 'typeof_' or a postfix '_type'? I really
dislike that uppercase T.
Maybe even a postfix _t would be more than enough.

>  > - Getting initializers
>  >
>  >   "As mentioned before, the static initialization is important". Why?
>  >   You still do not explain it.
>
>  Click "before".

Right, missed the link. Probably this part should go in an advanced
section or at least an had-hoc section on static initialization.
Certainlin not in the quick start.

>
>  > - Object generators
>  >
>  >   It is completely un-obvious what are you trying to explain
>  >   here. Even clicking on the link about object generators doesn't
>  >   really enlighten me much: if the name 'always' parallels the mpl
>  >   counterpart it should be a function that always returns the same
>  >   value. But why you call it 'always_return' and not just 'always'?
>  >
>  >   Ah, I see, you later name the actual object 'always'. But why the
>  >   adapted class (or would be better to call it 'facaded'?) was
>  >   postifixed _result? If it were just a metafunction to compute the
>  >   result type I would understand, but the 'call' method makes it
>  >   something more than an mpl metafunction.
>
>  always_result means a return type of always.

sure, but it also contains the body of 'always' (i.e. the call
member), so it is a misleading name.

>  >   "bind expressions my be less readable. nestN offers a better syntax"
>  >
>  >   egg::nest2(plus)(5, egg::nest2(_1_(_1))(_1_(_2), _0_(_1)))
>  >             (i6)(minus, i7);
>  >
>  >   Is that a joke?!?!? :) I use both boost.bind and lambda.bind
>  >   extensively and while I might not like the syntax, at least is clear
>  >   what it does.  the _N_(_M) syntas not only is *extremely* ugly, but
>  >   I have absolutely no idea of what are you trying to do! Please
>  >   explain.
>  >
>  >   The haskel-like lambda syntax doesn't help. I can barely read simple
>  >   haskel expressions, certainly not nested lambdas. I expect most C++
>  >   programmers (even very sophisticate ones) wouldn't either.
>
>  nestN returns a function which returns a function which returns a function...
>  Probably I should change the notation into C++0x lambda expression?

That would probably help a little. Much more prose (like a step by
step analisys of what that expression means) would be much better.

>
>  > I find the prefix T_ extremely ugly. I preferred what I
>  > think egg used previously, i.e. the op_ notation.
>
>  This convention is not restricted to FunctionObjects.
>  `T_` is short of "typeof".
>  This convention is a workaound for "missing decltype" in C++03.

I know, I just find the syntax ugly.

>
>  > Even better would be
>  > to segregate function object types to their own namespace, a-la fusion
>  > or proto. (i.e. the functional namespace).
>
>  See this:
>
>    namespace poost {
>        namespace op {
>            struct foo {};
>        }
>
>        op::foo const foo = {};
>
>        namespace nested {
>            namespace op {
>                struct bar {};
>            }
>            op::bar const bar = {};
>
>            void test()
>            {
>                op::foo my_foo = foo; // doesn't compile
>            }
>        }
>     }
>
>  Thus, I've rejected segregated-namespace-way.

uh?
...
void  test() {
   poost::op::foo my_foo = foo; // it compiles!
}

Seems a very simple fix (and arguably the right thing in the first place!)

>
>  > - Pipable and Ambi: These concepts are intermixed and too fuzzy
>  > IMHO. All the details of Ambi and Pibable, should be collapsed on a
>  > simpler Pibaple concept
>
>  I made some efforts to define these concept.
>  I believe this way is the simplest (for now).
>
>  > (and BTW, Ambi is not a self descriptive name):
>
>  I will be happy if a better name for ambi is found in this review.
>
>  > " A pipable function object 'f' is an unary polymorphic function
>  > object for which the expression 'x | f' is equivalent to f(x) ".
>  >
>  > Making a function object pipable should just be a matter of deriving
>  > it from an (empty) egg::pipable, which would manifest the intention of
>  > the object to participate in pipelines.
>
>  I'm not sure it has an advantage over higer-order functions.

Less intellectual overhead :). I need to know less things to
appreciate what 'pipable' means. Concept should be as simple as
possible IMHO. (you can of course add "... but not simpler" :) )

>
>  >  Oven should provide a free
>  > function operator| (that will be found via adl, and sfinae'd on
>  > is_base_and_derived<pibable, Rhs>).
>
>  Sorry, I couldn't understand this sentence.
>  Why is Oven stated here?
>

I meant Egg of course. What I wanted to say is:

Egg provides a templated operator| in egg namespace. There is a single
implementation for this operator and is:

namespace pibable_ {

struct pipable {}; // ADL hook

template<typename Lhs, typename Rhs>
typename
boost::lazy_enable_if<bost::is_base_and_derived<pipable, Rhs>,
boost::result_of<Rhs(Lhs&)> >::type
operator|(Lhs& lhs, Rhs rhs) {
   return rhs(lhs);
}

template<typename Lhs, typename Rhs>
typename
boost::lazy_enable_if<bost::is_base_and_derived<pipable, Rhs>,
boost::result_of<Rhs(const Lhs&)> >::type
operator|(const Lhs& lhs, Rhs rhs) {
   return rhs(lhs);
}
}

Any unary function object that wants to model the pipable concept must
derive from pipable_::pipable. This will trigger ADL and will find the
operator | when necessary.  (In c++0x of course you would put
operator| in the global namespace and make it a template function
constrained on the (non auto) concept Pipable)

This of course only work with unary callable entities. to make a
non-unary entity unary, apply the appropriate curry/bind grease :).
This will also get rid of the need for Ambi.

As an extension (I'm not proposing it, it is just for the sake of
discussion), '|' could be a synonym for compose (IIRC it is spelled
'$' in haskell):

 if lhs and rhs are callable entites (in practice you detect pipability):
   a | b
 is the same as:
   compose(b, a);

 else if only b is a callable entity:
   a | b
 is the same as
   compose(b, always(a));

 else the expression is illegal.

[IIRC in haskel values are in practice treated as nullary functions,
so the use of 'always' here would mimic the functional comunity usage]

This means that you can create pipelines of transforms:

   map(my_range, my_first_view|my_second_view)

Would be the same as:

   map(my_range, compose(my_second_view, my_first_view));

or

   map(my_range, my_first_view) | protect(lazy(_1, my_second_view))

[where map returns a pair of transform_iterators]. As an alternative
for the ugly protect + lazy, read further.

>  > Adapting function objects of arbitrary arity to make them pibable is
>  > really orthogonal. Lambda would works just fine, but egg could provide
>  > (if already doesn't)adaptors to add curriability functionality to a
>  > generic function object:
>  >
>  >    struct op_foo : curriable<op_foo> {
>  >       typedef whatever result_type;
>  >       template<class A1, class A2, class A3>
>  >       result_type operator()(A1, A2, A3);
>  >    } foo;
>  >
>  >    x | foo(_, 10, "bar");
>  >
>  > Note that you need to explicitly mark the missing argument. And in
>  > principle you can have more than one missing argument:
>  >
>  >   f(_, 10, _)("baz", "bar");
>
>  Sorry, I couldn't understand this proposal.
>  What does `x | foo(_, 10, "bar")` mean?

hum, let me see, in egg syntax it should be:

  compose(lazy(foo)(_1, 10, "bar), always(x))

but see below:

>  Also, f(_, 10, _)("baz", "bar"); seems supported by egg::lazy.

yes, the only difference is that the result is not a lambda expression
(as if there was an implicit protect):
see the difference between:

  lazy(foo)(lazy(bar)(_1, _2)); // ll::bind(foo, ll::bind(bar, _1))

and:

  lazy(foo)(protect(lazy(bar)(_1, _2))); // ll::bind(foo,
protect(ll::bind(bar, _1)))

with my syntax (actually this is lifted directly from the generalized
currying in FC++), you could spell the latter:

  foo(bar(_,_)); // s

this is important if egg were (as one would expect) to register its
operator| with boost::lambda:

 map(my_range, _1 | lazy(is_less)(_1, 10)); // does not work!

 map(my_range, _1 | protect(lazy(is_less)(_1, 10))) ; //ok

 map(my_range, _1 | is_less(_, 10)); // also ok

A nice name for the generalized curry operation is of course curry:

  auto is_less = curry(is_less_non_curriable);

 assert(  is_less(_, 10)(7) == true );

This is trivially implementable with lazy + protect, but an ah-hoc
implementation might be simpler and easier on the compiler (no need
for full blown lambda support). Also, i spell the missing parameters
'_' because that's what FC++ used, 'deferred' might also be a good
name.

What do you think?

>
>  > - Major Function Object: this name is not descriptive.  Make it Lambda
>  > Compatible Polymorphic Function Object.
>
>  A specific name should not be contained in this concept name
>  so that MajorFunctionObject can support yet another lambda library.ù

Hum, the support hook shuld only be boost::result_of and its protocol.

>
>  > Or better, modify boost.lambda
>  > so that any Polymorphic function object work with it (shouldn't be
>  > hard), so that we do not need an ad-hoc concept.
>
>  It seems hard.

I do not think so, in fact I think that there are patches around. On
the other hand, I'm not the one doing it, so I shouldn't complain.

>
>  > - Static Function Object: again, you are mixing a concept, with
>  > pertains types, with restriction on object usages or initialization. I
>  > do not think that you can encode the fact that 'f' is statically
>  > initialized in a concept. Also, this 'concept' badly need a rationale!
>
>  I'm going to modify this concept.
>  The "stateless" requirement will be added,
>  and the static-initialization guarantee will be removed.
>  Do you think it is right?

I do not know if it is right, but I have only needed static
initialization for stateless function objects. Others might have
different experiences (and they should better speak now ;) ).

>  > - Writing you strategies: why should I want to write my own strategy?
>  >   What can I do exactly?
>
>  See the Eric's request below.
>
>  >   In valid expression you introduce apply_little, but you do not
>  >   document it anywhere it seems (its use seems obvious, though).
>
>  It isn't placed in Valid expressions table?

yes, my fault, but those tables are almost unreadable to me, sorry :(

>
>  > Where
>  >   is the custom forward strategy in the valid expression box? How
>  >   should be interpreted.
>
>  Sorry, I couldn't understand this question.

Me neither :). I have no idea of what I was asking.

>  > === Function builders
>  >
>  > - function: finally we find the function class! I object in another
>  >   class in boost called function. What about function_adaptor?
>
>  This is not an adaptor.

Hum, IMHO,yes. It is just adapt a class from the internal Minor
function object convention to the external Major function object.

>  IIRC, Boost.Phoenix also uses "function".

Yes, I saw it too. But Phoenix is not a first class boost library yet
:). when it will be reviewed, I'll make the same comment.

>
>  > BTW, do you want to encourage stateful
>  >   function objects? Since I learned 'bind' I have never again written
>  >   a stateful function object.
>
>  I rarely use Boost.Bind and Boost.Lambda in production code.

Why not? I do all the time.

>
>  > It would be great if egg had a way to
>  >   express the result type of bind (in fact I think that lazy does
>  >   exactly that).
>
>  If a Boost.Lambda expression is simple, it is feasible.
>  The full support would be challenging work.

Agree. It would enought it just supported function composition (i.e.
no overloaded operators).

>
>  >   "it can be considered a port of callable". I do not know what
>  >   'callable' is. Please provide a link/reference.
>
>  Click 'callable'.

Ah, I see. Again, the word here doesn't show up as a link untill i
mouse over it.

>  > - implicit. "Implicit adds implicit conversion support to a cast from
>  >   function". What does that means? What are the use cases?
>
>  It automatically passes initialized types to cast functions.

could you provide an example?
>
>  >     Arguments except for the last one are captured by-copy. When you
>  >     need to capture by-reference, pass arguments using boost::ref."
>  >     Why the last one is an exception (I guess your use case, and mine too,
>  >     is to pass ranges and containers as last arguments)
>
>  No, it is not range problem.
>  Currying is "binding", per se.
>  It must copy arguments to make an intermidate FunctionObjects.
>  The last argument is not needed to be copied fortunately.

I figured it out later :).

>
>  >     In the example, mabye 'my_minus_base' would be a better name for
>  >     the struct?
>
>  I prefer prefix, but I'm sure I don't know english well. :-)
>

Oh, well, me neither :P

>  > - Uncurry. Uh? What is this useful for? BTW, a more useful facility
>  >   would be the ability to traverse all objects stored in a curry
>  >   closure (is closure the right term?), like the undocumented
>  >   boost.bind apply_visitor.
>
>  I couldn't find boost.bind apply_visitor.

The actual name is visit_each (which I think is the protocol used by
apply_visitor). There is some reference in bind/storage.hpp. The
actual api is not documented, but I guess that the intent is that
boost::visit_each(vistitor, bind-expression); should iterate on all
arguments closed in the bind expression. Searching the boost mailing
list reveals that bind_visitor has been broken for a while. I do not
know if it is still true (I have never used it - yet).

>  > - lazy. Useful, 'nuf said. I would also like to have optionally lazy
>  >   function objects, but it seems that I'm the only one who likes them
>  >   :).
>
>  In fact, I know another one who likes them.

:)

>  > - mono. I have never needed this, but I understand that it might be
>  >   useful sometimes.
>
>  IIRC, Dan Marsden's Boost.Traversal makes use of one.

I was missing the fact that it allows one to inspect the argument types.

>
>  > - nestN. This is still very very obscure to me. The more I read the
>  >   description the more confused I become. I understand that the
>  >   problems are nested lambda expressions, but I do not understand how
>  >   your soultion work, what the syntax is and why it must be so
>  >   complicated
>
>  A comparison between "bind" and "nestN" has been requested,
>  so I will add some explanations of details.

Thanks.

>
>  > (did you see how phoenix handles the problem via local
>  >   variables to lambda?)
>
>  Because Egg is not "yet another lambda library",
>  I'm not sure it should provide some cool syntax.

well, at least readable :)

>  If reviewers regard it as an inadequate component,
>  I will simply remove nestN from Egg.
>  BTW, how can we write nested lambda using phoenix?

  lambda[ for_each(_1, lambda(_a = _1)[ front(_a) += _1] )(range)

[at least It should work, given appropriate definitions of for_each and front]

>  >
>  > - bll_N. Is this just an alias for lambda placeholders?
>  >   why i can't just use _N and boost::lambda::placeholderN_type?
>
>  bll_N is a model of LexicallyTypedObject
>  so that you don't need to remember the name "placeholderN_type".

so now I have to remember two types instead of one :)

>  > - construct_braced{1,2}. Eh? what is this for?
>  >
>  > - construct_variadic. Same as above.
>
>  egg::generator needs this to initialize pod types.

Is it a general purpose utility of just an implementation detail?

>
>  >   Doesn't having to specify the signature partially defeats the
>  >   purpose? Why can't I just do:
>  >
>  >   BOOST_AUTO(f, unfuse(fuse(_1 + _2)))
>  >
>  >   and keep my lovely polimorphic-nes? i.e. would it be possible for
>  >   egg to (optionally) register all its type with boost.typeof?
>
>  Egg could do, but I think it is nearly impossible for Boost.Lambda.

Unless lambda registers its types of course. Anyways, It would be
enough for me if this worked:

BOOST_AUTO(foobar, compose(foo, bar));

>
>  > Of course the user would be responsible for registering its own
>  >   function objects.
>
>  I can't imagine users register their function objects.

Well, if an user wants to use typeof it needs to registers its types anyway.

Enough for now. More comments to come.

-- 
gpd