
AMDG I have not included the doxygen reference in this as I'll get to that along with the code. General: - missing .gitattributes introduction.html: - "...expression syntax that ... optimizes your expressions. " The syntax doesn't optimize. It's the Eigen implementation that optimizes. - Please use the correct syntax for bulleted lists. It's '*' not '-' in quickbook. http://www.boost.org/doc/html/quickbook/syntax/block.html#quickbook.syntax.b... header_organization.html: - I think it's better to put the actual name of the header as the primary text since that's what I actually need to know to #include it. tutorial/expressions.html: - "...any of the functions in Boost.YAP that takes" s/takes/take/ - "And if we evaluate it using..." And don't begin a sentence with a conjunction. - "An expression containing an expression<> subexpression and subexpressions instantiated from N other ExpressionTemplates can be passed as an argument to any of the Expression-accepting Boost.YAP functions." This sentence is hard to read because it uses "expression" too many times in too many different ways. - "there are lots of examples of how to do this in the Examples section" A link on "Examples" would be nice. tutorial/kinds_of_exceptions.html: - "kinds of expression" s/expression/expressions/ - "represents the leaf-node in an expression tree" I think "a leaf-node" would be more correct as an expression tree may have more than one leaf. - "An if_else expression is analogous to the C++ ternary operator (?:), but without the common-type conversion logic." So does this mean that such conversions are represented explicitly or are they simply not allowed? tutorial/operators.html: - BOOST_YAP_USER_BINARY_OPERATOR_MEMBER(plus, ::lazy_vector_expr) I feel like it should be possible to avoid needing to specify the ExpressionTemplate. It should be possible to deduce the return type from `this`. Also, for binary operators, I'd really prefer to have a single macro that defines both expr + x and x + expr, as defining both is usually what we want for symmetric operators. tutorial/explicit_transformations.html: - "result is created by visiting each node in expr, in top-down, breadth-first order" Is it really breadth-first? I would find that quite surprising as normal recursion creates a depth-first traversal. - "In other words, if returns any non-Boost.YAP argument..." s/if/it/ - "If situations when you want to..." s/If/In/ - "--" An mdash is \u2014 (or you can use UTF-8 for the qbk source, I think). - "evaluate() Is wrong for this task" s/Is/is/ tutorial/evaluating_expressions.html: - I don't think that BOOST_YAP_CONVERSION_OPERATOR_TEMPLATE is a good idea. Users can already define their own expression template types if the default behavior isn't good enough and your documentation encourages this anyway. A macro like this is just asking for accidental incompatibility. tutorial/operator_macros.html: - What about BOOST_YAP_USER_ANY_UDT_BINARY_OPERATOR? tutorial/how_expression_operands_are_treated.html: - "This implies that an rvalue remain treatable as a..." s/remain/remains/. Also this sentence is somewhat awkward. - T & - > PARTIAL_DECAY(T) & This can't possibly be right. If you decay a reference to a function or array into a pointer, then you need to treat the pointer as an rvalue. tutorial/customization_points.html: - what do you need evaluate_as for? I'm having a hard time thinking of uses that wouldn't be better handled as an explicit transform (Or perhaps just wrapping the expression in an implicit_cast_expr). The same goes for transform_expression. Having more knobs and dials just makes the API confusing. Also it looks to me like transform_expression will cause any placeholders to get dropped on the floor, as transform_expression doesn't take the extra arguments that would be used to replace placeholders. - transform_expression is the odd man out in terms of naming. I think it should be eval_something. You're already using the term transform as something else. tutorial/transform_matching.html: - The negate example: struct xform { // This transform negates terminals. auto operator() (boost::yap::terminal_tag, double x) { return -x; } // This transform removes negations. auto operator() (boost::yap::negate_tag, double x) { return x; } } seems somewhat confused. The behavior I would expect from this transform is that it can match expressions of the form `d` or `-d` and will return -d or d respectively. This implies that the correct behavior is not to apply the terminal_tag transform before accessing negate_tag. The later statement: // Only applies the negate_tag transform; prints -9. makes no sense as the negate_tag transform alone will give 9. - "The TagTransform is preferred, except in the case of a call expression, in which case the ExpressionTransform is preferred" Why is call treated specially? From an outsider's POV it just seems weird. At the very least it deserves an explanation in the rationale. - "The above only applies when you have a ExpressionTransform" s/a/an/ examples/hello_world.html: - I have no idea what you're talking about regarding eager evaluation in proto. The corresponding Proto example also uses an explicit evaluate. examples/hello_world_redux.html: - "That's better!" What's better than what? examples/calc1.html: - "Here we can first see how much C++14-and-later language features help the end user -- the Proto version is much, much longer." I disagree with this assertion. The real reason that this is shorter than the Proto version is that you've baked placeholder substitution into evaluate, which I think is a mistake--are you building a lambda library or are you building a generic ET library? examples/calc2.html: - This really misses the point of proto's calc2 example. An equivalent example with YAP would be to define a new ExpressionTemplate with an overloaded call operator. examples/calc3.html: - The way you're using get_arity is rather wacky as you know the arity statically anyway. The original Proto example makes more sense. examples/lazy_vector.html: - "This move is something of a hack." I'm not sure that I would consider it a hack. The general idea is that in evaluate(transform(expr)), any values held by expr should get moved into the result of transform so that they can be passed to evaluate. - "We're able to write this more simply than the equivalent Proto code; since YAP is always lazy, we can just use the expressions below directly." Actually the same code works for proto as well. I suspect that BOOST_PROTO_AUTO was just used for symmetry with expr1. examples/vector.html: - return boost::yap::make_terminal(std::move(vec[n])); Move is probably wrong as you're working with a reference to begin with. (Of course, it doesn't really matter for double, but imagine that you have a vector<unique_ptr> instead.) examples/mixed.html: - I think you'd be better off making sin_tag a regular function object like in the proto example. examples/map_list_of: - map.emplace( Key{std::forward<Key2 &&>(key)}, Value{std::forward<Value2 &&>(value)} ); Why create extra temporaries here? - return transform.map; This doesn't allow NRVO. examples/future_group.html: - "Assertion that left and right are compatible types" I don't think that is_same is the correct test. What you want to guarantee is that the return types are the same on both sides. manual/object_code.html: - "The object code generated by modern template metaprogramming can be quite good, in large part due to the drastic reduction in the amount of metaprogramming necessary to determine the result type" I'm not sure I see the connection here. Type-based metaprogramming generates no object code to begin with. In Christ, Steven Watanabe