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