AMDG *I vote to accept proto* Documentation General: Do you think that you could use tag_of<> or ::proto_tag consistently? users_guide\expression_construction.html The actual type of _1 looks like this: expr< tag::terminal, args0< placeholder1 >, 0 > Should this be args1< placeholder1 >? Ok. No. I'm not really happy with this, but I don't see a better way. I guess the best way to think of it is that the number represents the number of proto expressions children rather than the number of arguments. users_guide\expression_construction\operator_overloads.html Note The expr<> struct lives in the boost::proto namespace, as do all of Proto's operator overloads. The overloads are found via ADL (Argument-Dependent Lookup). That is why expressions must be "tainted" with Proto-ness for Proto to be able to build trees out of expressions. Is ADL the only reason? I should hope that the operators are overloaded to take expr<>. users_guide\expression_construction\left_right_arg.html flatten() returns a view which makes a tree appear as a flat Fusion sequence. If the top-most node has a tag type T, then the elements of the flattened sequence are the children nodes that do not have tag type T Is there a way to customize the decision of which nodes to expand? users_guide\expression_construction\tags_and_meta_functions.html tag::posit: I don't like this name. It confused me at first since posit is a complete word which means something totally different. positive would be better. IMO. ternary ?: You can't overload that operator!? nary function call n-ary function call? users_guide\expression_introspection\if_and_not.html: if_< is_same< _arg, char const * >() > Why ()? Why a transform? Is this boost::is_same? users_guide\expression_introspection\defining_dsel_grammars.html: We can use matches<> and BOOST_MPL_ASSERT() to issue readable compile-time errors for invalid expressions, as below: I'm only partially convinced. The error message will be short, sure, but if the expression is complicated, it may not be enough information, since this a very coarse grained error message. users_guide\expression_transformation.html: A grammar decorated with transforms is a function object that takes three parameters: expr -- the Proto expression to transform state -- the initial state of the transformation visitor -- any optional mutable state information What type should "state" and "visitor" be? Why are they two different arguments? Do transforms work if the grammar has transforms at any level? Or do I need a transform at the top level? users_guide\expression_transformation\canned_transforms\if.html: The result of applying the first transform should be a compile-time Boolean. If it is true... Meaning it should be an MPL Integral Constant Wrapper? If non-zero? users_guide\expression_transformation\canned_transforms\make.html This uses proto:::make<> under the covers to evaluate the transform.) s/:::/::/ users_guide\expression_transformation\canned_transforms\when.html that it, when<> also behaves like a transform. s/it/is/ users_guide\expression_transformation\canned_transforms\fold_tree.html: fusion::cons<FoldTreeToList, _state>(FoldTreeToList, _state) I'm getting confused. Let me see... This is a transform that applies FoldTreeToList to its argument and uses the result to construct a fusion::cons with the current state. Question. Why reverse_fold_tree? would reverse_fold work as well because it would first evaluate the right side of the comma, creating a fusion::cons and then take the left side and push it onto the front? users_guide\expression_transformation\is_callable.html: ...a problem for a type like std::vector<_arg(_arg1)>(), which is a valid transform... Can't you substitute /first/ and /then/ check is_callable? users_guide\expression_extension\extends.html: What do I do if I want a statically initialized terminal that has a custom domain? Inhetitance from extends<> makes my type a non-aggregate... It looks like the comments in users_guide\examples\mixed.html in DereferenceCtx and IncrementCtx were copied from vector.html and not updated. Also, VectorOps should be MixedOps? boost\proto\args\hpp.html Why is args0 unary!? boost\proto\context\callable_eval.html: context(Expr::proto_tag(), arg_c\<0\>(expr), arg_c\<1\>(expr), ...) s/\// Why does callable_eval have no members listed? boost\proto\context\callable_context.html: Is it just more boostbook weirdness that the Synopsis has an empty eval<>? boost\proto\context\callable\hpp.html: Do you really need to show all the specializations of callable_eval? boost\proto\context\null\hpp.html ditto. boost\proto\display_expr_id535716.html: The one argument form writes to std::cout, right? boost\proto\domainns_\domain.html: boost::proto::domainns_::domain Is domainns_ a private namespace to block ADL? If so, it should not appear in the documentation... boost\proto\functional\eval.html: A general comment about eval: It should be noted that the context is *not* copied, and so it is safe for it to have mutable state. (Since this is not usual for function objects which is what callable_context implements) boost\proto\expr\hpp.html: I don't think that you should need to show all the specializations of expr in the documentation. exprns_ is a private namespace? It shouldn't appear in the documentation. boost\proto\exprns_\expr_Tag,Args,0__id530744.html: operator= is specified as returning the same type!? Is this a doxygen or boostbook problem? static expr::make... What are the overloads that take a size_t template parameter for? boost\proto\fusion\hpp.html What is the rational for only bringing flatten into namespace proto. boost\proto\is_extension.html: What does this do? boost\proto\operators\hpp.html: Why does if_else specify the return type while all the others say /unspecified/ boost\proto\proto_fwd\hpp.html: this is not pretty. boost\proto\transform\fold\hpp.html: Why does this say that the it contains fold_tree, but fold_tree doesn't appear in the listing. boost\proto\transform\reverse_fold.html: This says that reverse_fold simply inherits from fold!? boost\proto\transform\fold_tree.html: Is there some way to do a fold tree that recurses over some set of tags rather than a single fixed tag? boost\proto\transform\make.html: Function pointer types How are function pointer types interpreted as transforms? Oh. OK. They are treated the same as functions. Source: args.hpp: No comments. (Besides what I said about arg0 earlier) context.hpp: no comments. debug.hpp: why doesn't this use detail/prefix.hpp and suffix.hpp This should include detail/dont_care.hpp Why does the display_expr take the depth first? I would expect that users would want to override the default output stream more often then the starting depth. deep_copy.hpp: Ought to #include <boost/mpl/if.hpp> The global function object proto::deep_copy can cause the usual subtle ODR violation for objects in headers with internal linkage. I don't think that this header needs to #include <boost/proto/generate.hpp> domain.hpp: This should #include <boost/ref.hpp> and does not need to #include <boost/proto/generate.hpp> eval.hpp: No comments. (Yeah!) expr.hpp: Why is operator= only overloaded for non-const expr's when they are terminals? Same thing for operator[] Doesn't use #include <boost/preprocessor/arithmetic/inc.hpp> nor #include <boost/preprocessor/facilities/intercept.hpp> You're not using #include <boost/utility/result_of.hpp> extends.hpp: needs to #include <boost/preprocessor/repetition/enum_trailing_params.hpp> and <boost/preprocessor/repitition/repeat_from_to.hpp>. Does not need <boost/preprocessor/arithmetic/dec.hpp> or <boost/preprocessor/facilities/intercept.hpp> Should there be a BOOST_PROTO_EXTENDS_ALL macro, so that I don't have to write out all four macros? At the very least, you need to note that BOOST_PROTO_EXTENDS does not overload [], (), and =, since it differs from boost::proto_extends in this respect. fusion.hpp: Needs to #include <boost/mpl/if.hpp> Why do you put BOOST_PROTO_CALLABLE() in the definitions of functional::flatten/pop_front/reverse and then specialize is_callable too? The specializations of fusion::extension::is_view_impl use Iterator as the name of the template parameter of apply?? Why does value_of_impl<proto::tag::proto_expr_iterator> have a specialization of apply for references? The primary template works, doesn't it? generate.hpp: What is the point of expr_traits? Doesn't expr expose this information directly? It is in namespace detail so it isn't intended to be specialized by users is it? Is there a particular reason for not defining a generator as a polymorphic function object? Efficiency? The template template arguments for pod_generator are worrisome. Take a look at the example in extends.hpp for is_proto_expr. If I want to make a POD wrapper it seems that I need /two/ template arguments and thus cannot use pod_generator at all... make_expr.hpp: You might note the format of DATA in the macros. I'm finding it difficult to work out what they are doing. BOOST_PROTO_AT_TYPE(...) why add_const? The implementation of BOOST_PROTO_VARARG_TEMPLATE_YES_ converting a seq to a tuple to a list is clumsy. Why isn't it implemented using BOOST_PP_SEQ_FOR_EACH_I_R? Same thing goes for BOOST_PROTO_TEMPLATE_PARAMS_YES_ What is BOOST_PROTO_SEQ_PUSH_FRONT for? Oh. I get it. This is already being instantiated inside a BOOST_PP_SEQ_FOR_EACH_I... I'm thinking that these macros belong in their own file. matches.hpp Should include <boost/type_traits/is_array.hpp> I don't quite get vararg_matches. The specialization for Zero = false looks fishy. If I understand correctly, you are explicitly forcing the size to /include/ the vararg argument. In fact, I'm not sure that I like the fact that you are forcibly overriding the size of the arg sequence at the expr level at all. In addition, the argument "From" to vararg_matches_impl will skip over the argument that corresponds to the vararg. What am I missing? Oh. I see, vararg_matches_impl doesn't take a half-open range... Is there a noticeable performance improvement vs. doing this more naturally? Ok it looks right, but it took quite a bit of work to demonstrate. I think that you are testing the element that is in the same position as the vararg<> in both matches<> and vararg_matches<>, though? Why is matches_ specialized to return false for a terminal tag and proto::_? operators.hpp: no comments. proto.hpp: no comments. proto_fwd.hpp: Why do you hard code arg0, arg1, ... rather than using the preprocessor? I know it's easy but it's still not configurable. proto_typeof.hpp: no comments. ref.hpp: why does unref leave array references as references, but does not treat plain arrays differently from other values? In other files you create an object of type functional::* for unref you duplicate the overloads at namespace scope. It would be best to stick to a single style consistantly, IMO. tags.hpp: no comments. traits.hpp: I don't see what BOOST_MPL_AUX_LAMBDA_ARITY_PARAM in detail::callable is for. Why does as_expr not copy arrays? It differs from deep_copy in this? All the operator definitions are the same thing over and over and over again. Why don't you use a macro? Oh. Except the documentation comments... context/callable.hpp: no comments. context/default.hpp no comments. context/null.hpp no comments. detail/as_lvalue.hpp The name uncv is misleading since it is only unconst. detail/dont_care.hpp no comments. detail/funop.hpp: no comments. detail/pop_front.hpp: no comments. prefix.hpp: no comments. reverse.hpp: no comments. suffix.hpp: no comments. transform/arg.hpp: why does transform::_ref have a leading _ unlike expr? transform/bind.hpp: no comments. transform/fold.hpp: I don't think that detail::as_callable is a good name. It should be more like: bind_visitor_and_transform_for_fusion_fold transform/fold_tree.hpp: no comments. transform/make.hpp: Why are there specializations of construct_ for expr both for any number of arguments and for each number of arguments? The documentation says that If X is a template like Object2<Y0,Y1,...>, ... Otherwise, if X is a transform ... The tests actually happen in the opposite order. In other words, a templated transform is treated as a transform, not a template. transform/pass_through.hpp: no comments. transform/when.hpp no comments. General: It would probably better to define a macro BOOST_PROTO_HAS_SEPARATE_FUSION rather than constantly saying #if BOOST_VERSION < 103500 In Christ, Steven Watanabe
Steven Watanabe wrote:
AMDG
*I vote to accept proto*
Thanks.
Documentation
General: Do you think that you could use tag_of<> or ::proto_tag consistently?
I think in my code I consistently use ::proto_tag because it incurs one fewer template instantiation. tag_of<> is only provided for the times when you need a metafunction; e.g., in transforms.
users_guide\expression_construction.html The actual type of _1 looks like this: expr< tag::terminal, args0< placeholder1 >, 0 > Should this be args1< placeholder1 >? Ok. No. I'm not really happy with this, but I don't see a better way. I guess the best way to think of it is that the number represents the number of proto expressions children rather than the number of arguments.
Are you concerned with the "0" in args0? It represents the arity of the expression.
users_guide\expression_construction\operator_overloads.html Note The expr<> struct lives in the boost::proto namespace, as do all of Proto's operator overloads. The overloads are found via ADL (Argument-Dependent Lookup). That is why expressions must be "tainted" with Proto-ness for Proto to be able to build trees out of expressions. Is ADL the only reason? I should hope that the operators are overloaded to take expr<>.
They don't. The reason is because they should also work for user-defined types that extend Proto expressions. These do not necessarily inherit from expr<>. The operator overloads are suitably SFINAE'd so they won't gobble up anything they shouldn't.
users_guide\expression_construction\left_right_arg.html flatten() returns a view which makes a tree appear as a flat Fusion sequence. If the top-most node has a tag type T, then the elements of the flattened sequence are the children nodes that do not have tag type T Is there a way to customize the decision of which nodes to expand?
There isn't.
users_guide\expression_construction\tags_and_meta_functions.html tag::posit: I don't like this name. It confused me at first since posit is a complete word which means something totally different. positive would be better. IMO.
My (screwy?) logic is as follows: unary minus is called "negate" because there is a std::negate function object that applies unary minus. And what is the opposite of the verb "negate"? It's "posit".
ternary ?: You can't overload that operator!?
So? That doesn't mean I can't have a tag for it and process it in default_context with ?: as if it had been overloaded. Proto provides an if_else() ternary function, which behaves as the ?: operator.
nary function call n-ary function call?
operator(). Yes, should I hyphenate?
users_guide\expression_introspection\if_and_not.html: if_< is_same< _arg, char const * >() > Why ()? Why a transform? Is this boost::is_same?
Yes, this is boost::is_same. What else would I use there aside from a transform?
users_guide\expression_introspection\defining_dsel_grammars.html: We can use matches<> and BOOST_MPL_ASSERT() to issue readable compile-time errors for invalid expressions, as below: I'm only partially convinced. The error message will be short, sure, but if the expression is complicated, it may not be enough information, since this a very coarse grained error message.
It would be nice to be able to tell people *why* their pattern doesn't match the grammar. It's a hard problem and I haven't figured it out yet. It is probably better than the alternative, which would be to generate a cryptic error and megabytes of spew. For those willing to wade through the spew and examine the library source, it /might/ provide more context to understand the error. Those people are few, IMO.
users_guide\expression_transformation.html: A grammar decorated with transforms is a function object that takes three parameters: expr -- the Proto expression to transform state -- the initial state of the transformation visitor -- any optional mutable state information What type should "state" and "visitor" be? Why are they two different arguments?
Whatever you want. There are two different parameters because some transforms (e.g., fold) will update the state parameter automatically. Sometimes you want to pass around a mutable blob that only gets modified by you, not by fold. That's when you use a visitor. The Haskell "Scrap Your Boilerplate" technique (aka, The Mother of All Traversals) has similar parameters. The state is called an "accumulation" and the visitor is called the "context". I designed this part of Proto before I knew about the Haskell work. I think it's interesting that we converged on the same functional specification.
Do transforms work if the grammar has transforms at any level? Or do I need a transform at the top level?
All grammar elements have a default transform. So if the default on the top level is acceptable to you, you don't need to specify it.
users_guide\expression_transformation\canned_transforms\if.html: The result of applying the first transform should be a compile-time Boolean. If it is true... Meaning it should be an MPL Integral Constant Wrapper? If non-zero?
Yes.
users_guide\expression_transformation\canned_transforms\make.html This uses proto:::make<> under the covers to evaluate the transform.) s/:::/::/
Thanks.
users_guide\expression_transformation\canned_transforms\when.html that it, when<> also behaves like a transform. s/it/is/
Thanks.
users_guide\expression_transformation\canned_transforms\fold_tree.html: fusion::cons<FoldTreeToList, _state>(FoldTreeToList, _state) I'm getting confused. Let me see... This is a transform that applies FoldTreeToList to its argument and uses the result to construct a fusion::cons with the current state. Question. Why reverse_fold_tree? would reverse_fold work as well because it would first evaluate the right side of the comma, creating a fusion::cons and then take the left side and push it onto the front?
I think you'd end up with a cons<cons<>, cons<> >, which is not what you want. Try it and see!
users_guide\expression_transformation\is_callable.html: ...a problem for a type like std::vector<_arg(_arg1)>(), which is a valid transform... Can't you substitute /first/ and /then/ check is_callable?
I could do that, yes. And I seem to recall having a reason for not doing that. ... time passes ... Either the reason escapes me, or else my reasoning was wrong. I'll investigate this.
users_guide\expression_extension\extends.html: What do I do if I want a statically initialized terminal that has a custom domain? Inhetitance from extends<> makes my type a non-aggregate...
Right, there is a BOOST_PROTO_EXTENDS() macro for this, but it's already been noted that is isn't documented.
It looks like the comments in users_guide\examples\mixed.html in DereferenceCtx and IncrementCtx were copied from vector.html and not updated. Also, VectorOps should be MixedOps?
Thanks.
boost\proto\args\hpp.html Why is args0 unary!?
I'm sorry, where are you looking? args0<> does have one element in it: the type of the terminal. It has 0 child expressions, though, so its arity is really zero.
boost\proto\context\callable_eval.html: context(Expr::proto_tag(), arg_c\<0\>(expr), arg_c\<1\>(expr), ...) s/\//
Thanks.
Why does callable_eval have no members listed?
Because the primary template is indeed empty. Only the specializations have members.
boost\proto\context\callable_context.html: Is it just more boostbook weirdness that the Synopsis has an empty eval<>?
Yes, eval<> has a base that isn't showing up. BoostBook bug. Crud.
boost\proto\context\callable\hpp.html: Do you really need to show all the specializations of callable_eval?
Probably not, but getting something else would involve waging a costly war with our documentation toolchain.
boost\proto\context\null\hpp.html ditto.
Right.
boost\proto\display_expr_id535716.html: The one argument form writes to std::cout, right?
Yes, noted.
boost\proto\domainns_\domain.html: boost::proto::domainns_::domain Is domainns_ a private namespace to block ADL? If so, it should not appear in the documentation...
You're right. More battles with the documentation toolchain. :-(
boost\proto\functional\eval.html: A general comment about eval: It should be noted that the context is *not* copied, and so it is safe for it to have mutable state. (Since this is not usual for function objects which is what callable_context implements)
True.
boost\proto\expr\hpp.html: I don't think that you should need to show all the specializations of expr in the documentation. exprns_ is a private namespace? It shouldn't appear in the documentation.
I guess. :-/ The fact that these types live in special namespaces is a part of the interface, in a way, in that it avoid ADL problems. I'm of two minds about this.
boost\proto\exprns_\expr_Tag,Args,0__id530744.html: operator= is specified as returning the same type!? Is this a doxygen or boostbook problem?
Doxygen or BoostBook, I don't know.
static expr::make... What are the overloads that take a size_t template parameter for?
So that you can initialize a terminal<int[4]>::type -- that is, you can have a terminal that holds an array by value.
boost\proto\fusion\hpp.html What is the rational for only bringing flatten into namespace proto.
There's no sense having proto::pop_front() and proto::reverse(). They would do the same thing as fusion::pop_front() and fusion::reverse(). I wouldn't even need these if Fusion provided function object equivalents of its free functions.
boost\proto\is_extension.html: What does this do?
Another piece that fell through the cracks. It lets you non-intrusively adapt a type to be a Proto terminal, as long as you bring the proto::exops namespace into scope (which is also undocumented, argh!).
boost\proto\operators\hpp.html: Why does if_else specify the return type while all the others say /unspecified/
An implementation artifact. The binary operators do not have their return type expressed in terms of make_expr<> because it would incur unnecessary template instantiations. That's tractable with only 2 argumemts. With 3, the overload set and return type calculation became too tedious to write out, so I just used make_expr<> and required the parameters to be const&. Not ideal, but also not a big deal, IMO.
boost\proto\proto_fwd\hpp.html: this is not pretty.
It's a horrible mess. Doxygen doesn't emit documentation for forward declarations, but it does for typedefs. Not sure what to do about it. :-/
boost\proto\transform\fold\hpp.html: Why does this say that the it contains fold_tree, but fold_tree doesn't appear in the listing.
I can't imagine why. The comment from fold_tree.hpp seems to have mysteriously migrated there. Ah! In fold_tree.hpp, I have mistakenly labeled it with \file fold.hpp. Fixed.
boost\proto\transform\reverse_fold.html: This says that reverse_fold simply inherits from fold!?
[sigh] I know. reverse_fold really inherits from fold<call<_reverse(Sequence)>, State0, Fun>. I tried everything to get it to show up correctly, but lost that battle with Doxygen. Really, sometimes I wonder if Doxygen is more trouble than it's worth. At other times, I'm sure it is.
boost\proto\transform\fold_tree.html: Is there some way to do a fold tree that recurses over some set of tags rather than a single fixed tag?
No, but it wouldn't be difficult to define. fold_tree's implementation is dead simple.
boost\proto\transform\make.html: Function pointer types How are function pointer types interpreted as transforms? Oh. OK. They are treated the same as functions.
Source:
args.hpp: No comments. (Besides what I said about arg0 earlier)
context.hpp: no comments.
debug.hpp: why doesn't this use detail/prefix.hpp and suffix.hpp
It probably should.
This should include detail/dont_care.hpp
Right.
Why does the display_expr take the depth first? I would expect that users would want to override the default output stream more often then the starting depth.
You might be right. I have no strong feelings.
deep_copy.hpp: Ought to #include <boost/mpl/if.hpp>
Right.
The global function object proto::deep_copy can cause the usual subtle ODR violation for objects in headers with internal linkage.
Right. It should be a function, anyway, so ADL can find it.
I don't think that this header needs to #include <boost/proto/generate.hpp>
domain.hpp: This should #include <boost/ref.hpp> and does not need to
Right.
#include <boost/proto/generate.hpp>
Actually it needs this one because one of domain<>'s template parameters defaults to default_generator, defined in generate.hpp. People should be able to use domain<> without having to separately #include generate.hpp.
eval.hpp: No comments. (Yeah!)
Whew.
expr.hpp: Why is operator= only overloaded for non-const expr's when they are terminals? Same thing for operator[]
IMO, they're unlikely to ever be used and leaving them out measurably improves compile times. They would only conceivably be used in expression like (a+b)=x or (a+b)[x]. However, Proto's operator overloads return const rvalues, so a non-const operator= or operator[] on a non-terminal would never even be considered. Ditto for operator(), where leaving the const overloads out has a dramatic effect on compile times.
Doesn't use #include <boost/preprocessor/arithmetic/inc.hpp> nor #include <boost/preprocessor/facilities/intercept.hpp> You're not using #include <boost/utility/result_of.hpp>
Thanks.
extends.hpp: needs to #include <boost/preprocessor/repetition/enum_trailing_params.hpp> and <boost/preprocessor/repitition/repeat_from_to.hpp>. Does not need <boost/preprocessor/arithmetic/dec.hpp> or <boost/preprocessor/facilities/intercept.hpp>
Agreed, except for dec.hpp.
Should there be a BOOST_PROTO_EXTENDS_ALL macro, so that I don't have to write out all four macros? At the very least, you need to note that BOOST_PROTO_EXTENDS does not overload [], (), and =, since it differs from boost::proto_extends in this respect.
Right, I might rename BOOST_PROTO_EXTENDS to BOOST_PROTO_BASIC_EXTENDS, and have BOOST_PROTO_EXTENDS mean the same thing as extends<>. And document it. :-/
fusion.hpp: Needs to #include <boost/mpl/if.hpp>
Gotcha.
Why do you put BOOST_PROTO_CALLABLE() in the definitions of functional::flatten/pop_front/reverse and then specialize is_callable too?
You're right, I don't have to. The is_callable specialization shortcuts a few template instantiations, tho. And BOOST_PROTO_CALLABLE() is so that any types derived from flatten/pop_front/reverse are also callable. (As if anybody would want to do that.)
The specializations of fusion::extension::is_view_impl use Iterator as the name of the template parameter of apply??
Typo.
Why does value_of_impl<proto::tag::proto_expr_iterator> have a specialization of apply for references? The primary template works, doesn't it?
Yup.
generate.hpp: What is the point of expr_traits? Doesn't expr expose this information directly? It is in namespace detail so it isn't intended to be specialized by users is it?
Correct. I have to be very careful here not to do anything which could instantiate the expression type here. There are places in the code where the return type of an expression's member function is expressed in terms of generate, which is expressed in terms of the expression type, etc... Loopy compile problems.
Is there a particular reason for not defining a generator as a polymorphic function object? Efficiency?
No good reason. I should change it.
The template template arguments for pod_generator are worrisome. Take a look at the example in extends.hpp for is_proto_expr. If I want to make a POD wrapper it seems that I need /two/ template arguments and thus cannot use pod_generator at all...
Well, in that example, my_terminal<> is a terminal wrapper, not a general expression wrapper, so you wouldn't pass it to pod_generator. I understand your concern, but I don't want to use something like an mpl placeholder expression here because of overhead. The generator protocol is so dead simple that is generate<> and pod_generate<> don't mean your special needs, it takes ~2 minutes to bang together a custom generator that does.
make_expr.hpp: You might note the format of DATA in the macros. I'm finding it difficult to work out what they are doing.
What's your objection?
BOOST_PROTO_AT_TYPE(...) why add_const?
If fusion::value_at<> returns a reference, add_const is a no-op. Otherwise, it lets me pass rvalues. IIRC.
The implementation of BOOST_PROTO_VARARG_TEMPLATE_YES_ converting a seq to a tuple to a list is clumsy. Why isn't it implemented using BOOST_PP_SEQ_FOR_EACH_I_R? Same thing goes for BOOST_PROTO_TEMPLATE_PARAMS_YES_ What is BOOST_PROTO_SEQ_PUSH_FRONT for? Oh. I get it. This is already being instantiated inside a BOOST_PP_SEQ_FOR_EACH_I... I'm thinking that these macros belong in their own file.
It's all very complicated. I took me days to write those stupid macros. Please don't make me look at them again -- they make my eyes bleed. The best that can be said about them is that they work. I confess I'm a terrible PP metaprogrammer.
matches.hpp Should include <boost/type_traits/is_array.hpp>
Good catch. How do you do that?
I don't quite get vararg_matches. The specialization for Zero = false looks fishy. If I understand correctly, you are explicitly forcing the size to /include/ the vararg argument. In fact, I'm not sure that I like the fact that you are forcibly overriding the size of the arg sequence at the expr level at all. In addition, the argument "From" to vararg_matches_impl will skip over the argument that corresponds to the vararg. What am I missing? Oh. I see, vararg_matches_impl doesn't take a half-open range... Is there a noticeable performance improvement vs. doing this more naturally? Ok it looks right, but it took quite a bit of work to demonstrate.
I have sunk weeks into the implementation of proto::matches<> to make it as fast as possible. I play some dirty pool in there, I know. I think it's all kosher, tho.
I think that you are testing the element that is in the same position as the vararg<> in both matches<> and vararg_matches<>, though?
Really, where? I don't see that. Keep in mind that template instantiations are memoized by the compiler. Asking for the same instantiation twice is "free".
Why is matches_ specialized to return false for a terminal tag and proto::_?
Huh! That doesn't look right, does it? Could be a relic of the time when terminals were degenerate unary expressions.
operators.hpp: no comments.
proto.hpp: no comments.
proto_fwd.hpp: Why do you hard code arg0, arg1, ... rather than using the preprocessor? I know it's easy but it's still not configurable.
That's pretty dumb of me. I'll fix it.
proto_typeof.hpp: no comments.
ref.hpp: why does unref leave array references as references, but does not treat plain arrays differently from other values?
You're right, I'm being a bit schizo about array types. The question is, does deep_copy cause arrays to be stored by value? I worry about immense amounts of data getting copied all over. But maybe the answer is yes, it probably should. A simple reference wrapper avoids the copy in that case. At the very least, I need to be consistent.
In other files you create an object of type functional::* for unref you duplicate the overloads at namespace scope. It would be best to stick to a single style consistantly, IMO.
Yes, they should all be free functions, so ADL can find them.
tags.hpp: no comments.
traits.hpp: I don't see what BOOST_MPL_AUX_LAMBDA_ARITY_PARAM in detail::callable is for.
It's for working around gcc bugs.
Why does as_expr not copy arrays? It differs from deep_copy in this?
More schizo array weirdness. Will fix.
All the operator definitions are the same thing over and over and over again. Why don't you use a macro? Oh. Except the documentation comments...
Yes, it's for Doxygen.
context/callable.hpp: no comments.
context/default.hpp no comments.
context/null.hpp no comments.
detail/as_lvalue.hpp The name uncv is misleading since it is only unconst.
It's a detail, but ok.
detail/dont_care.hpp no comments.
detail/funop.hpp: no comments.
detail/pop_front.hpp: no comments.
prefix.hpp: no comments.
reverse.hpp: no comments.
suffix.hpp: no comments.
transform/arg.hpp: why does transform::_ref have a leading _ unlike expr?
Because if it were just "ref" then, with gcc's strange name look-up implementation, it would be ambiguous with the boost::ref() function. Crazy and stupid, but not my fault.
transform/bind.hpp: no comments.
transform/fold.hpp: I don't think that detail::as_callable is a good name. It should be more like: bind_visitor_and_transform_for_fusion_fold
Ha! A bit verbose, perhaps...
transform/fold_tree.hpp: no comments.
transform/make.hpp: Why are there specializations of construct_ for expr both for any number of arguments and for each number of arguments?
Damned if I know. :-D I'll fix it.
The documentation says that If X is a template like Object2<Y0,Y1,...>, ... Otherwise, if X is a transform ... The tests actually happen in the opposite order. In other words, a templated transform is treated as a transform, not a template.
Great catch. Looks like a documentation bug, I think the code is doing the right thing.
transform/pass_through.hpp: no comments.
transform/when.hpp no comments.
General:
It would probably better to define a macro BOOST_PROTO_HAS_SEPARATE_FUSION rather than constantly saying #if BOOST_VERSION < 103500
Why is that better? You obviously put a huge effort into this review. I'm deeply grateful for your contribution. Thank you. -- Eric Niebler Boost Consulting www.boost-consulting.com
From: Eric Niebler
Steven Watanabe wrote:
users_guide\expression_construction\tags_and_meta_functions.html tag::posit: I don't like this name. It confused me at first since posit is a complete word which means something totally different. positive would be better. IMO.
My (screwy?) logic is as follows: unary minus is called "negate" because there is a std::negate function object that applies unary minus. And what is the opposite of the verb "negate"? It's "posit".
?? The New Collins Concise English Dictionary (1982) by my desk has: Negate: 1. to nullify; invalidate. 2. to contradict Posit: 1. to assume or put forward as fact or the factual basis for an argument. 2. to put in position. I don't really see how you can say that "to posit" is the opposite of "to negate".
ternary ?: You can't overload that operator!?
So? That doesn't mean I can't have a tag for it and process it in default_context with ?: as if it had been overloaded. Proto provides an if_else() ternary function, which behaves as the ?: operator.
nary function call n-ary function call?
operator(). Yes, should I hyphenate?
users_guide\expression_introspection\if_and_not.html: if_< is_same< _arg, char const * >() > Why ()? Why a transform? Is this boost::is_same?
Yes, this is boost::is_same. What else would I use there aside from a transform?
users_guide\expression_introspection\defining_dsel_grammars.html: We can use matches<> and BOOST_MPL_ASSERT() to issue readable compile-time errors for invalid expressions, as below: I'm only partially convinced. The error message will be short, sure, but if the expression is complicated, it may not be enough information, since this a very coarse grained error message.
It would be nice to be able to tell people *why* their pattern doesn't match the grammar. It's a hard problem and I haven't figured it out yet.
It is probably better than the alternative, which would be to generate a cryptic error and megabytes of spew. For those willing to wade through the spew and examine the library source, it /might/ provide more context to understand the error. Those people are few, IMO.
users_guide\expression_transformation.html: A grammar decorated with transforms is a function object that takes three parameters: expr -- the Proto expression to transform state -- the initial state of the transformation visitor -- any optional mutable state information What type should "state" and "visitor" be? Why are they two different arguments?
Whatever you want. There are two different parameters because some transforms (e.g., fold) will update the state parameter automatically. Sometimes you want to pass around a mutable blob that only gets modified by you, not by fold. That's when you use a visitor.
The Haskell "Scrap Your Boilerplate" technique (aka, The Mother of All Traversals) has similar parameters. The state is called an "accumulation" and the visitor is called the "context". I designed this part of Proto before I knew about the Haskell work. I think it's interesting that we converged on the same functional specification.
Do transforms work if the grammar has transforms at any level? Or do I need a transform at the top level?
All grammar elements have a default transform. So if the default on the top level is acceptable to you, you don't need to specify it.
users_guide\expression_transformation\canned_transforms\if.html: The result of applying the first transform should be a compile-time Boolean. If it is true... Meaning it should be an MPL Integral Constant Wrapper? If non-zero?
Yes.
users_guide\expression_transformation\canned_transforms\make.html This uses proto:::make<> under the covers to evaluate the transform.) s/:::/::/
Thanks.
users_guide\expression_transformation\canned_transforms\when.html that it, when<> also behaves like a transform. s/it/is/
Thanks.
users_guide\expression_transformation\canned_transforms\fold_tree.html:
fusion::cons<FoldTreeToList, _state>(FoldTreeToList, _state) I'm getting confused. Let me see... This is a transform that applies FoldTreeToList to its argument and uses the result to construct a fusion::cons with the current state. Question. Why reverse_fold_tree? would reverse_fold work as well because it would first evaluate the right side of the comma, creating a fusion::cons and then take the left side and push it onto the front?
I think you'd end up with a cons<cons<>, cons<> >, which is not what you want. Try it and see!
users_guide\expression_transformation\is_callable.html: ...a problem for a type like std::vector<_arg(_arg1)>(), which is a valid transform... Can't you substitute /first/ and /then/ check is_callable?
I could do that, yes. And I seem to recall having a reason for not doing that.
... time passes ...
Either the reason escapes me, or else my reasoning was wrong. I'll investigate this.
users_guide\expression_extension\extends.html: What do I do if I want a statically initialized terminal that has a custom domain? Inhetitance from extends<> makes my type a non-aggregate...
Right, there is a BOOST_PROTO_EXTENDS() macro for this, but it's already been noted that is isn't documented.
It looks like the comments in users_guide\examples\mixed.html in DereferenceCtx and IncrementCtx were copied from vector.html and not updated. Also, VectorOps should be MixedOps?
Thanks.
boost\proto\args\hpp.html Why is args0 unary!?
I'm sorry, where are you looking? args0<> does have one element in it: the type of the terminal. It has 0 child expressions, though, so its arity is really zero.
boost\proto\context\callable_eval.html: context(Expr::proto_tag(), arg_c\<0\>(expr), arg_c\<1\>(expr), ...) s/\//
Thanks.
Why does callable_eval have no members listed?
Because the primary template is indeed empty. Only the specializations have members.
boost\proto\context\callable_context.html: Is it just more boostbook weirdness that the Synopsis has an empty eval<>?
Yes, eval<> has a base that isn't showing up. BoostBook bug. Crud.
boost\proto\context\callable\hpp.html: Do you really need to show all the specializations of callable_eval?
Probably not, but getting something else would involve waging a costly war with our documentation toolchain.
boost\proto\context\null\hpp.html ditto.
Right.
boost\proto\display_expr_id535716.html: The one argument form writes to std::cout, right?
Yes, noted.
boost\proto\domainns_\domain.html: boost::proto::domainns_::domain Is domainns_ a private namespace to block ADL? If so, it should not appear in the documentation...
You're right. More battles with the documentation toolchain. :-(
boost\proto\functional\eval.html: A general comment about eval: It should be noted that the context is *not* copied, and so it is safe for it to have mutable state. (Since this is not usual for function objects which is what callable_context implements)
True.
boost\proto\expr\hpp.html: I don't think that you should need to show all the specializations of expr in the documentation. exprns_ is a private namespace? It shouldn't appear in the documentation.
I guess. :-/ The fact that these types live in special namespaces is a part of the interface, in a way, in that it avoid ADL problems. I'm of two minds about this.
boost\proto\exprns_\expr_Tag,Args,0__id530744.html: operator= is specified as returning the same type!? Is this a doxygen or boostbook problem?
Doxygen or BoostBook, I don't know.
static expr::make... What are the overloads that take a size_t template parameter for?
So that you can initialize a terminal<int[4]>::type -- that is, you can have a terminal that holds an array by value.
boost\proto\fusion\hpp.html What is the rational for only bringing flatten into namespace proto.
There's no sense having proto::pop_front() and proto::reverse(). They would do the same thing as fusion::pop_front() and fusion::reverse(). I wouldn't even need these if Fusion provided function object equivalents of its free functions.
boost\proto\is_extension.html: What does this do?
Another piece that fell through the cracks. It lets you non-intrusively adapt a type to be a Proto terminal, as long as you bring the proto::exops namespace into scope (which is also undocumented, argh!).
boost\proto\operators\hpp.html: Why does if_else specify the return type while all the others say /unspecified/
An implementation artifact. The binary operators do not have their return type expressed in terms of make_expr<> because it would incur unnecessary template instantiations. That's tractable with only 2 argumemts. With 3, the overload set and return type calculation became too tedious to write out, so I just used make_expr<> and required the parameters to be const&. Not ideal, but also not a big deal, IMO.
boost\proto\proto_fwd\hpp.html: this is not pretty.
It's a horrible mess. Doxygen doesn't emit documentation for forward declarations, but it does for typedefs. Not sure what to do about it. :-/
boost\proto\transform\fold\hpp.html: Why does this say that the it contains fold_tree, but fold_tree doesn't appear in the listing.
I can't imagine why. The comment from fold_tree.hpp seems to have mysteriously migrated there. Ah! In fold_tree.hpp, I have mistakenly labeled it with \file fold.hpp. Fixed.
boost\proto\transform\reverse_fold.html: This says that reverse_fold simply inherits from fold!?
[sigh] I know. reverse_fold really inherits from fold<call<_reverse(Sequence)>, State0, Fun>. I tried everything to get it to show up correctly, but lost that battle with Doxygen. Really, sometimes I wonder if Doxygen is more trouble than it's worth. At other times, I'm sure it is.
boost\proto\transform\fold_tree.html: Is there some way to do a fold tree that recurses over some set of tags rather than a single fixed tag?
No, but it wouldn't be difficult to define. fold_tree's implementation is dead simple.
boost\proto\transform\make.html: Function pointer types How are function pointer types interpreted as transforms? Oh. OK. They are treated the same as functions.
Source:
args.hpp: No comments. (Besides what I said about arg0 earlier)
context.hpp: no comments.
debug.hpp: why doesn't this use detail/prefix.hpp and suffix.hpp
It probably should.
This should include detail/dont_care.hpp
Right.
Why does the display_expr take the depth first? I would expect that users would want to override the default output stream more often then the starting depth.
You might be right. I have no strong feelings.
deep_copy.hpp: Ought to #include <boost/mpl/if.hpp>
Right.
The global function object proto::deep_copy can cause the usual subtle ODR violation for objects in headers with internal linkage.
Right. It should be a function, anyway, so ADL can find it.
I don't think that this header needs to #include <boost/proto/generate.hpp>
domain.hpp: This should #include <boost/ref.hpp> and does not need to
Right.
#include <boost/proto/generate.hpp>
Actually it needs this one because one of domain<>'s template parameters defaults to default_generator, defined in generate.hpp. People should be able to use domain<> without having to separately #include generate.hpp.
eval.hpp: No comments. (Yeah!)
Whew.
expr.hpp: Why is operator= only overloaded for non-const expr's when they are terminals? Same thing for operator[]
IMO, they're unlikely to ever be used and leaving them out measurably improves compile times. They would only conceivably be used in expression like (a+b)=x or (a+b)[x]. However, Proto's operator overloads return const rvalues, so a non-const operator= or operator[] on a non-terminal would never even be considered. Ditto for operator(), where leaving the const overloads out has a dramatic effect on compile times.
Doesn't use #include <boost/preprocessor/arithmetic/inc.hpp> nor #include <boost/preprocessor/facilities/intercept.hpp> You're not using #include <boost/utility/result_of.hpp>
Thanks.
extends.hpp: needs to #include <boost/preprocessor/repetition/enum_trailing_params.hpp> and <boost/preprocessor/repitition/repeat_from_to.hpp>. Does not need <boost/preprocessor/arithmetic/dec.hpp> or <boost/preprocessor/facilities/intercept.hpp>
Agreed, except for dec.hpp.
Should there be a BOOST_PROTO_EXTENDS_ALL macro, so that I don't have to write out all four macros? At the very least, you need to note that BOOST_PROTO_EXTENDS does not overload [], (), and =, since it differs from boost::proto_extends in this respect.
Right, I might rename BOOST_PROTO_EXTENDS to BOOST_PROTO_BASIC_EXTENDS, and have BOOST_PROTO_EXTENDS mean the same thing as extends<>. And document it. :-/
fusion.hpp: Needs to #include <boost/mpl/if.hpp>
Gotcha.
Why do you put BOOST_PROTO_CALLABLE() in the definitions of functional::flatten/pop_front/reverse and then specialize is_callable too?
You're right, I don't have to. The is_callable specialization shortcuts a few template instantiations, tho. And BOOST_PROTO_CALLABLE() is so that any types derived from flatten/pop_front/reverse are also callable. (As if anybody would want to do that.)
The specializations of fusion::extension::is_view_impl use Iterator as the name of the template parameter of apply??
Typo.
Why does value_of_impl<proto::tag::proto_expr_iterator> have a specialization of apply for references? The primary template works, doesn't it?
Yup.
generate.hpp: What is the point of expr_traits? Doesn't expr expose this information directly? It is in namespace detail so it isn't intended to be specialized by users is it?
Correct. I have to be very careful here not to do anything which could instantiate the expression type here. There are places in the code where the return type of an expression's member function is expressed in terms of generate, which is expressed in terms of the expression type, etc... Loopy compile problems.
Is there a particular reason for not defining a generator as a polymorphic function object? Efficiency?
No good reason. I should change it.
The template template arguments for pod_generator are worrisome. Take a look at the example in extends.hpp for is_proto_expr. If I want to make a POD wrapper it seems that I need /two/ template arguments and thus cannot use pod_generator at all...
Well, in that example, my_terminal<> is a terminal wrapper, not a general expression wrapper, so you wouldn't pass it to pod_generator. I understand your concern, but I don't want to use something like an mpl placeholder expression here because of overhead. The generator protocol is so dead simple that is generate<> and pod_generate<> don't mean your special needs, it takes ~2 minutes to bang together a custom generator that does.
make_expr.hpp: You might note the format of DATA in the macros. I'm finding it difficult to work out what they are doing.
What's your objection?
BOOST_PROTO_AT_TYPE(...) why add_const?
If fusion::value_at<> returns a reference, add_const is a no-op. Otherwise, it lets me pass rvalues. IIRC.
The implementation of BOOST_PROTO_VARARG_TEMPLATE_YES_ converting a seq to a tuple to a list is clumsy. Why isn't it implemented using BOOST_PP_SEQ_FOR_EACH_I_R? Same thing goes for BOOST_PROTO_TEMPLATE_PARAMS_YES_ What is BOOST_PROTO_SEQ_PUSH_FRONT for? Oh. I get it. This is already being instantiated inside a BOOST_PP_SEQ_FOR_EACH_I... I'm thinking that these macros belong in their own file.
It's all very complicated. I took me days to write those stupid macros. Please don't make me look at them again -- they make my eyes bleed. The best that can be said about them is that they work. I confess I'm a terrible PP metaprogrammer.
matches.hpp Should include <boost/type_traits/is_array.hpp>
Good catch. How do you do that?
I don't quite get vararg_matches. The specialization for Zero = false looks fishy. If I understand correctly, you are explicitly forcing the size to /include/ the vararg argument. In fact, I'm not sure that I like the fact that you are forcibly overriding the size of the arg sequence at the expr level at all. In addition, the argument "From" to vararg_matches_impl will skip over the argument that corresponds to the vararg. What am I missing? Oh. I see, vararg_matches_impl doesn't take a half-open range... Is there a noticeable performance improvement vs. doing this more naturally? Ok it looks right, but it took quite a bit of work to demonstrate.
I have sunk weeks into the implementation of proto::matches<> to make it as fast as possible. I play some dirty pool in there, I know. I think it's all kosher, tho.
I think that you are testing the element that is in the same position as the vararg<> in both matches<> and vararg_matches<>, though?
Really, where? I don't see that. Keep in mind that template instantiations are memoized by the compiler. Asking for the same instantiation twice is "free".
Why is matches_ specialized to return false for a terminal tag and proto::_?
Huh! That doesn't look right, does it? Could be a relic of the time when terminals were degenerate unary expressions.
operators.hpp: no comments.
proto.hpp: no comments.
proto_fwd.hpp: Why do you hard code arg0, arg1, ... rather than using the preprocessor? I know it's easy but it's still not configurable.
That's pretty dumb of me. I'll fix it.
proto_typeof.hpp: no comments.
ref.hpp: why does unref leave array references as references, but does not treat plain arrays differently from other values?
You're right, I'm being a bit schizo about array types. The question is, does deep_copy cause arrays to be stored by value? I worry about immense amounts of data getting copied all over. But maybe the answer is yes, it probably should. A simple reference wrapper avoids the copy in that case. At the very least, I need to be consistent.
In other files you create an object of type functional::* for unref you duplicate the overloads at namespace scope. It would be best to stick to a single style consistantly, IMO.
Yes, they should all be free functions, so ADL can find them.
tags.hpp: no comments.
traits.hpp: I don't see what BOOST_MPL_AUX_LAMBDA_ARITY_PARAM in detail::callable is for.
It's for working around gcc bugs.
Why does as_expr not copy arrays? It differs from deep_copy in this?
More schizo array weirdness. Will fix.
All the operator definitions are the same thing over and over and over again. Why don't you use a macro? Oh. Except the documentation comments...
Yes, it's for Doxygen.
context/callable.hpp: no comments.
context/default.hpp no comments.
context/null.hpp no comments.
detail/as_lvalue.hpp The name uncv is misleading since it is only unconst.
It's a detail, but ok.
detail/dont_care.hpp no comments.
detail/funop.hpp: no comments.
detail/pop_front.hpp: no comments.
prefix.hpp: no comments.
reverse.hpp: no comments.
suffix.hpp: no comments.
transform/arg.hpp: why does transform::_ref have a leading _ unlike expr?
Because if it were just "ref" then, with gcc's strange name look-up implementation, it would be ambiguous with the boost::ref() function. Crazy and stupid, but not my fault.
transform/bind.hpp: no comments.
transform/fold.hpp: I don't think that detail::as_callable is a good name. It should be more like: bind_visitor_and_transform_for_fusion_fold
Ha! A bit verbose, perhaps...
transform/fold_tree.hpp: no comments.
transform/make.hpp: Why are there specializations of construct_ for expr both for any number of arguments and for each number of arguments?
Damned if I know. :-D I'll fix it.
The documentation says that If X is a template like Object2<Y0,Y1,...>, ... Otherwise, if X is a transform ... The tests actually happen in the opposite order. In other words, a templated transform is treated as a transform, not a template.
Great catch. Looks like a documentation bug, I think the code is doing the right thing.
transform/pass_through.hpp: no comments.
transform/when.hpp no comments.
General:
It would probably better to define a macro BOOST_PROTO_HAS_SEPARATE_FUSION rather than constantly saying #if BOOST_VERSION < 103500
Why is that better?
You obviously put a huge effort into this review. I'm deeply grateful for your contribution. Thank you.
-- Martin Bonner Senior Software Engineer/Team Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com disclaimer
Martin Bonner wrote: [something possibly useful] .... then forgot to snip >600 lines of discussion to which he had nothing to add. Sorry :-) -- Martin Bonner Senior Software Engineer/Team Leader PI SHURLOK LTD Telephone: +44 1223 441434 / 203894 (direct) Fax: +44 1223 203999 Email: martin.bonner@pi-shurlok.com www.pi-shurlok.com disclaimer
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Martin Bonner Sent: 14 March 2008 10:33 To: boost@lists.boost.org Subject: Re: [boost] Proto review
From: Eric Niebler
Steven Watanabe wrote:
users_guide\expression_construction\tags_and_meta_functions.html tag::posit: I don't like this name. It confused me at first since posit is a complete word which means something totally different. positive would be better. IMO.
My (screwy?) logic is as follows: unary minus is called "negate" because there is a std::negate function object that applies unary minus. And what is the opposite of the verb "negate"? It's "posit".
?? The New Collins Concise English Dictionary (1982) by my desk has:
Negate: 1. to nullify; invalidate. 2. to contradict
Posit: 1. to assume or put forward as fact or the factual basis for an argument. 2. to put in position.
I don't really see how you can say that "to posit" is the opposite of "to negate".
A little googling shows that various people have already invented "posate" as the opposite of 'negate' and this makes much more sense to me, even if it hasn't made it into the Oxford English Dictionary yet. ("posate" may mean something rude in italian too, but hey/cioa ;-) Paul --- Paul A Bristow Prizet Farmhouse, Kendal, Cumbria UK LA8 8AB +44 1539561830 & SMS, Mobile +44 7714 330204 & SMS pbristow@hetp.u-net.com
Paul A Bristow wrote:
A little googling shows that various people have already invented "posate" as the opposite of 'negate' and this makes much more sense to me, even if it hasn't made it into the Oxford English Dictionary yet.
("posate" may mean something rude in italian too, but hey/cioa ;-)
I like posate.
On Fri, 14 Mar 2008, Michael Marcin wrote:
Paul A Bristow wrote:
A little googling shows that various people have already invented "posate" as the opposite of 'negate' and this makes much more sense to me, even if it hasn't made it into the Oxford English Dictionary yet.
I like posate.
I don't. +x and -x are entirely different beasts. Negative/negate is an action; generally (x)!=(-x) and (x)==(-(-x)). Positive is not; generally (x)==(+x). The closest action is identity(x), not abs(x) since (+x)!=abs(x). But identity is a poor name since it is generally considered to be different than +. For example, "identity(x)==+x" is fine but "identity(5)==+5" feels wrong. Joel's affirm falls into the same category as identity. ~ Daniel
dherring@ll.mit.edu wrote:
On Fri, 14 Mar 2008, Michael Marcin wrote:
I like posate. I don't. <snip>
This discussion is taking on bike shed characteristics. I don't want a simple naming issue to overshadow issues of substance. I will agree to reconsider the name for the unary plus operator. Let's move on. If you haven't already, why not write a review of Proto? :-) -- Eric Niebler Boost Consulting www.boost-consulting.com
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Eric Niebler Sent: 14 March 2008 21:28 To: boost@lists.boost.org Subject: Re: [boost] Proto review
dherring@ll.mit.edu wrote:
On Fri, 14 Mar 2008, Michael Marcin wrote:
I like posate. I don't. <snip>
This discussion is taking on bike shed characteristics.
Oh Woe - the ultimate put-down - being accused of starting a bike-shed discussion :-((
I don't want a simple naming issue to overshadow issues of substance. I will agree to reconsider the name for the unary plus operator. Let's move on.
If you haven't already, why not write a review of Proto? :-)
A glance at the docs led me to conclude that it's so obviously a must-have that I wouldn't presume to ;-) (You can count that as a 'yes' vote if you need to). Paul --- Paul A Bristow Prizet Farmhouse, Kendal, Cumbria UK LA8 8AB +44 1539561830 & SMS, Mobile +44 7714 330204 & SMS pbristow@hetp.u-net.com
Paul A Bristow wrote:
dherring@ll.mit.edu wrote:
On Fri, 14 Mar 2008, Michael Marcin wrote:
I like posate. I don't. <snip>
This discussion is taking on bike shed characteristics.
Oh Woe - the ultimate put-down - being accused of starting a bike-shed discussion :-((
Naming issues are important. Maybe I was wrong to cut off that discussion. Sorry if I came off as strident. -- Eric Niebler Boost Consulting www.boost-consulting.com
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Eric Niebler Sent: 15 March 2008 19:17 To: boost@lists.boost.org Subject: Re: [boost] Proto review
Paul A Bristow wrote:
dherring@ll.mit.edu wrote:
On Fri, 14 Mar 2008, Michael Marcin wrote:
I like posate. I don't. <snip>
This discussion is taking on bike shed characteristics.
Oh Woe - the ultimate put-down - being accused of starting a bike-shed discussion :-((
Naming issues are important. Maybe I was wrong to cut off that discussion. Sorry if I came off as strident.
But correct :-) Paul --- Paul A Bristow Prizet Farmhouse, Kendal, Cumbria UK LA8 8AB +44 1539561830 & SMS, Mobile +44 7714 330204 & SMS pbristow@hetp.u-net.com
Martin Bonner wrote:
?? The New Collins Concise English Dictionary (1982) by my desk has:
Negate: 1. to nullify; invalidate. 2. to contradict
Posit: 1. to assume or put forward as fact or the factual basis for an argument. 2. to put in position.
I don't really see how you can say that "to posit" is the opposite of "to negate".
?? "To put forward as fact" seems like the opposite of "to contradict" to me. One says, "it's true!" The other, "it's false!" The only other term that I have found that is acceptable for the unary plus operator is "unary_plus." That just seems so ... boring. ;-) And a little strange to have "negate" and "unary_plus". Blech. Is there a mathematician who can tell me if the operation performed by the unary plus operator has a name? -- Eric Niebler Boost Consulting www.boost-consulting.com
On Fri, Mar 14, 2008 at 3:50 PM, Eric Niebler <eric@boost-consulting.com> wrote:
Martin Bonner wrote:
?? The New Collins Concise English Dictionary (1982) by my desk has:
Negate: 1. to nullify; invalidate. 2. to contradict
Posit: 1. to assume or put forward as fact or the factual basis for an argument. 2. to put in position.
I don't really see how you can say that "to posit" is the opposite of "to negate".
?? "To put forward as fact" seems like the opposite of "to contradict" to me. One says, "it's true!" The other, "it's false!"
The only other term that I have found that is acceptable for the unary plus operator is "unary_plus." That just seems so ... boring. ;-) And a little strange to have "negate" and "unary_plus". Blech.
Is there a mathematician who can tell me if the operation performed by the unary plus operator has a name?
What about identity? ;) just-kidding-ly-yours, -- gpd
On Fri, 14 Mar 2008, Eric Niebler wrote:
The only other term that I have found that is acceptable for the unary plus operator is "unary_plus." That just seems so ... boring. ;-) And a little strange to have "negate" and "unary_plus". Blech.
Is there a mathematician who can tell me if the operation performed by the unary plus operator has a name?
Absolute value? I believe the following names are gaining traction. unary binary ----------------- + positive plus - negative minus This is reflected in the Itanium C++ ABI name mangling as unary binary ----------------- + ps pl - nv mi ~ Daniel
AMDG Eric Niebler wrote:
users_guide\expression_construction\operator_overloads.html Note The expr<> struct lives in the boost::proto namespace, as do all of Proto's operator overloads. The overloads are found via ADL (Argument-Dependent Lookup). That is why expressions must be "tainted" with Proto-ness for Proto to be able to build trees out of expressions. Is ADL the only reason? I should hope that the operators are overloaded to take expr<>.
They don't. The reason is because they should also work for user-defined types that extend Proto expressions. These do not necessarily inherit from expr<>. The operator overloads are suitably SFINAE'd so they won't gobble up anything they shouldn't.
Ok. I'm happy as long as there is some defense against wild overloads that match anything.
boost\proto\args\hpp.html Why is args0 unary!?
I'm sorry, where are you looking? args0<> does have one element in it: the type of the terminal. It has 0 child expressions, though, so its arity is really zero.
I think I understand now.
static expr::make... What are the overloads that take a size_t template parameter for?
So that you can initialize a terminal<int[4]>::type -- that is, you can have a terminal that holds an array by value.
It looks like doxygen/boostbook is getting confused here, because the fact that the argument is an array doesn't show up.
expr.hpp: Why is operator= only overloaded for non-const expr's when they are terminals? Same thing for operator[]
IMO, they're unlikely to ever be used and leaving them out measurably improves compile times. They would only conceivably be used in expression like (a+b)=x or (a+b)[x]. However, Proto's operator overloads return const rvalues, so a non-const operator= or operator[] on a non-terminal would never even be considered. Ditto for operator(), where leaving the const overloads out has a dramatic effect on compile times.
Ok. Anyway, the const overloads will work for non-const arguments.
extends.hpp: needs to #include <boost/preprocessor/repetition/enum_trailing_params.hpp> and <boost/preprocessor/repitition/repeat_from_to.hpp>. Does not need <boost/preprocessor/arithmetic/dec.hpp> or <boost/preprocessor/facilities/intercept.hpp>
Agreed, except for dec.hpp.
Right. I must have searched for dec rather than DEC...
The template template arguments for pod_generator are worrisome. Take a look at the example in extends.hpp for is_proto_expr. If I want to make a POD wrapper it seems that I need /two/ template arguments and thus cannot use pod_generator at all...
Well, in that example, my_terminal<> is a terminal wrapper, not a general expression wrapper, so you wouldn't pass it to pod_generator. I understand your concern, but I don't want to use something like an mpl placeholder expression here because of overhead. The generator protocol is so dead simple that is generate<> and pod_generate<> don't mean your special needs, it takes ~2 minutes to bang together a custom generator that does.
Ok. As long as it's documented what the interface I have to meet is.
make_expr.hpp: You might note the format of DATA in the macros. I'm finding it difficult to work out what they are doing.
What's your objection?
Just that a few comments would have made it much easier to follow.
I have sunk weeks into the implementation of proto::matches<> to make it as fast as possible. I play some dirty pool in there, I know. I think it's all kosher, tho.
Ok. I'll suggest though that vararg_matches_impl should take a closed range i.e. [1, 3] to check the arguments 1, 2, and 3 rather than offsetting it by one [2, 4] to match arguments 1, 2, and 3.
I think that you are testing the element that is in the same position as the vararg<> in both matches<> and vararg_matches<>, though?
Really, where? I don't see that. Keep in mind that template instantiations are memoized by the compiler. Asking for the same instantiation twice is "free".
Never mind. I was confused.
General:
It would probably better to define a macro BOOST_PROTO_HAS_SEPARATE_FUSION rather than constantly saying #if BOOST_VERSION < 103500
Why is that better?
Just a general prejudice against "magic" numbers In Christ, Steven Watanabe
Eric Niebler wrote:
Steven Watanabe wrote:
users_guide\expression_transformation\is_callable.html: ...a problem for a type like std::vector<_arg(_arg1)>(), which is a valid transform... Can't you substitute /first/ and /then/ check is_callable?
I could do that, yes. And I seem to recall having a reason for not doing that.
... time passes ...
Either the reason escapes me, or else my reasoning was wrong. I'll investigate this.
In trying to implement this, I reminded myself of why it doesn't work. Take the case of wanting to use functional::make_expr<> as a transform. I'll use as my example a variation of the Distribute transform I wrote for Markus Werle. using functional::make_expr; struct Distribute : when< or_<plus<_,_>, minus<_,_> > , make_expr<tag_of<_> >( Recurse(_left) , Recurse(_right)) ) > {}; The idea is to match a plus or a minus and create a new node *of the same type* with transformed children. The above transform doesn't work as-is today. The reason is because Proto first checks is_callable<make_expr<tag_of<_> > >::value before doing anything. It's true, and so we never substitute tag_of<_> before invoking the make_expr<> transform. You might say, "well, don't check is_callable<> first. Just do the substitution and check after." There are two things wrong with that. The first is that it is slow. Every template type must be completely disassembled and reassembled before you know what to do with it. But the killer is that in some cases it, it is simply wrong. Often I pass one transform as a template parameter to another. For instance, foo<_arg> might be a transform that first applies the _arg transform, and then does foo on it. If Proto actually replaced _arg with an expression before invoking the foo<> transform, it would be an error! Another way to see the problem is to consider that, within a transform, the following should logically behave identically: typedef foo<_arg> Foo; struct Foo : foo<_arg> {}; These are two equivalent ways of defining some transform Foo today. If I took the "substitute first, ask questions later" approach, these would mean different things, because the _arg in the first line would be subject to substitution, but the _arg in the second would not. You might then say, "well, the problem is that is_callable<make_expr<X>
::value was true. It should be false." But no, that's obviously wrong because make_expr<> really *is* callable. Saying it's not would break make_expr<tag::plus> ...
You can chase the reasoning to all sorts of strange places. is_callable<X> should rip apart templates and examine their constituents to determine the result -- but that breaks things too, like the foo<_arg> example which should be invoked as-is without substitution. It comes down to this ... Proto can't know whether you want substitution first or not, so it errs on the side of doing the simple and efficient thing. If you want to do substitution first and *then* invoke the resulting callable transform, you can use bind: using functional::make_expr; struct Distribute : when< or_<plus<_,_>, minus<_,_> > , bind< make_expr<tag_of<_> >( Recurse(_left) , Recurse(_right)) ) > > {}; (I think bind may be incorrectly named. I may call it "lazy".) All this belongs in a rationale, if only so that I don't forget it again. -- Eric Niebler Boost Consulting www.boost-consulting.com
participants (8)
-
dherring＠ll.mit.edu -
Eric Niebler -
Giovanni Piero Deretta -
Joel -
Martin Bonner -
Michael Marcin -
Paul A Bristow -
Steven Watanabe