Hi all, Following is my Review of the upcoming Boost.Local library. I will as others split my review into the three parts that make up this library: local functions, local blocks, scoped exit. Let me first start with some general comments. I personally find the decision to support two syntaxes not a good path to follow. While I understand the rationale behind it, and the desire to have the variadic macro version, having both might become a maintenance nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just stick with one and concentrate on the functionality? Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice. Local Functions: From the tutorial section on Local Functions: "Local functions are defined using the following macros (see the Reference section) from within a declarative context (this is a limitation with respect to C++11 lambda functions which can instead be declared within expressions):" Right, to have the lambdas in a "declarative context" use auto f = [...](...){...} The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter. I would consider binding variables from the enclosing scope one of the major weak points of Boost.Local. The need to binding those variables is obvious. However, how it is done clutters the arguments list of the BOOST_LOCAL_FUNCTION_PARAMS to an extend that it is not immediately clear what the arity of the local function is. It would make things clearer to have something like: BOOST_LOCAL_FUNCTION_PARAMS(...) BOOST_LOCAL_FUNCTION_CAPTURE(...) Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations. WRT to your note in the documentation that C++11 can not bind const reference. I suggest the following workaround: int i = 0; auto f = [&](){ auto const & icr = i; ... }; I agree that this and _this should be used always, instead of having both _this and this. On other notes, Lorenzo already clarified the defect in the library that it can't really be used as a closure. This would have been my next point. All in all the documentation is very accurate and in good shape. Additionally i played around with this specific part of the library a little bit in order to see how i can break it. TBH, I didn't (and still don't) trust the macros to do the right thing always, my knowledge of the PP isn't good enough to judge that particular part of the implementation. However, by deliberately trying to break the breaks (putting typos in the variables, omitting commas etc.) the error messages are of course very arcane, and you have to know exactly where and what part of the macros involved failed and why. Of course, these error message can be deciphered very easily once you have enough experience with that library. But i would argue that this is the case for any other (TMP based) library. Which brings me to my next point. I think that Boost.Local does not fulfill its own purpose. While it is of course true that errors in the function bodies are written in "usual C++" the surrounding macros are, IMHO, just cluttering the code. The points Lorenzo makes in the motivational section are not met. I would argue that most of the times, namespaces and classes are good enough, and such a macro syntax should be avoided. For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions. Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does). Local Exit: Why do we need two libraries doing the exact same thing with neither of the adding any functionality. I suggest to improve ScopedExit instead. In the alternatives section, which version of Phoenix do you use? Now to the usual questions:
- What is your evaluation of the design?
I think the design is not what can be considered Modern C++. However, in the context of what the author tried to achieve, he did a good job.
- What is your evaluation of the implementation?
The implementation looks solid!
- What is your evaluation of the documentation?
The documentation is very good!
- What is your evaluation of the potential usefulness of the library?
The potential usefulness of the library is out of question, that is why C++11 now has lambdas, and libraries like Bind, Lambda and Phoenix exist. However, I do not believe that Local adds any value to the existing solution of the described problems.
- Did you try to use the library? With what compiler? Did you have any problems?
Yes. gcc 4.6.2. no problems.
- How much effort did you put into your evaluation? A glance? A > quick reading? In-depth study?
Reading of the documentation. Looking briefly at the implementation.
- Are you knowledgeable about the problem domain?
Yes. I am the author of Phoenix V3. I vote to not include the library into boost. Regards, Thomas
Thanks for the review, Thomas! I just have a couple comments. On Fri, Nov 18, 2011 at 10:49 AM, Thomas Heller <thom.heller@googlemail.com>wrote: [...]
Let me first start with some general comments. I personally find the decision to support two syntaxes not a good path to follow. While I understand the rationale behind it, and the desire to have the variadic macro version, having both might become a maintenance nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just stick with one and concentrate on the functionality? Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice.
Ummm...was the double negative in the last sentence intentional? I'm guessing you only meant to include one or the other "not", not both. [...]
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I *believe* there has been an instance in my own code where I had to const_cast a range to coerce a range algorithm to use const_iterator's for efficiency reasons...or something like that. I don't remember the details of the situation, but I can imagine instances where const access to an object would be more efficient than mutable access, e.g., if mutable access via operator[] must return a proxy. It can be difficult to guarantee in general that the compiler would optimize a syntactically-mutable-semantically-const access into the equivalent syntactically-const access. - Jeff
On 11/18/2011 01:27 PM, Jeffrey Lee Hellrung, Jr. wrote:
Thanks for the review, Thomas! I just have a couple comments.
On Fri, Nov 18, 2011 at 10:49 AM, Thomas Heller <thom.heller@googlemail.com>wrote: [...]
Let me first start with some general comments. I personally find the decision to support two syntaxes not a good path to follow. While I understand the rationale behind it, and the desire to have the variadic macro version, having both might become a maintenance nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just stick with one and concentrate on the functionality? Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice.
Ummm...was the double negative in the last sentence intentional? I'm guessing you only meant to include one or the other "not", not both.
Yes, the last "not" was accidental.
[...]
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I *believe* there has been an instance in my own code where I had to const_cast a range to coerce a range algorithm to use const_iterator's for efficiency reasons...or something like that. I don't remember the details of the situation, but I can imagine instances where const access to an object would be more efficient than mutable access, e.g., if mutable access via operator[] must return a proxy. It can be difficult to guarantee in general that the compiler would optimize a syntactically-mutable-semantically-const access into the equivalent syntactically-const access.
I see. But why do you need local blocks to achieve that?
- Jeff
On Fri, Nov 18, 2011 at 12:02 PM, Thomas Heller <thom.heller@googlemail.com>wrote:
On 11/18/2011 01:27 PM, Jeffrey Lee Hellrung, Jr. wrote:
Thanks for the review, Thomas! I just have a couple comments.
On Fri, Nov 18, 2011 at 10:49 AM, Thomas Heller <thom.heller@googlemail.com>wrote: [...]
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I *believe* there has been an instance in my own code where I had to const_cast a range to coerce a range algorithm to use const_iterator's for efficiency reasons...or something like that. I don't remember the details of the situation, but I can imagine instances where const access to an object would be more efficient than mutable access, e.g., if mutable access via operator[] must return a proxy. It can be difficult to guarantee in general that the compiler would optimize a syntactically-mutable-**semantically-const access into the equivalent syntactically-const access.
I see. But why do you need local blocks to achieve that?
I don't. I could just const_cast use of the variable everywhere I want it const-qualified. Or define a reference-to-const to the same effect. ...or use a local block. You just sounded like you were angling for a legitimate use case for local blocks, and I tried to supply a plausible (if rare) one. - Jeff
On 11/18/2011 03:27 PM, Jeffrey Lee Hellrung, Jr. wrote:
On Fri, Nov 18, 2011 at 12:02 PM, Thomas Heller <thom.heller@googlemail.com>wrote:
On 11/18/2011 01:27 PM, Jeffrey Lee Hellrung, Jr. wrote:
Thanks for the review, Thomas! I just have a couple comments.
On Fri, Nov 18, 2011 at 10:49 AM, Thomas Heller <thom.heller@googlemail.com>wrote: [...]
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I *believe* there has been an instance in my own code where I had to const_cast a range to coerce a range algorithm to use const_iterator's for efficiency reasons...or something like that. I don't remember the details of the situation, but I can imagine instances where const access to an object would be more efficient than mutable access, e.g., if mutable access via operator[] must return a proxy. It can be difficult to guarantee in general that the compiler would optimize a syntactically-mutable-**semantically-const access into the equivalent syntactically-const access.
I see. But why do you need local blocks to achieve that?
I don't. I could just const_cast use of the variable everywhere I want it const-qualified. Or define a reference-to-const to the same effect. ...or use a local block.
You just sounded like you were angling for a legitimate use case for local blocks, and I tried to supply a plausible (if rare) one.
Right, and I am still thinking that having things like: { auto const & const_ref_to_object = ... } is preferrable (For C++03, replace it with the proper call to BOOST_AUTO or repeat the type). And if you really want a new, independent scope, why not just create a new function to make your intention even clearer. Can't think of a unique name for that? Hey, there are namespaces. IMHO, C++ already provides all necessary tools to express what local block does.
On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Hi all,
Hello Thomas and thank you very much for your review.
Following is my Review of the upcoming Boost.Local library. I will as others split my review into the three parts that make up this library: local functions, local blocks, scoped exit.
Let me first start with some general comments. I personally find the decision to support two syntaxes not a good path to follow. While I understand the rationale behind it, and the desire to have the variadic macro version, having both might become a maintenance
Why is that? From a user prospective, you just pick a syntax based on what your compiler can do, write your programs in such a syntax, and maintain that one syntax. From a library implementation prospective, supporting both syntax is trivial. There is a macro that converts variadic into a sequence up front and all the library is programmed using pp sequences (which are a better pp data structure than variadics as explained by Paul many times). So as a library maintainer you only see pp sequences after the 1st macro expansion and you just have to maintain pp-sequences-- no pp code is duplicated at all to support both syntaxes. Pseudo code: // VARIADIC_TO_SEQ(...): converts its argument from variadics to pp-seq (or it leaves it unchanged if it is already a seq). #if VARIADIC #deifne LOCAL_FUNCTION_PARAMS(...) LOCAL_AUX_FUNCTION_PARAMS_SEQ(VARIADIC_TO_SEQ(__VA_ARGS__)) #else #define LOCAL_FUNCTION_PARAMS(seq) LOCAL_AUX_FUNCTION_PARAMS_SEQ(seq) #endif Therefore, I'd conclude that there is no added maintenance cost or even implementation complexity in order to support both syntaxes. I can add this rationale to the Implementation section (currently it is no there so it might be natural to think that having both syntaxes comes to a high implementation cost even if it does not).
nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just
Which part of the documentation are you referring too? After both syntaxes are introduced: https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/index.html... The rest of the docs just explain the functionality. Even if I provide all examples in both syntaxes, I don't think it distracts the user-- do you? Can you point out the part of the documentation that are confusing for the user because of the "double" syntax? (Maybe I can improve them.)
stick with one and concentrate on the functionality?
Because having the sequencing syntax comes at no maintenance or implementation cost and it allows to use the library on compilers without variadic support.
Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this
Why Boost.Local does not solve this appropriately? What do you mean by "appropriately" in this context?
appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice.
I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code. IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter. -- from: http://lists.boost.org/Archives/boost/2011/02/176598.php -- On Thu, Feb 3, 2011 at 11:48 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 2/4/2011 12:29 PM, Gregory Crosswhite wrote:
Local should be viewed as being complimentary to Phoenix rather competing with it, and it should be evaluated on its own merits.
I think I have to agree with this. In as much as we have Boost-FOREACH in addition to std::for_each, Boost.Local is a good and useful library that is simple, practical and effective enough as it is. -- end --
-- from: http://lists.boost.org/Archives/boost/2011/02/176552.php -- On Thu, Feb 3, 2011 at 10:45 AM, Thomas Heller <thom.heller@googlemail.com> wrote:
http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/ht... A boilerplate macro for that would be a very welcome contribution... -- end --
So this boiler-plate-saving macros are a commonly accepted practice, why wouldn't they be appropriate for local functions?
Local Functions:
From the tutorial section on Local Functions: "Local functions are defined using the following macros (see the Reference section) from within a declarative context (this is a limitation with respect to C++11 lambda functions which can instead be declared within expressions):"
Right, to have the lambdas in a "declarative context" use auto f = [...](...){...}
I meant that this is a limitation of Boost.Local which cannot be used within expression (and not a limitation of C++11 lambdas). I will try to make this text more clear in the docs.
The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter.
Others have made the same comments (with you a total of 3 people). However, others have made the opposite comment (2 liked default, 5+ didn't say anything about it). In my experience, this quickly becomes a non-issue after the 1st week that you start using the library.
I would consider binding variables from the enclosing scope one of the major weak points of Boost.Local. The need to binding those variables is obvious. However, how it is done clutters the arguments list of the BOOST_LOCAL_FUNCTION_PARAMS to an extend that it is not immediately clear what the arity of the local function is. It would make things clearer to have something like: BOOST_LOCAL_FUNCTION_PARAMS(...) BOOST_LOCAL_FUNCTION_CAPTURE(...)
You are the first one to make this comment. I think that if you follow the convention to list the function parameters first and the binds last, you can easily see the function arity (at least that is my experience). The library does not enforce this order, but that is the convention I use in the docs. With that you look at the parameters from left to right until you find an parameter that said `bind` and there it is the function arity.
Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations.
I don't think so... here bind is clearly used in a different context.
WRT to your note in the documentation that C++11 can not bind const reference. I suggest the following workaround:
int i = 0; auto f = [&](){ auto const & icr = i; ... };
Yes, I will add that to the docs ad suggested by Vicente. This has the down side of (1) requiring to use a different name for the constant variabe `icr` and (2) write the extra code to declare the const variable. I agree that these are both minor points (but in some specific domains like Contract Programming however (1) is a big issue because you will have to explain the user that they need to use the magic name `icr` to refer to the variable in programming their assertions).
I agree that this and _this should be used always, instead of having both _this and this.
Yes, I will always use this_ (because it's a keyword in the context where it is used in Boost.Local). BTW, I understand that Phoenix uses _this, do you have a reference to the docs that discuss the use of _this? (I looked for it but I couldn't find it... I probably looked in the wrong place.)
On other notes, Lorenzo already clarified the defect in the library that it can't really be used as a closure. This would have been my next point.
What defect? I can fix the closure of values. The closure of reference suffers of the same limitations of C++11 lambdas and that's the accepted standard definition of closure for a stack based language like C++. http://en.wikipedia.org/wiki/Closure_(computer_science)#Implementation_and_t... Can you please clarify this point?
All in all the documentation is very accurate and in good shape. Additionally i played around with this specific part of the library a little bit in order to see how i can break it. TBH, I didn't (and still don't) trust the macros to do the right thing always, my knowledge of the PP isn't good enough to judge that particular part of the implementation. However, by deliberately trying to break the breaks (putting typos in the variables, omitting commas etc.) the error messages are of course very arcane, and you have to know exactly where and what part of the macros involved failed and why. Of course, these error message can be deciphered very easily once you have enough experience with that library. But i would argue that this is the case for any other (TMP based) library. Which brings me to my next point. I think that Boost.Local does not fulfill its own purpose. While it is of course true that errors in the function bodies are written in "usual C++" the surrounding macros are, IMHO, just
The function declaration is trivial and it is very easy to get it right (just inspecting it). If you make an error and you need the compiler's help to understand what you did wrong that is usually in the function definition. The function definition is outside the macros so you will get the usual compiler error support in programming the function correctly. I do find this true in my personal programming experience and others have made the same comment: -- from: http://lists.boost.org/Archives/boost/2011/02/176517.php -- On Wed, Feb 2, 2011 at 5:12 PM, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
On 02/02/2011 13:23, Thomas Heller wrote:
To quote the Boost.Local docs: "Warning Unfortunately, there are intrinsic limitations to the amount of syntactic error checking that can be done by the parsing macros implemented using the preprocessor. As a consequence, an error in using the parenthesized syntax might result in cryptic preprocessor errors. The best way to identify and correct these errors is usually to visually inspect the signature comparing it with the parenthesized syntax grammar (see the Grammar section). When syntactic errors can be detected by the parsing macros, they are raised at compile-time using error messages of the form ERROR_description_text."
Enumeration of parameters and bindings is not where code complexity is. --end quote--
cluttering the code. The points Lorenzo makes in the motivational section are not met. I would argue that most of the times, namespaces and classes
Which points are not met?
are good enough, and such a macro syntax should be avoided. For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions.
I don't see how Boost.Local prevents improving other Boost libraries-- a part from the fact that I did spend time in developing Boost.Local instead of improving the other libraries :). In fact we all concluded that Boost.Local is complimentary to Boost.Phoenix, Boost.Lambda, and Boost.Bind in that it provided statement syntax for the function definition which these other existing Boost library do not (they provide expression syntax for the function definition): http://lists.boost.org/Archives/boost/2011/02/176453.php http://lists.boost.org/Archives/boost/2011/02/176589.php To be honest, if we didn't conclude upfront on the complementarity of Boost.Local with respect to other existing Boost libraries, I wouldn't even spend the time for the submission-- why would I submit a redundant library to Boost?
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I am sure there are lots of people that will never use a local blocks. That said, I personally needed to program (to ensure the constant-correctness requirement of assertions in Contract Programming): int x = 0; const { assert( x == 0 ); } You can do that with local blocks: int x = 0; BOOST_LOCAL_BLOCK(const bind x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END As we said, using lambdas or other Boost libraries there are other ways to do that but in my case I needed to keep the `assertion( x == 0 )`;` unchanged (including the variable name `x`) and these other ways (including C++11 lambdas) don't allow for that. More in general, local blocks allow you to specify the sematic that a variable should be thread as const by the next bunch of instruction. I personally find that useful, others have made the same comment (Vicente, Pierre, Jeff, and another couple of people-- see their review to Boost.Local).
Local Exit: Why do we need two libraries doing the exact same thing with neither of the adding any functionality. I suggest to improve ScopedExit instead.
I agree. The ScopeExit improvements alone are not sufficient to motivate Boost.Local (I'd just add const bindings and this binding to ScopeExit for that). Boost.Local is about local functions, the improved scope exits (i.e., local exits) should be somehow merged with the existing ScopeExit (in fact that's a question to the reviewers).
In the alternatives section, which version of Phoenix do you use?
I can't double check now but I am pretty sure was the one from Boost 1.47 (I will add this information to the Alternatives section). As I mentioned in the Alternatives section, compilation-time was not an issue for me after I started to #include files selectively (which is very reasonable) also I was very impressed by the short run-times when compiler optimization is enabled (good job!).
Now to the usual questions:
- What is your evaluation of the design?
I think the design is not what can be considered Modern C++. However, in the
What do you mean by "modern C++"?
context of what the author tried to achieve, he did a good job.
- What is your evaluation of the implementation?
The implementation looks solid!
- What is your evaluation of the documentation?
The documentation is very good!
- What is your evaluation of the potential usefulness of the library?
The potential usefulness of the library is out of question, that is why C++11 now has lambdas, and libraries like Bind, Lambda and Phoenix exist. However, I do not believe that Local adds any value to the existing solution of the described problems.
I disagree. Boost.Local adds value because it allows you to use statement syntax to program the function body (and that is why Boost.Local is complimentary to these other Boost libraries): https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca...
- Did you try to use the library? With what compiler? Did you have > any problems?
Yes. gcc 4.6.2. no problems.
- How much effort did you put into your evaluation? A glance? A > quick reading? In-depth study?
Reading of the documentation. Looking briefly at the implementation.
- Are you knowledgeable about the problem domain?
Yes. I am the author of Phoenix V3.
I vote to not include the library into boost.
Given that you have made several comments (which I sincerely appreciate), could you please summarize what are the main reasons for voting "no"? Thanks a lot for your review! --Lorenzo
On 11/18/2011 04:44 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Hi all,
Hello Thomas and thank you very much for your review.
Following is my Review of the upcoming Boost.Local library. I will as others split my review into the three parts that make up this library: local functions, local blocks, scoped exit.
Let me first start with some general comments. I personally find the decision to support two syntaxes not a good path to follow. While I understand the rationale behind it, and the desire to have the variadic macro version, having both might become a maintenance
Why is that? From a user prospective, you just pick a syntax based on what your compiler can do, write your programs in such a syntax, and maintain that one syntax. From a library implementation prospective, supporting both syntax is trivial. There is a macro that converts variadic into a sequence up front and all the library is programmed using pp sequences (which are a better pp data structure than variadics as explained by Paul many times). So as a library maintainer you only see pp sequences after the 1st macro expansion and you just have to maintain pp-sequences-- no pp code is duplicated at all to support both syntaxes. Pseudo code:
// VARIADIC_TO_SEQ(...): converts its argument from variadics to pp-seq (or it leaves it unchanged if it is already a seq).
#if VARIADIC
#deifne LOCAL_FUNCTION_PARAMS(...) LOCAL_AUX_FUNCTION_PARAMS_SEQ(VARIADIC_TO_SEQ(__VA_ARGS__))
#else
#define LOCAL_FUNCTION_PARAMS(seq) LOCAL_AUX_FUNCTION_PARAMS_SEQ(seq)
#endif
Therefore, I'd conclude that there is no added maintenance cost or even implementation complexity in order to support both syntaxes. I can add this rationale to the Implementation section (currently it is no there so it might be natural to think that having both syntaxes comes to a high implementation cost even if it does not).
I see, makes sense.
nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just
Which part of the documentation are you referring too? After both syntaxes are introduced:
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/index.html...
Every section has at least one example and the syntax box showing both. https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... Has one paragraph about the differences. https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... Is basically all about the different syntax possibilities. The rest only has examples and an optional syntax box showing both. The corresponding reference sections, obviously have the same.
The rest of the docs just explain the functionality. Even if I provide all examples in both syntaxes, I don't think it distracts the user-- do you? Can you point out the part of the documentation that are confusing for the user because of the "double" syntax? (Maybe I can improve them.)
stick with one and concentrate on the functionality?
Because having the sequencing syntax comes at no maintenance or implementation cost and it allows to use the library on compilers without variadic support.
It is not that it is confusing, but why do you need two around? Isn't one enough? What about if you decide to change/add something to the syntax, you need to update anything else. From a user point of view, what are the advantages/disadvantages over one or the others? This is all not clear, if I would have a been a user i would clearly choose the sequencing syntax, cause it is portable (over compilers). So, why do you need to clutter the docs with having to explain the two syntaxes?
Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this
Why Boost.Local does not solve this appropriately? What do you mean by "appropriately" in this context?
It tries do make it easier for users to define local functions in order to make their lifes easier so they don't have to clutter their code with local functions. This is what i fail to see: 1) The macros do not even closely resemble any kind of function declarations in C++ 2) If you just want to write a simple local function, the epi- and prologue is longer than the actual function body (this is true for almost all your examples). And once the function body exceeds that limit I wonder why it should have been a local function to begin with.
appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice.
I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
What kind of boilerplate code? Defining a global function? Most likely no. Defining a local functor? Yeah, probably, but do we really need that? Defining a global functor? This is not possible with Local, so the answer would be "no" here.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
Well, I didn't say that I think ScopeExit is good. In fact, I never saw the need to use it. As another reviewer noted, i'd rather encapsulate that in a class to logically divide that from the actual code. For ForEach the situation is completely different. Apart from having it to spell all caps, it looks exactly like a range based for each loop should look like. Boost.Local's Macros (yes, plural!) however, have nothing in common with the way you declare functions in C++.
-- from: http://lists.boost.org/Archives/boost/2011/02/176598.php -- On Thu, Feb 3, 2011 at 11:48 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 2/4/2011 12:29 PM, Gregory Crosswhite wrote:
Local should be viewed as being complimentary to Phoenix rather competing with it, and it should be evaluated on its own merits.
I think I have to agree with this. In as much as we have Boost-FOREACH in addition to std::for_each, Boost.Local is a good and useful library that is simple, practical and effective enough as it is. -- end --
See above.
-- from: http://lists.boost.org/Archives/boost/2011/02/176552.php -- On Thu, Feb 3, 2011 at 10:45 AM, Thomas Heller <thom.heller@googlemail.com> wrote:
http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/ht... A boilerplate macro for that would be a very welcome contribution... -- end --
So this boiler-plate-saving macros are a commonly accepted practice, why wouldn't they be appropriate for local functions?
Because Local Functions sole API are macros.
Local Functions:
From the tutorial section on Local Functions: "Local functions are defined using the following macros (see the Reference section) from within a declarative context (this is a limitation with respect to C++11 lambda functions which can instead be declared within expressions):"
Right, to have the lambdas in a "declarative context" use auto f = [...](...){...}
I meant that this is a limitation of Boost.Local which cannot be used within expression (and not a limitation of C++11 lambdas). I will try to make this text more clear in the docs.
The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter.
Others have made the same comments (with you a total of 3 people). However, others have made the opposite comment (2 liked default, 5+ didn't say anything about it). In my experience, this quickly becomes a non-issue after the 1st week that you start using the library.
Right, i have to learn a new syntax in order to learn how to define local functions. I note this in order to support the argument that Boost.Local does not solve the issues discussed above.
I would consider binding variables from the enclosing scope one of the major weak points of Boost.Local. The need to binding those variables is obvious. However, how it is done clutters the arguments list of the BOOST_LOCAL_FUNCTION_PARAMS to an extend that it is not immediately clear what the arity of the local function is. It would make things clearer to have something like: BOOST_LOCAL_FUNCTION_PARAMS(...) BOOST_LOCAL_FUNCTION_CAPTURE(...)
You are the first one to make this comment. I think that if you follow the convention to list the function parameters first and the binds last, you can easily see the function arity (at least that is my experience). The library does not enforce this order, but that is the convention I use in the docs. With that you look at the parameters from left to right until you find an parameter that said `bind` and there it is the function arity.
This is not something i would expect from a library that should make it easy to write local functions.
Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations.
I don't think so... here bind is clearly used in a different context.
Yes that is what i meant. It is a overloading of terms. <snip>
BTW, I understand that Phoenix uses _this, do you have a reference to the docs that discuss the use of _this? (I looked for it but I couldn't find it... I probably looked in the wrong place.)
Nope it hasn't been documented because there is no real use for that yet.
On other notes, Lorenzo already clarified the defect in the library that it can't really be used as a closure. This would have been my next point.
What defect? I can fix the closure of values. The closure of reference suffers of the same limitations of C++11 lambdas and that's the accepted standard definition of closure for a stack based language like C++. http://en.wikipedia.org/wiki/Closure_(computer_science)#Implementation_and_t...
Can you please clarify this point?
Yeah, this is exactly what i meant.
All in all the documentation is very accurate and in good shape. Additionally i played around with this specific part of the library a little bit in order to see how i can break it. TBH, I didn't (and still don't) trust the macros to do the right thing always, my knowledge of the PP isn't good enough to judge that particular part of the implementation. However, by deliberately trying to break the breaks (putting typos in the variables, omitting commas etc.) the error messages are of course very arcane, and you have to know exactly where and what part of the macros involved failed and why. Of course, these error message can be deciphered very easily once you have enough experience with that library. But i would argue that this is the case for any other (TMP based) library. Which brings me to my next point. I think that Boost.Local does not fulfill its own purpose. While it is of course true that errors in the function bodies are written in "usual C++" the surrounding macros are, IMHO, just
The function declaration is trivial and it is very easy to get it right (just inspecting it). If you make an error and you need the compiler's help to understand what you did wrong that is usually in the function definition. The function definition is outside the macros so you will get the usual compiler error support in programming the function correctly.
Yes that is correct. Nevertheless I don't find the Boost.Local in any way "natural"
I do find this true in my personal programming experience and others have made the same comment: -- from: http://lists.boost.org/Archives/boost/2011/02/176517.php -- On Wed, Feb 2, 2011 at 5:12 PM, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
On 02/02/2011 13:23, Thomas Heller wrote:
To quote the Boost.Local docs: "Warning Unfortunately, there are intrinsic limitations to the amount of syntactic error checking that can be done by the parsing macros implemented using the preprocessor. As a consequence, an error in using the parenthesized syntax might result in cryptic preprocessor errors. The best way to identify and correct these errors is usually to visually inspect the signature comparing it with the parenthesized syntax grammar (see the Grammar section). When syntactic errors can be detected by the parsing macros, they are raised at compile-time using error messages of the form ERROR_description_text."
Enumeration of parameters and bindings is not where code complexity is. --end quote--
I agree. This doesn't change the fact, that most of the times when you want local functions, you want to have them for only a few lines of code. This changes the complexity completely ... because then most of the code you have to write for that local function is boilerplate to avoid boilerplate. (So i would ask myself ... why didn't i just write that global function myself real quick?)
cluttering the code. The points Lorenzo makes in the motivational section are not met. I would argue that most of the times, namespaces and classes
Which points are not met?
See above.
are good enough, and such a macro syntax should be avoided. For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions.
I don't see how Boost.Local prevents improving other Boost libraries-- a part from the fact that I did spend time in developing Boost.Local instead of improving the other libraries :).
In fact we all concluded that Boost.Local is complimentary to Boost.Phoenix, Boost.Lambda, and Boost.Bind in that it provided statement syntax for the function definition which these other existing Boost library do not (they provide expression syntax for the function definition): http://lists.boost.org/Archives/boost/2011/02/176453.php http://lists.boost.org/Archives/boost/2011/02/176589.php To be honest, if we didn't conclude upfront on the complementarity of Boost.Local with respect to other existing Boost libraries, I wouldn't even spend the time for the submission-- why would I submit a redundant library to Boost?
Well, at least it doesn't add anything to any of the existing boost libraries. While it obviously does a completely different thing than the other three mentioned. Sorry to say it so bluntly, doesn't mean that it does a good job in doing so. Since you like quoting from these old threads. I wasn't a fan of the syntax back then either.
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I am sure there are lots of people that will never use a local blocks. That said, I personally needed to program (to ensure the constant-correctness requirement of assertions in Contract Programming):
int x = 0; const { assert( x == 0 ); }
You can do that with local blocks:
int x = 0; BOOST_LOCAL_BLOCK(const bind x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END
As we said, using lambdas or other Boost libraries there are other ways to do that but in my case I needed to keep the `assertion( x == 0 )`;` unchanged (including the variable name `x`) and these other ways (including C++11 lambdas) don't allow for that.
More in general, local blocks allow you to specify the sematic that a variable should be thread as const by the next bunch of instruction. I personally find that useful, others have made the same comment (Vicente, Pierre, Jeff, and another couple of people-- see their review to Boost.Local).
Sorry, I just don't get it ... why does it need to be surrounded by a macro? <snip>
Now to the usual questions:
- What is your evaluation of the design?
I think the design is not what can be considered Modern C++. However, in the
What do you mean by "modern C++"?
context of what the author tried to achieve, he did a good job.
- What is your evaluation of the implementation?
The implementation looks solid!
- What is your evaluation of the documentation?
The documentation is very good!
- What is your evaluation of the potential usefulness of the library?
The potential usefulness of the library is out of question, that is why C++11 now has lambdas, and libraries like Bind, Lambda and Phoenix exist. However, I do not believe that Local adds any value to the existing solution of the described problems.
I disagree. Boost.Local adds value because it allows you to use statement syntax to program the function body (and that is why Boost.Local is complimentary to these other Boost libraries): https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca...
I can't fully appreciate the value here. Not because of other existing libraries, but because normal functions are way more expressive and clear than these macros can ever be.
- Did you try to use the library? With what compiler? Did you have> any problems?
Yes. gcc 4.6.2. no problems.
- How much effort did you put into your evaluation? A glance? A> quick reading? In-depth study?
Reading of the documentation. Looking briefly at the implementation.
- Are you knowledgeable about the problem domain?
Yes. I am the author of Phoenix V3.
I vote to not include the library into boost.
Given that you have made several comments (which I sincerely appreciate), could you please summarize what are the main reasons for voting "no"?
One of the biggest points is certainly the syntax. The other thing is that I don't like the idea of advertising the heavy use of macros in application developer codes to interface a boost library.
On Fri, Nov 18, 2011 at 8:35 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/18/2011 04:44 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Hi all,
Hello Thomas and thank you very much for your review.
Following is my Review of the upcoming Boost.Local library. I will as others split my review into the three parts that make up this library: local functions, local blocks, scoped exit.
Let me first start with some general comments. I personally find the decision to support two syntaxes not a good path to follow. While I understand the rationale behind it, and the desire to have the variadic macro version, having both might become a maintenance
Why is that? From a user prospective, you just pick a syntax based on what your compiler can do, write your programs in such a syntax, and maintain that one syntax. From a library implementation prospective, supporting both syntax is trivial. There is a macro that converts variadic into a sequence up front and all the library is programmed using pp sequences (which are a better pp data structure than variadics as explained by Paul many times). So as a library maintainer you only see pp sequences after the 1st macro expansion and you just have to maintain pp-sequences-- no pp code is duplicated at all to support both syntaxes. Pseudo code:
// VARIADIC_TO_SEQ(...): converts its argument from variadics to pp-seq (or it leaves it unchanged if it is already a seq).
#if VARIADIC
#deifne LOCAL_FUNCTION_PARAMS(...) LOCAL_AUX_FUNCTION_PARAMS_SEQ(VARIADIC_TO_SEQ(__VA_ARGS__))
#else
#define LOCAL_FUNCTION_PARAMS(seq) LOCAL_AUX_FUNCTION_PARAMS_SEQ(seq)
#endif
Therefore, I'd conclude that there is no added maintenance cost or even implementation complexity in order to support both syntaxes. I can add this rationale to the Implementation section (currently it is no there so it might be natural to think that having both syntaxes comes to a high implementation cost even if it does not).
I see, makes sense.
nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just
Which part of the documentation are you referring too? After both syntaxes are introduced:
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/index.html...
Every section has at least one example and the syntax box showing both. https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... Has one paragraph about the differences. https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... Is basically all about the different syntax possibilities. The rest only has examples and an optional syntax box showing both. The corresponding reference sections, obviously have the same.
Vicente was the only other reviewer to mention to present the sequencing syntax as a workaround instead that trough the entire docs. If this was a real issue, the documentation could be re-written that way: With just the variadics syntax and mentioning the sequencing syntax in one place only for compilers without variadics. However, all others have not mention that the two syntaxes make the docs confusing.
The rest of the docs just explain the functionality. Even if I provide all examples in both syntaxes, I don't think it distracts the user-- do you? Can you point out the part of the documentation that are confusing for the user because of the "double" syntax? (Maybe I can improve them.)
stick with one and concentrate on the functionality?
Because having the sequencing syntax comes at no maintenance or implementation cost and it allows to use the library on compilers without variadic support.
It is not that it is confusing, but why do you need two around? Isn't one
Because users that have variadic compilers might prefer to use , instead of ().
enough? What about if you decide to change/add something to the syntax, you need to update anything else.
No because the only difference between the syntaxes are the () instead of the , and the VARIADIC_TO_SEQ replaces , to () for you. You don't have to update "everything else" because the syntaxes are fully equivalent by definition (it's just about using commas instead of parenthesis as token separators).
From a user point of view, what are the advantages/disadvantages over one or the others?
If a user likes to use , instead of () as separators and has a variadic compiler, he/she can use the variaidc syntax (familiar syntax advantage). If a user does not have a variadic compiler, he/she can use the sequencing sytnax (portability advantage).
This is all not clear, if I would have a been a user i would clearly choose the sequencing syntax, cause it is portable (over compilers). So, why do you need to clutter the docs with having to explain the two syntaxes?
I tried to explain that here "The use of the variadic macro syntax might lead to more readable code however it should only be used when programmers know that their code will always be compiled with variadic macros support to avoid portability issues." https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/index.html... Is this not clear? Shall I try to re-word that text?
Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this
Why Boost.Local does not solve this appropriately? What do you mean by "appropriately" in this context?
It tries do make it easier for users to define local functions in order to make their lifes easier so they don't have to clutter their code with local functions. This is what i fail to see:
Sorry, I don't understand this sentence. Can you please re-word it?
1) The macros do not even closely resemble any kind of function declarations in C++
Many have expressed different opinion appreciating instead the syntax. For example (but also other email thread on Boost and reviews): -- from: http://lists.boost.org/Archives/boost/2011/02/176638.php -- On Fri, Feb 4, 2011 at 5:43 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 2/5/11 6:03 AM, Alexander Nasonov wrote:
As far is a am concerned, I still find this syntax overly verbose. I do realize though that this is just a toy example. For better comparison, Here is how the same thing would like in Boost.Phoenix:
Steven and I were playing with different syntaxes few years ago
We came up with something like this:
void BOOST_LOCAL_FUNCTION ( BOOST_BIND((factor)(&sum)), double num ) { sum += factor * num; std::clog<< "Summed:"<< sum<< std::endl; } BOOST_LOCAL_FUNCTION_DECL(add)
http://thread.gmane.org/gmane.comp.lib.boost.devel/168612/focus=168694
That is nice! -- end --
That said, I understand how this is clearly a subjective point and I respect your opinion.
2) If you just want to write a simple local function, the epi- and prologue is longer than the actual function body (this is true for almost all your
A part form the two macro names, you don't have more tokens than the result type, parameter declarations, bind declarations, and local function name. IMO, these are all elements that syntactically should be part of the local function declaration regardless of the length of the body because they describe the local function interface and its bindings.
examples). And once the function body exceeds that limit I wonder why it should have been a local function to begin with.
Others have mentioned that it is fine for them to have long functions and local functions especially in FP: -- from: http://lists.boost.org/Archives/boost/2011/02/176653.php --
In functional programming, there is no difference between defining lambdas and functions (except the former cannot be recursive), and it is not viewed as bad practice to have functions spanning a couple hundred lines. -- end --
But of course I understand if you prefer to structure your code differently.
appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice.
I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
What kind of boilerplate code? Defining a global function? Most likely no. Defining a local functor? Yeah, probably, but do we really need that?
Yes, the boiler-plate code to define the local functor. And yes, you need that to implement the local function... Sorry but I'm missing something here... Can you please explain what you mean better with the two question marks above?
Defining a global functor? This is not possible with Local, so the answer would be "no" here.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
Well, I didn't say that I think ScopeExit is good. In fact, I never saw the
Can you please clarify? Would you make to ScopeExit the same criticisms you are making to Boost.Local syntax and its macro usage or not? // (1)
need to use it. As another reviewer noted, i'd rather encapsulate that in a class to logically divide that from the actual code. For ForEach the situation is completely different. Apart from having it to spell all caps, it looks exactly like a range based for each loop should look like. Boost.Local's Macros (yes, plural!) however, have nothing in common with the way you declare functions in C++.
Again, I understand that you personally view the syntax as not natural. (I am honestly sorry to hear that because I truly spent a lot of effort in trying to make the syntax as natural as possible based on everyone's input but I don't pretend to be able to make everyone happy so it's ok.)
-- from: http://lists.boost.org/Archives/boost/2011/02/176598.php -- On Thu, Feb 3, 2011 at 11:48 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 2/4/2011 12:29 PM, Gregory Crosswhite wrote:
Local should be viewed as being complimentary to Phoenix rather competing with it, and it should be evaluated on its own merits.
I think I have to agree with this. In as much as we have Boost-FOREACH in addition to std::for_each, Boost.Local is a good and useful library that is simple, practical and effective enough as it is.
-- end --
See above.
-- from: http://lists.boost.org/Archives/boost/2011/02/176552.php -- On Thu, Feb 3, 2011 at 10:45 AM, Thomas Heller <thom.heller@googlemail.com> wrote:
http://svn.boost.org/svn/boost/sandbox/SOC/2010/phoenix3/libs/phoenix/doc/ht... A boilerplate macro for that would be a very welcome contribution...
-- end --
So this boiler-plate-saving macros are a commonly accepted practice, why wouldn't they be appropriate for local functions?
Because Local Functions sole API are macros.
Assuming that it's bad to provide macros as part of the API, why it becomes OK if there are 3 functions and 1 macro in the API? The 1 macro will still suffer from the macro issues... how would the 3 normal functions be able to mitigate the issues of the 1 macro? Sorry but I'm not understanding your argument here... Can you please explain it more? Plus the same is true for other libraries like ScopeExit. But answering (1) should answer this too :)
Local Functions:
From the tutorial section on Local Functions: "Local functions are defined using the following macros (see the Reference section) from within a declarative context (this is a limitation with respect to C++11 lambda functions which can instead be declared within expressions):"
Right, to have the lambdas in a "declarative context" use auto f = [...](...){...}
I meant that this is a limitation of Boost.Local which cannot be used within expression (and not a limitation of C++11 lambdas). I will try to make this text more clear in the docs.
The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter.
Others have made the same comments (with you a total of 3 people). However, others have made the opposite comment (2 liked default, 5+ didn't say anything about it). In my experience, this quickly becomes a non-issue after the 1st week that you start using the library.
Right, i have to learn a new syntax in order to learn how to define local
That is true for Boost.Lambda, Boost.Phenix, and even for C++11 (because most programmers are now only familiar with C++03). All these tools require you to learn a new syntax as Boost.Local does. So what is your point?
functions. I note this in order to support the argument that Boost.Local does not solve the issues discussed above.
Let me clarify that Boost.Local goal is to support local functions with bindings and statement syntax for the function definition. Boost.Local never tried to solve this problem without introducing a new syntax for the function declaration (in fact earlier version of the library had an even more exotic syntax for declaring the local function).
I would consider binding variables from the enclosing scope one of the major weak points of Boost.Local. The need to binding those variables is obvious. However, how it is done clutters the arguments list of the BOOST_LOCAL_FUNCTION_PARAMS to an extend that it is not immediately clear what the arity of the local function is. It would make things clearer to have something like: BOOST_LOCAL_FUNCTION_PARAMS(...) BOOST_LOCAL_FUNCTION_CAPTURE(...)
You are the first one to make this comment. I think that if you follow the convention to list the function parameters first and the binds last, you can easily see the function arity (at least that is my experience). The library does not enforce this order, but that is the convention I use in the docs. With that you look at the parameters from left to right until you find an parameter that said `bind` and there it is the function arity.
This is not something i would expect from a library that should make it easy to write local functions.
IMO and experience ordering the parameters this way is trivial but again I can see how this is subjective so I respect your opinion.
Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations.
I don't think so... here bind is clearly used in a different context.
Yes that is what i meant. It is a overloading of terms.
Why would it be confusing? You spent a few hours reading the docs, do you honestly find it confusing? If so, do you think that a name different than bind should be used? (I can use any name we can think of and agree on.) For example, would "capture" be more clear: int x = 0; void BOOST_LOCAL_FUNCTION_PARAMS(const capture& x) { ... // some long set of instruction ;) } BOOST_LOCAL_FUNCTION_NAME(f) f();
<snip>
BTW, I understand that Phoenix uses _this, do you have a reference to the docs that discuss the use of _this? (I looked for it but I couldn't find it... I probably looked in the wrong place.)
Nope it hasn't been documented because there is no real use for that yet.
OK, thanks.
On other notes, Lorenzo already clarified the defect in the library that it can't really be used as a closure. This would have been my next point.
What defect? I can fix the closure of values. The closure of reference suffers of the same limitations of C++11 lambdas and that's the accepted standard definition of closure for a stack based language like C++.
http://en.wikipedia.org/wiki/Closure_(computer_science)#Implementation_and_t...
Can you please clarify this point?
Yeah, this is exactly what i meant.
But that is OK because a closure is defined as a function plus its environment (so local functions fully follow within this definition). The persistence of the environment outside the scope of definition of the closure is a different issue and (as for C++11 lambdas) for C++ it is accepted that references will no longer be valid outside their scope of definition even if they are part of the closure. I can implement that so Boost.Local can be used for closures.
All in all the documentation is very accurate and in good shape. Additionally i played around with this specific part of the library a little bit in order to see how i can break it. TBH, I didn't (and still don't) trust the macros to do the right thing always, my knowledge of the PP isn't good enough to judge that particular part of the implementation. However, by deliberately trying to break the breaks (putting typos in the variables, omitting commas etc.) the error messages are of course very arcane, and you have to know exactly where and what part of the macros involved failed and why. Of course, these error message can be deciphered very easily once you have enough experience with that library. But i would argue that this is the case for any other (TMP based) library. Which brings me to my next point. I think that Boost.Local does not fulfill its own purpose. While it is of course true that errors in the function bodies are written in "usual C++" the surrounding macros are, IMHO, just
The function declaration is trivial and it is very easy to get it right (just inspecting it). If you make an error and you need the compiler's help to understand what you did wrong that is usually in the function definition. The function definition is outside the macros so you will get the usual compiler error support in programming the function correctly.
Yes that is correct. Nevertheless I don't find the Boost.Local in any way "natural"
Again, I understand and I think that's the main issue here (which unfortunately I cannot address).
I do find this true in my personal programming experience and others have made the same comment: -- from: http://lists.boost.org/Archives/boost/2011/02/176517.php -- On Wed, Feb 2, 2011 at 5:12 PM, Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
On 02/02/2011 13:23, Thomas Heller wrote:
To quote the Boost.Local docs: "Warning Unfortunately, there are intrinsic limitations to the amount of syntactic error checking that can be done by the parsing macros implemented using the preprocessor. As a consequence, an error in using the parenthesized syntax might result in cryptic preprocessor errors. The best way to identify and correct these errors is usually to visually inspect the signature comparing it with the parenthesized syntax grammar (see the Grammar section). When syntactic errors can be detected by the parsing macros, they are raised at compile-time using error messages of the form ERROR_description_text."
Enumeration of parameters and bindings is not where code complexity is.
--end quote--
I agree. This doesn't change the fact, that most of the times when you want local functions, you want to have them for only a few lines of code. This
As I mentioned before someone mentioned that not to be the case for them (because they write long functions and local functions). I understand you write short one-liner local functions most of the times for which Boost.Lambda or Boost.Phoenix is your tool of choice. However, others might do differently so why wouldn't we given them the choice to use statement syntax to program their local functions? Just because they need to use a couple of macros? I think such a choice should be left to the users of our libraries.
changes the complexity completely ... because then most of the code you have to write for that local function is boilerplate to avoid boilerplate. (So i would ask myself ... why didn't i just write that global function myself real quick?)
cluttering the code. The points Lorenzo makes in the motivational section are not met. I would argue that most of the times, namespaces and classes
Which points are not met?
See above.
Actually, I think this is summarized by your very last answer below (so see my reply there).
are good enough, and such a macro syntax should be avoided. For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions.
I don't see how Boost.Local prevents improving other Boost libraries-- a part from the fact that I did spend time in developing Boost.Local instead of improving the other libraries :).
In fact we all concluded that Boost.Local is complimentary to Boost.Phoenix, Boost.Lambda, and Boost.Bind in that it provided statement syntax for the function definition which these other existing Boost library do not (they provide expression syntax for the function definition): http://lists.boost.org/Archives/boost/2011/02/176453.php http://lists.boost.org/Archives/boost/2011/02/176589.php To be honest, if we didn't conclude upfront on the complementarity of Boost.Local with respect to other existing Boost libraries, I wouldn't even spend the time for the submission-- why would I submit a redundant library to Boost?
Well, at least it doesn't add anything to any of the existing boost libraries. While it obviously does a completely different thing than the other three mentioned. Sorry to say it so bluntly, doesn't mean that it does
Sorry, I don't understand this sentence. Can you please re-word that?
a good job in doing so. Since you like quoting from these old threads. I wasn't a fan of the syntax back then either.
Please allow me to clarify that I "like" to quote from these "old" threads for a very important reason: They express the point of view of my users upon which I designed my library. For me, these discussions we had back then on Boost.Local were not lost in time, I instead spent the last ~year reading them over and over again to make sure I captured all my user's requirements. I took these threads and the discussions we had very seriously-- maybe too seriously :)
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I am sure there are lots of people that will never use a local blocks. That said, I personally needed to program (to ensure the constant-correctness requirement of assertions in Contract Programming):
int x = 0; const { assert( x == 0 ); }
You can do that with local blocks:
int x = 0; BOOST_LOCAL_BLOCK(const bind x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END
As we said, using lambdas or other Boost libraries there are other ways to do that but in my case I needed to keep the `assertion( x == 0 )`;` unchanged (including the variable name `x`) and these other ways (including C++11 lambdas) don't allow for that.
More in general, local blocks allow you to specify the sematic that a variable should be thread as const by the next bunch of instruction. I personally find that useful, others have made the same comment (Vicente, Pierre, Jeff, and another couple of people-- see their review to Boost.Local).
Sorry, I just don't get it ... why does it need to be surrounded by a macro?
Because I could not find any other way to declare the local function using statement syntax for its definition without using macros. Does this make sense?
<snip>
Now to the usual questions:
- What is your evaluation of the design?
I think the design is not what can be considered Modern C++. However, in the
What do you mean by "modern C++"?
context of what the author tried to achieve, he did a good job.
- What is your evaluation of the implementation?
The implementation looks solid!
- What is your evaluation of the documentation?
The documentation is very good!
- What is your evaluation of the potential usefulness of the library?
The potential usefulness of the library is out of question, that is why C++11 now has lambdas, and libraries like Bind, Lambda and Phoenix exist. However, I do not believe that Local adds any value to the existing solution of the described problems.
I disagree. Boost.Local adds value because it allows you to use statement syntax to program the function body (and that is why Boost.Local is complimentary to these other Boost libraries):
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca...
I can't fully appreciate the value here. Not because of other existing libraries, but because normal functions are way more expressive and clear than these macros can ever be.
But "normal" functions cannot be declared at local scope so they are not sufficient for the problem domain of this library.
- Did you try to use the library? With what compiler? Did you have> any problems?
Yes. gcc 4.6.2. no problems.
- How much effort did you put into your evaluation? A glance? A> quick reading? In-depth study?
Reading of the documentation. Looking briefly at the implementation.
- Are you knowledgeable about the problem domain?
Yes. I am the author of Phoenix V3.
I vote to not include the library into boost.
Given that you have made several comments (which I sincerely appreciate), could you please summarize what are the main reasons for voting "no"?
One of the biggest points is certainly the syntax. The other thing is that I
I understand you don't like the syntax (and I truly think this a good enough reason for you to reject and never use the library). As I said before I am sorry because I really tried to make the syntax as natural as possible but I cannot pretend to be able to satisfy everyone.
don't like the idea of advertising the heavy use of macros in application developer codes to interface a boost library.
Again, there are other Boost libraries that do the same (e.g., Boost.ScopeExit) so why should we threat Boost.Local differently? But answering (1) should answer this too. Thank you very much for the clarifications. --Lorenzo
On 11/18/2011 09:37 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 8:35 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/18/2011 04:44 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this
Why Boost.Local does not solve this appropriately? What do you mean by "appropriately" in this context?
It tries do make it easier for users to define local functions in order to make their lifes easier so they don't have to clutter their code with local functions. This is what i fail to see:
Sorry, I don't understand this sentence. Can you please re-word it?
1) The macros do not even closely resemble any kind of function declarations in C++
Many have expressed different opinion appreciating instead the syntax. For example (but also other email thread on Boost and reviews):
That said, I understand how this is clearly a subjective point and I respect your opinion.
2) If you just want to write a simple local function, the epi- and prologue is longer than the actual function body (this is true for almost all your
A part form the two macro names, you don't have more tokens than the result type, parameter declarations, bind declarations, and local function name. IMO, these are all elements that syntactically should be part of the local function declaration regardless of the length of the body because they describe the local function interface and its bindings.
examples). And once the function body exceeds that limit I wonder why it should have been a local function to begin with.
Others have mentioned that it is fine for them to have long functions and local functions especially in FP:
-- from: http://lists.boost.org/Archives/boost/2011/02/176653.php --
In functional programming, there is no difference between defining lambdas and functions (except the former cannot be recursive), and it is not viewed as bad practice to have functions spanning a couple hundred lines. -- end --
But of course I understand if you prefer to structure your code differently.
appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice.
I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
What kind of boilerplate code? Defining a global function? Most likely no. Defining a local functor? Yeah, probably, but do we really need that?
Yes, the boiler-plate code to define the local functor. And yes, you need that to implement the local function... Sorry but I'm missing something here... Can you please explain what you mean better with the two question marks above?
The question is about what I my point of criticism is all about: Why do we need a macro to define a function, that adds no more true value other than that it was defined in function scope?
Defining a global functor? This is not possible with Local, so the answer would be "no" here.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
Well, I didn't say that I think ScopeExit is good. In fact, I never saw the
Can you please clarify? Would you make to ScopeExit the same criticisms you are making to Boost.Local syntax and its macro usage or not? // (1)
Yes, I would make the criticism to ScopeExit.
need to use it. As another reviewer noted, i'd rather encapsulate that in a class to logically divide that from the actual code. For ForEach the situation is completely different. Apart from having it to spell all caps, it looks exactly like a range based for each loop should look like. Boost.Local's Macros (yes, plural!) however, have nothing in common with the way you declare functions in C++.
Again, I understand that you personally view the syntax as not natural. (I am honestly sorry to hear that because I truly spent a lot of effort in trying to make the syntax as natural as possible based on everyone's input but I don't pretend to be able to make everyone happy so it's ok.)
Look, I am not trying to make you feel bad. I know that you put a lot of effort into making this library. If you remember, I wasn't happy about it from the start on, and i said so. As did others.
The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter.
Others have made the same comments (with you a total of 3 people). However, others have made the opposite comment (2 liked default, 5+ didn't say anything about it). In my experience, this quickly becomes a non-issue after the 1st week that you start using the library.
Right, i have to learn a new syntax in order to learn how to define local
That is true for Boost.Lambda, Boost.Phenix, and even for C++11 (because most programmers are now only familiar with C++03). All these tools require you to learn a new syntax as Boost.Local does. So what is your point?
The point is that the FP libraries offer additional value
functions. I note this in order to support the argument that Boost.Local does not solve the issues discussed above.
Let me clarify that Boost.Local goal is to support local functions with bindings and statement syntax for the function definition. Boost.Local never tried to solve this problem without introducing a new syntax for the function declaration (in fact earlier version of the library had an even more exotic syntax for declaring the local function).
Right, looks like i just miss the point.
Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations.
I don't think so... here bind is clearly used in a different context.
Yes that is what i meant. It is a overloading of terms.
Why would it be confusing? You spent a few hours reading the docs, do you honestly find it confusing? If so, do you think that a name different than bind should be used? (I can use any name we can think of and agree on.) For example, would "capture" be more clear:
int x = 0; void BOOST_LOCAL_FUNCTION_PARAMS(const capture& x) { ... // some long set of instruction ;) } BOOST_LOCAL_FUNCTION_NAME(f) f();
I never said it is confusing to me. I said to users. The comment above was trying to avoid overloading of terms. "bind" certainly has some history in boost and C++. That is all. You could argue that bind your context does something similar.
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I am sure there are lots of people that will never use a local blocks. That said, I personally needed to program (to ensure the constant-correctness requirement of assertions in Contract Programming):
int x = 0; const { assert( x == 0 ); }
You can do that with local blocks:
int x = 0; BOOST_LOCAL_BLOCK(const bind x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END
As we said, using lambdas or other Boost libraries there are other ways to do that but in my case I needed to keep the `assertion( x == 0 )`;` unchanged (including the variable name `x`) and these other ways (including C++11 lambdas) don't allow for that.
More in general, local blocks allow you to specify the sematic that a variable should be thread as const by the next bunch of instruction. I personally find that useful, others have made the same comment (Vicente, Pierre, Jeff, and another couple of people-- see their review to Boost.Local).
Sorry, I just don't get it ... why does it need to be surrounded by a macro?
Because I could not find any other way to declare the local function using statement syntax for its definition without using macros. Does this make sense?
No. I was aiming at: Why do you need a local function at all, when all you want is a new scope where you don't have access to the outer variables? That sounds awfully familiar to what a normal function is.
I can't fully appreciate the value here. Not because of other existing libraries, but because normal functions are way more expressive and clear than these macros can ever be.
But "normal" functions cannot be declared at local scope so they are not sufficient for the problem domain of this library.
This is exactly what I am trying to say. It looks to me that local is trying to solve a non-issue (at least for me). I argue that local functions are not needed in C++.
On Sat, Nov 19, 2011 at 12:15 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/18/2011 09:37 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 8:35 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/18/2011 04:44 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Additionally, despite the introductory motivating statement ("Local functions are a form of information hiding and are useful for dividing procedural tasks into subtasks which are only meaningful locally, avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts.") I can not see that this library solves this
Why Boost.Local does not solve this appropriately? What do you mean by "appropriately" in this context?
It tries do make it easier for users to define local functions in order to make their lifes easier so they don't have to clutter their code with local functions. This is what i fail to see:
Sorry, I don't understand this sentence. Can you please re-word it?
1) The macros do not even closely resemble any kind of function declarations in C++
Many have expressed different opinion appreciating instead the syntax. For example (but also other email thread on Boost and reviews):
That said, I understand how this is clearly a subjective point and I respect your opinion.
2) If you just want to write a simple local function, the epi- and prologue is longer than the actual function body (this is true for almost all your
A part form the two macro names, you don't have more tokens than the result type, parameter declarations, bind declarations, and local function name. IMO, these are all elements that syntactically should be part of the local function declaration regardless of the length of the body because they describe the local function interface and its bindings.
examples). And once the function body exceeds that limit I wonder why it should have been a local function to begin with.
Others have mentioned that it is fine for them to have long functions and local functions especially in FP:
-- from: http://lists.boost.org/Archives/boost/2011/02/176653.php --
In functional programming, there is no difference between defining lambdas and functions (except the former cannot be recursive), and it is not viewed as bad practice to have functions spanning a couple hundred lines.
-- end --
But of course I understand if you prefer to structure your code differently.
appropriately. I personally think that exposing macros as the interface to program with the library does not lead to a nice and clean design of ones code. Boost.Local is very different from other heavily macro based libraries like Boost.Parameter. It is different, because I would consider Boost.Local not as a infrastructure library but as a library for end users. This being said, i would not like to see application code cluttered with overly verbose calls to macros. As boost is trying to not only push the boundaries of C++ it should also try to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice.
I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
What kind of boilerplate code? Defining a global function? Most likely no. Defining a local functor? Yeah, probably, but do we really need that?
Yes, the boiler-plate code to define the local functor. And yes, you need that to implement the local function... Sorry but I'm missing something here... Can you please explain what you mean better with the two question marks above?
The question is about what I my point of criticism is all about: Why do we need a macro to define a function, that adds no more true value other than that it was defined in function scope?
Because using the macros we can program local functions using statement syntax for their bodies. The value is both (1) it's defined locally (close to the code that uses the function) and (2) its body uses statement syntax.
Defining a global functor? This is not possible with Local, so the answer would be "no" here.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
Well, I didn't say that I think ScopeExit is good. In fact, I never saw the
Can you please clarify? Would you make to ScopeExit the same criticisms you are making to Boost.Local syntax and its macro usage or not? // (1)
Yes, I would make the criticism to ScopeExit.
OK, thanks a lot for clarifying your position on ScopeExit too.
need to use it. As another reviewer noted, i'd rather encapsulate that in a class to logically divide that from the actual code. For ForEach the situation is completely different. Apart from having it to spell all caps, it looks exactly like a range based for each loop should look like. Boost.Local's Macros (yes, plural!) however, have nothing in common with the way you declare functions in C++.
Again, I understand that you personally view the syntax as not natural. (I am honestly sorry to hear that because I truly spent a lot of effort in trying to make the syntax as natural as possible based on everyone's input but I don't pretend to be able to make everyone happy so it's ok.)
Look, I am not trying to make you feel bad. I know that you put a lot of effort into making this library. If you remember, I wasn't happy about it from the start on, and i said so. As did others.
The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter.
Others have made the same comments (with you a total of 3 people). However, others have made the opposite comment (2 liked default, 5+ didn't say anything about it). In my experience, this quickly becomes a non-issue after the 1st week that you start using the library.
Right, i have to learn a new syntax in order to learn how to define local
That is true for Boost.Lambda, Boost.Phenix, and even for C++11 (because most programmers are now only familiar with C++03). All these tools require you to learn a new syntax as Boost.Local does. So what is your point?
The point is that the FP libraries offer additional value
As does my library. The additional value offered by my library is: (1) functions can be defined locally, (2) statement syntax can be used to program the function bodies, and (3) closures.
functions. I note this in order to support the argument that Boost.Local does not solve the issues discussed above.
Let me clarify that Boost.Local goal is to support local functions with bindings and statement syntax for the function definition. Boost.Local never tried to solve this problem without introducing a new syntax for the function declaration (in fact earlier version of the library had an even more exotic syntax for declaring the local function).
Right, looks like i just miss the point.
Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations.
I don't think so... here bind is clearly used in a different context.
Yes that is what i meant. It is a overloading of terms.
Why would it be confusing? You spent a few hours reading the docs, do you honestly find it confusing? If so, do you think that a name different than bind should be used? (I can use any name we can think of and agree on.) For example, would "capture" be more clear:
int x = 0; void BOOST_LOCAL_FUNCTION_PARAMS(const capture& x) { ... // some long set of instruction ;) } BOOST_LOCAL_FUNCTION_NAME(f) f();
I never said it is confusing to me. I said to users. The comment above was trying to avoid overloading of terms. "bind" certainly has some history in boost and C++. That is all. You could argue that bind your context does something similar.
OK so let's use capture instead of bind.
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I am sure there are lots of people that will never use a local blocks. That said, I personally needed to program (to ensure the constant-correctness requirement of assertions in Contract Programming):
int x = 0; const { assert( x == 0 ); }
You can do that with local blocks:
int x = 0; BOOST_LOCAL_BLOCK(const bind x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END
As we said, using lambdas or other Boost libraries there are other ways to do that but in my case I needed to keep the `assertion( x == 0 )`;` unchanged (including the variable name `x`) and these other ways (including C++11 lambdas) don't allow for that.
More in general, local blocks allow you to specify the sematic that a variable should be thread as const by the next bunch of instruction. I personally find that useful, others have made the same comment (Vicente, Pierre, Jeff, and another couple of people-- see their review to Boost.Local).
Sorry, I just don't get it ... why does it need to be surrounded by a macro?
Because I could not find any other way to declare the local function using statement syntax for its definition without using macros. Does this make sense?
No. I was aiming at: Why do you need a local function at all, when all you want is a new scope where you don't have access to the outer variables? That sounds awfully familiar to what a normal function is.
All we want is (1) a function defined locally, (2) to use statement syntax to program its body, and (3) to bind variables from the enclosing environment. That is precisely what my library offers. It is not a "normal" global function because (1) it not defined locally and (3) it does not bind from the enclosing scope. It is not a "normal" local function (member of a local class) because it does not bind variables in scope (3) (plus it cannot be passed as template parameter).
I can't fully appreciate the value here. Not because of other existing libraries, but because normal functions are way more expressive and clear than these macros can ever be.
But "normal" functions cannot be declared at local scope so they are not sufficient for the problem domain of this library.
This is exactly what I am trying to say. It looks to me that local is trying to solve a non-issue (at least for me). I argue that local functions are not needed in C++.
I don't understand... in your review you said: On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
- What is your evaluation of the potential usefulness of the library?
The potential usefulness of the library is out of question, that is why C++11 now has lambdas, and libraries like Bind, Lambda and Phoenix exist. However, I do not believe that Local adds any value to the existing solution of the described problems.
So are you voting "no" because: a) You don't think local functions are needed in C++. b) Or, you don't like macros in general and you don't like Boost.Local syntax in particular. c) You do not believe that Boost.Local adds any value to existing solutions to program local functions. Thanks a lot for the clarifications. --Lorenzo
Please let me clarify my vote again. Please also note, that the quality of the implementation and documentation is out of doubt. I vote to not include Boost.Local into Boost for the following reason. I obviously can not appreciate the value of local functions in C++. C++, IMHO, has better ways to structure code (namespaces and classes). Therefor, Boost.Local is not an alternative to C++-Lambdas, Boost.Bind, Boost.Lambda or Boost.Phoenix, but to regular C++ functions That being said, I don't see Boost.Local as a complement to the other already existing functional programming style libraries. I see it as a complement to regular functions. It is true that this implementation of local functions leads to function objects that can be used as callbacks where something callable is expected. However, that does not mean that they are higher order functions, which in turn means that they are not easily composable (that means, there is no way to compose two local functions to form another local function other than calling the macros again). Which has another side effect that they are completely incompatible with the existing solutions (ok, you can bind them again). Also, i think the verbosity of the macros defeat their initial purpose of "avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts". Additionally I can not see how local functions hide any information at all, the information is still visible (although hidden behind two enclosing, quite verbose, macro calls). I admit the fact, that we as C++ Programmers need Macros to avoid repeating ourselves (aka boilerplate code). A lot of libraries do exactly this, and it is considered good practice. My main point of critique was that the Boost.Local seem to implement a quite complex logic and, basically, their own language. I would not consider the preprocessor the proper place to implement a DSEL. Again, Lorenzo's effort and ingenuity of creating this very library is out of question, I solely believe that it is not the proper solution to this problem.
On Nov 21, 2011, at 11:36 AM, Thomas Heller wrote:
I vote to not include Boost.Local into Boost for the following reason. I obviously can not appreciate the value of local functions in C++.
I similarly have trouble seeing the value of the higher-order functional programming constructs provided by Phoenix. Personally when I want to program in a functional programming style I write my code in a language Haskell rather than shoehorning functional programming into C++ constructs that were never designed for such a purpose. When I translate my Haskell code into C++ (as I have done for a numerical code I once wrote), I would never think of using Phoenix, but instead I would change the idioms that I was using to match C++'s stateful, imperative nature. However, despite my criticisms I am in no way against the inclusion of Phoenix in Boost! I personally believe that programmers should have a toolbox with a broad selection of high-quality tools so that when a particular tool fits their problem, they can just grab it and make their lives easier. For this reason, although I do not use Phoenix myself or see myself using it in the foreseeable future to solve any of the problems that I face, I am happy to have it exist since clearly it is very useful for people who have need for such things as it provides --- and heck, maybe one day even I will stumble into a problem where it provides the best solution and be glad it is around. :-)
C++, IMHO, has better ways to structure code (namespaces and classes).
Although you don't see any reason to use Boost.Local when we already have classes and namespaces, similarly Linus doesn't see any reason to use C++ when C has structs and function pointers. :-) That is to say, Boost.Local doesn't let you do something you couldn't do without it --- just like the functional programming libraries don't. However, it can make life easier when you are facing a problem where the most natural fit is a local function defined at the point where it is used.
Therefor, Boost.Local is not an alternative to C++-Lambdas, Boost.Bind, Boost.Lambda or Boost.Phoenix, but to regular C++ functions
I am perfectly happy to give you an admission that Boost.Local is not a complete alternative to C++-Lambdas, Boost.Bind, Boost.Lambda or Boost.Phoenix and hence should not be directly compared with them --- as long as you are no longer claiming that Boost.Local is made redundant by these other options. :-)
That being said, I don't see Boost.Local as a complement to the other already existing functional programming style libraries. I see it as a complement to regular functions. It is true that this implementation of local functions leads to function objects that can be used as callbacks where something callable is expected.
Yes, and the way that I see it that is Boost.Locals' major point of existing --- to make callbacks easier to write for those of us who want compatibility with C++03. And that is good enough for me. :-)
However, that does not mean that they are higher order functions, which in turn means that they are not easily composable (that means, there is no way to compose two local functions to form another local function other than calling the macros again). Which has another side effect that they are completely incompatible with the existing solutions (ok, you can bind them again).
Fair enough, but that is true of plain C++ functions and methods and we don't consider them made redundant by the functional programming libraries. :-) Nonetheless, I really do have to concede that until this discussion I never really thought of Phoenix as a functional programming library that just *happened* to also be useful for callbacks. While I still don't see a use for Phoenix myself in the foreseeable future, I am really glad that we have had this discussion because it has given me more insight as to what Phoenix is all about, and made it more likely that I would consider it in the future now that my understanding of its purpose has been clarified.
Also, i think the verbosity of the macros defeat their initial purpose of "avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts".
I think that you must misunderstand what is meant by "avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts". The point is that the function is declared at the point of use rather than elsewhere; the "verbosity of the macros" does not change this and hence could not defeat this purpose.
Additionally I can not see how local functions hide any information at all, the information is still visible (although hidden behind two enclosing, quite verbose, macro calls).
Local functions hide information in two senses. First, they hide information in the same way that private members of classes do: although they are plainly visible in the code (and in particular the *public*-facing headers) you still can't easily get to them. (Of course, someone can technically mangle names to get access to the local functions and so they are not completely inaccessible, but likewise someone can use pointer tricks to get access to private variables and so in this sense local functions are not *less* hidden then private members.) Second, they keep the implementation details of a function "hidden" inside the function, so as not to provide noise *outside* the function to people who just want to treat the function as a black box. An additional advantage of this approach is that if a function needs to be copied or moved, you can just copy/move the body of the function as it its implementation is entirely self-contained.
I admit the fact, that we as C++ Programmers need Macros to avoid repeating ourselves (aka boilerplate code). A lot of libraries do exactly this, and it is considered good practice. My main point of critique was that the Boost.Local seem to implement a quite complex logic and, basically, their own language. I would not consider the preprocessor the proper place to implement a DSEL.
As I have said before, I find it somewhat difficult to believe that an author of *Phoenix* would find the Boost.Local DSL complicated. ;-) In my opinion, the Boost.Local DSL really isn't that all that complicated to use, though I will readily concede that at some point this becomes a matter of taste --- and as Lorenzo has said, taste is certainly a fair criteria to use in these reviews since we want all of the Boost libraries to be in good taste! As for the philosophical object to the heavy use of macros, I note that comparing Local which uses macros to Phoenix which uses template metaprogramming demonstrates a clear advantage of the macro approach: it provides error messages that can be understood by mere mortals. This is a feature that really can't be overstated, and if macros make it easier to provide this then I don't see the disadvantage of using them.
Again, Lorenzo's effort and ingenuity of creating this very library is out of question, I solely believe that it is not the proper solution to this problem.
We all appreciate your willingness to acknowledge this despite your personal distaste for the approach taken by Boost.Local. :-D Also, as I write earlier in this e-mail, while I strongly disagree with your arguments, the part of this discussion involving comparing Boost.Local to Boost.Phoenix has enlightened me as to what Boost.Phoenix is really about, for which I am greatly appreciative. Cheers, Greg
On 11/20/2011 09:27 PM, Gregory Crosswhite wrote:
On Nov 21, 2011, at 11:36 AM, Thomas Heller wrote:
I vote to not include Boost.Local into Boost for the following reason. I obviously can not appreciate the value of local functions in C++.
I similarly have trouble seeing the value of the higher-order functional programming constructs provided by Phoenix. Personally when I want to program in a functional programming style I write my code in a language Haskell rather than shoehorning functional programming into C++ constructs that were never designed for such a purpose. When I translate my Haskell code into C++ (as I have done for a numerical code I once wrote), I would never think of using Phoenix, but instead I would change the idioms that I was using to match C++'s stateful, imperative nature.
You should definitely take a look at spirit. Try to use Local in semantic actions.
However, despite my criticisms I am in no way against the inclusion of Phoenix in Boost! I personally believe that programmers should have a toolbox with a broad selection of high-quality tools so that when a particular tool fits their problem, they can just grab it and make their lives easier. For this reason, although I do not use Phoenix myself or see myself using it in the foreseeable future to solve any of the problems that I face, I am happy to have it exist since clearly it is very useful for people who have need for such things as it provides --- and heck, maybe one day even I will stumble into a problem where it provides the best solution and be glad it is around. :-)
I absolutely agree. However ...
C++, IMHO, has better ways to structure code (namespaces and classes).
Although you don't see any reason to use Boost.Local when we already have classes and namespaces, similarly Linus doesn't see any reason to use C++ when C has structs and function pointers. :-) That is to say, Boost.Local doesn't let you do something you couldn't do without it --- just like the functional programming libraries don't. However, it can make life easier when you are facing a problem where the most natural fit is a local function defined at the point where it is used.
... the proposed solution aren't real local functions, because you explicitly need to capture variables in scope and ...
Therefor, Boost.Local is not an alternative to C++-Lambdas, Boost.Bind, Boost.Lambda or Boost.Phoenix, but to regular C++ functions
I am perfectly happy to give you an admission that Boost.Local is not a complete alternative to C++-Lambdas, Boost.Bind, Boost.Lambda or Boost.Phoenix and hence should not be directly compared with them --- as long as you are no longer claiming that Boost.Local is made redundant by these other options. :-)
... I tried to distance myself from comparing Boost.Local to Bind, Lambda and Phoenix, because the comparison just isn't fair. I try to see them as an alternative to regular functions. And, TBH, it just doesn't cut it. Mainly because of the macro overhead introduced.
That being said, I don't see Boost.Local as a complement to the other already existing functional programming style libraries. I see it as a complement to regular functions. It is true that this implementation of local functions leads to function objects that can be used as callbacks where something callable is expected.
Yes, and the way that I see it that is Boost.Locals' major point of existing --- to make callbacks easier to write for those of us who want compatibility with C++03. And that is good enough for me. :-)
Right. TBH, I considered writing macros to reduce the boilerplate of creating such construct for Phoenix. I decided to stop investigating that because at the end of the day, the macros grew in complexity and actually writing the boilerplate yourself turned out to be clearer, shorter and more flexible.
However, that does not mean that they are higher order functions, which in turn means that they are not easily composable (that means, there is no way to compose two local functions to form another local function other than calling the macros again). Which has another side effect that they are completely incompatible with the existing solutions (ok, you can bind them again).
Fair enough, but that is true of plain C++ functions and methods and we don't consider them made redundant by the functional programming libraries. :-)
Nonetheless, I really do have to concede that until this discussion I never really thought of Phoenix as a functional programming library that just *happened* to also be useful for callbacks. While I still don't see a use for Phoenix myself in the foreseeable future, I am really glad that we have had this discussion because it has given me more insight as to what Phoenix is all about, and made it more likely that I would consider it in the future now that my understanding of its purpose has been clarified.
Well, when introducing phoenix, the main motivation is of course to provide an easy to use mechanism to define callbacks for functions like std::for_each. Turns out it can do a lot more.
Also, i think the verbosity of the macros defeat their initial purpose of "avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts".
I think that you must misunderstand what is meant by "avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts". The point is that the function is declared at the point of use rather than elsewhere; the "verbosity of the macros" does not change this and hence could not defeat this purpose.
Additionally I can not see how local functions hide any information at all, the information is still visible (although hidden behind two enclosing, quite verbose, macro calls).
Local functions hide information in two senses.
First, they hide information in the same way that private members of classes do: although they are plainly visible in the code (and in particular the *public*-facing headers) you still can't easily get to them. (Of course, someone can technically mangle names to get access to the local functions and so they are not completely inaccessible, but likewise someone can use pointer tricks to get access to private variables and so in this sense local functions are not *less* hidden then private members.)
Second, they keep the implementation details of a function "hidden" inside the function, so as not to provide noise *outside* the function to people who just want to treat the function as a black box. An additional advantage of this approach is that if a function needs to be copied or moved, you can just copy/move the body of the function as it its implementation is entirely self-contained.
Ok, i can live with that argument. However, i continue in claiming that the whole noise hiding and locality argument is defeated by the verbosity of the macros.
I admit the fact, that we as C++ Programmers need Macros to avoid repeating ourselves (aka boilerplate code). A lot of libraries do exactly this, and it is considered good practice. My main point of critique was that the Boost.Local seem to implement a quite complex logic and, basically, their own language. I would not consider the preprocessor the proper place to implement a DSEL.
As I have said before, I find it somewhat difficult to believe that an author of *Phoenix* would find the Boost.Local DSL complicated. ;-) In my opinion, the Boost.Local DSL really isn't that all that complicated to use, though I will readily concede that at some point this becomes a matter of taste --- and as Lorenzo has said, taste is certainly a fair criteria to use in these reviews since we want all of the Boost libraries to be in good taste!
Sorry, I wasn't entirely clear. I am all for DSELs, I just think that the PP is the wrong place. Admittingly, the complexity of functions is in the body, not the arguments, so arcane errors with the proposed library are more unlikely, but they do exist! I of course agree that the errors that come out of any ET based code are horrible. But they show exactly what went wrong, which is not so much the case when you do something wrong in the calls to the macro. To use one of Lorenzo's argument: "In my experience, this quickly becomes a non-issue after the 1st week that you start using the library." Which is true for all sorts of libraries.
As for the philosophical object to the heavy use of macros, I note that comparing Local which uses macros to Phoenix which uses template metaprogramming demonstrates a clear advantage of the macro approach: it provides error messages that can be understood by mere mortals. This is a feature that really can't be overstated, and if macros make it easier to provide this then I don't see the disadvantage of using them.
That is one of my points that i somehow can't convey: You don't need macros for that. Just code up a regular off-line function/function-object. Just try it, and you'll probably realize that the boilerplate you need is negligible compared to the boilerplate code you need to set up a local function.
Again, Lorenzo's effort and ingenuity of creating this very library is out of question, I solely believe that it is not the proper solution to this problem.
We all appreciate your willingness to acknowledge this despite your personal distaste for the approach taken by Boost.Local. :-D
Also, as I write earlier in this e-mail, while I strongly disagree with your arguments, the part of this discussion involving comparing Boost.Local to Boost.Phoenix has enlightened me as to what Boost.Phoenix is really about, for which I am greatly appreciative.
Well, we tried to keep Phoenix as close to C++ as possible and try to advertise as a C++-in-C++ DSEL. Of course, a lot of improvements can be made. One of which is to make it easier for users to use the "statement syntax". Which you, BTW, already can. Just not in function scope, for the reasons i tried to explain above. But this is not about Phoenix, but about Local.
Cheers, Greg
On Nov 21, 2011, at 2:16 PM, Thomas Heller wrote:
You should definitely take a look at spirit. Try to use Local in semantic actions.
Noted for future reference. :-)
... the proposed solution aren't real local functions, because you explicitly need to capture variables in scope and ...
If that is what you mean by "real local functions" then fair enough, Boost.Local does not provide "real local functions" --- and I do appreciate you clarifying what you meant by this. Nonetheless, I don't see why it should matter if the local functions are or are not "real local functions" by this definition because they are still enough like "local functions" to be more useful then not having local functions.
... I tried to distance myself from comparing Boost.Local to Bind, Lambda and Phoenix, because the comparison just isn't fair. I try to see them as an alternative to regular functions. And, TBH, it just doesn't cut it. Mainly because of the macro overhead introduced.
Yes, but the macro overhead buys you the ability to put the code where it is being used, and to avoid writing what is effectively the same code multiple times. To see one example of why this is an advantage, suppose that you need to bind another variable in your callback. With plain functions, you would need to scroll outside of your function to where you defined the callback function, add a parameter to that function, and then jump back to where it was being bound using Boost.Bind and then add the variable to the proper position. By contrast, with Boost.Local you don't have to jump around your code and you don't have to write multiple lines of code that essentially say the same thing, you can just add a single line to the callback macro --- which, again, is conveniently located *right* where it is being used so that you don't need to go far --- and you are done. Also, since the type is detected by Boost.Local automatically, if you change the type of the bound variable you don't have to do anything at all, whereas with a plain function callback you would have to scroll up and update the type manually.
Right. TBH, I considered writing macros to reduce the boilerplate of creating such construct for Phoenix. I decided to stop investigating that because at the end of the day, the macros grew in complexity and actually writing the boilerplate yourself turned out to be clearer, shorter and more flexible.
Clearly at this point you should have hired Lorenzo, since he is the macro master. ;-) It is a shame, though, because the existence of such might have made it more likely that I would have considered using Phoenix.
Ok, i can live with that argument. However, i continue in claiming that the whole noise hiding and locality argument is defeated by the verbosity of the macros.
Fair enough, though obviously I disagree. :-)
Sorry, I wasn't entirely clear. I am all for DSELs, I just think that the PP is the wrong place. Admittingly, the complexity of functions is in the body, not the arguments, so arcane errors with the proposed library are more unlikely, but they do exist! I of course agree that the errors that come out of any ET based code are horrible. But they show exactly what went wrong, which is not so much the case when you do something wrong in the calls to the macro.
I understand your point that technically macro error messages have a disadvantage over template error messages in that they don't show you exactly what went wrong since the code is implicitly generated and often not shown. However, in practice this advantage really doesn't matter, because when an error message is pages long and gives you no insight at all as to what *you* did wrong, then the fact that at least it is showing you exactly what went wrong in all the layers of code that were touched is cold comfort. :-) So in short, I don't see how using PP is necessarily at a disadvantage *in practice* when implementing DSELs, given that the error messages for complex template-metaprogramming libraries are hardly any better.
To use one of Lorenzo's argument: "In my experience, this quickly becomes a non-issue after the 1st week that you start using the library." Which is true for all sorts of libraries.
Yes, but this library has a shallower learning curve than many. I would say that the syntax became a non-issue for me after more like an hour using it.
As for the philosophical object to the heavy use of macros, I note that comparing Local which uses macros to Phoenix which uses template metaprogramming demonstrates a clear advantage of the macro approach: it provides error messages that can be understood by mere mortals. This is a feature that really can't be overstated, and if macros make it easier to provide this then I don't see the disadvantage of using them.
That is one of my points that i somehow can't convey: You don't need macros for that. Just code up a regular off-line function/function-object. Just try it, and you'll probably realize that the boilerplate you need is negligible compared to the boilerplate code you need to set up a local function.
Sheesh, do you really think that ever since starting to use Boost.Local I have never even once written a regular off-line function/function-object and compared the experience I had to using Boost.Local? ;-) First, boilerplate is not just about the number of characters typed, it is about the amount of redundant information you have to specify. Boost.Local allows me to declare the variables being bound at exactly one point in the code (which, again, is right next to where it is being used) and I don't even have to specify the types. Second, using Boost.Local allows me to locate the function body right where the callback is being passed in, which is often the most natural location for such information. The way I figure it, there is no point in making someone reading my code jump around to figure out exactly what the callback I just passed into another function is doing when I can provide that information exactly where and when they need it. Finally, as I said before Boost.Local allows me to "hide" a callback inside the function which uses it, which plain functions/function objects don't in the same way. All three of these features together are worth it in my mind, even if the actual typing were slightly more. So in short, no, Boost.Local is not really about just saving a little typing; I could understand, though, how you would be perplexed if that is what you thought it was primarily meant to do. :-) Cheers, Greg
On Nov 21, 2011, at 2:16 PM, Thomas Heller wrote:
... I tried to distance myself from comparing Boost.Local to Bind, Lambda and Phoenix, because the comparison just isn't fair. I try to see them as an alternative to regular functions. And, TBH, it just doesn't cut it. Mainly because of the macro overhead introduced.
On 11/21/2011 1:27 PM, Gregory Crosswhite wrote:
Yes, but the macro overhead buys you the ability to put the code where it is being used, and to avoid writing what is effectively the same code multiple times. To see one example of why this is an advantage, suppose that you need to bind another variable in your callback. With plain functions, you would need to scroll outside of your function to where you defined the callback function, add a parameter to that function, and then jump back to where it was being bound using Boost.Bind and then add the variable to the proper position. By contrast, with Boost.Local you don't have to jump around your code and you don't have to write multiple lines of code that essentially say the same thing, you can just add a single line to the callback macro --- which, again, is conveniently located *right* where it is being used so that you don't need to go far --- and you are done.
1) You don't have to scroll up and down and jump around your code if you avoid big function bodies --always a good practice anyway. 2) If you have lots of local functions, you'll still have to scroll up and down and jump around your code. This happens when you refactor big function bodies into N smaller functions where N is more than a few. If N == 1 or 2, then 1) above applies.
Also, since the type is detected by Boost.Local automatically, if you change the type of the bound variable yo u don't have to do anything at all, whereas with a plain function callback you would have to scroll up and update the type manually.
3) Make them templates. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Mon, Nov 21, 2011 at 12:27 AM, Gregory Crosswhite <gcrosswhite@gmail.com> wrote:
On Nov 21, 2011, at 2:16 PM, Thomas Heller wrote:
You should definitely take a look at spirit. Try to use Local in semantic actions.
Noted for future reference. :-)
... the proposed solution aren't real local functions, because you explicitly need to capture variables in scope and ...
If that is what you mean by "real local functions" then fair enough, Boost.Local does not provide "real local functions" --- and I do appreciate you clarifying what you meant by this. Nonetheless, I don't see why it should matter if the local functions are or are not "real local functions" by this definition because they are still enough like "local functions" to be more useful then not having local functions.
... I tried to distance myself from comparing Boost.Local to Bind, Lambda and Phoenix, because the comparison just isn't fair. I try to see them as an alternative to regular functions. And, TBH, it just doesn't cut it. Mainly because of the macro overhead introduced.
Yes, but the macro overhead buys you the ability to put the code where it is being used, and to avoid writing what is effectively the same code multiple times. To see one example of why this is an advantage, suppose that you need to bind another variable in your callback. With plain functions, you would need to scroll outside of your function to where you defined the callback function, add a parameter to that function, and then jump back to where it was being bound using Boost.Bind and then add the variable to the proper position. By contrast, with Boost.Local you don't have to jump around your code and you don't have to write multiple lines of code that essentially say the same thing, you can just add a single line to the callback macro --- which, again, is conveniently located *right* where it is being used so that you don't need to go far --- and you are done. Also, since the type is detected by Boost.Local automatically, if you change the type of the bound variable yo u don't have to do anything at all, whereas with a plain function callback you would have to scroll up and update the type manually.
Right. TBH, I considered writing macros to reduce the boilerplate of creating such construct for Phoenix. I decided to stop investigating that because at the end of the day, the macros grew in complexity and actually writing the boilerplate yourself turned out to be clearer, shorter and more flexible.
If there is still an interest in doing this, I'd be very happy to help at least in trying to identify which DSEL syntax is possible for such macros. To start I would need: 1) A example (that compiles) of the hand-written code without any macro. [current] 2) And then, forget what you know about macros and C++, and write down the syntax that you would instead like to use to program 1) if you could define your own language. [ideal] I can then look if 2) is possible and if not how to modify 2) by making it perhaps less ideal but feasible. After we have defined how the macros would look like, we can decide if it is worth implementing them or not. Thomas, Joel, and alt.: If you are still interested in doing this, start another email thread so we can discuss how these potential Phoenix macros could look like.
Clearly at this point you should have hired Lorenzo, since he is the macro master. ;-)
(Nope, Paul is (I really mean it, I'm not just saying it to act modest).)
It is a shame, though, because the existence of such might have made it more likely that I would have considered using Phoenix.
Ok, i can live with that argument. However, i continue in claiming that the whole noise hiding and locality argument is defeated by the verbosity of the macros.
Fair enough, though obviously I disagree. :-)
Sorry, I wasn't entirely clear. I am all for DSELs, I just think that the PP is the wrong place. Admittingly, the complexity of functions is in the body, not the arguments, so arcane errors with the proposed library are more unlikely, but they do exist! I of course agree that the errors that come out of any ET based code are horrible. But they show exactly what went wrong, which is not so much the case when you do something wrong in the calls to the macro.
I understand your point that technically macro error messages have a disadvantage over template error messages in that they don't show you exactly what went wrong since the code is implicitly generated and often not shown. However, in practice this advantage really doesn't matter, because when an error message is pages long and gives you no insight at all as to what *you* did wrong, then the fact that at least it is showing you exactly what went wrong in all the layers of code that were touched is cold comfort. :-)
So in short, I don't see how using PP is necessarily at a disadvantage *in practice* when implementing DSELs, given that the error messages for complex template-metaprogramming libraries are hardly any better.
To use one of Lorenzo's argument: "In my experience, this quickly becomes a non-issue after the 1st week that you start using the library." Which is true for all sorts of libraries.
Yes, but this library has a shallower learning curve than many. I would say that the syntax became a non-issue for me after more like an hour using it.
As for the philosophical object to the heavy use of macros, I note that comparing Local which uses macros to Phoenix which uses template metaprogramming demonstrates a clear advantage of the macro approach: it provides error messages that can be understood by mere mortals. This is a feature that really can't be overstated, and if macros make it easier to provide this then I don't see the disadvantage of using them.
That is one of my points that i somehow can't convey: You don't need macros for that. Just code up a regular off-line function/function-object. Just try it, and you'll probably realize that the boilerplate you need is negligible compared to the boilerplate code you need to set up a local function.
Sheesh, do you really think that ever since starting to use Boost.Local I have never even once written a regular off-line function/function-object and compared the experience I had to using Boost.Local? ;-)
First, boilerplate is not just about the number of characters typed, it is about the amount of redundant information you have to specify. Boost.Local allows me to declare the variables being bound at exactly one point in the code (which, again, is right next to where it is being used) and I don't even have to specify the types.
Second, using Boost.Local allows me to locate the function body right where the callback is being passed in, which is often the most natural location for such information. The way I figure it, there is no point in making someone reading my code jump around to figure out exactly what the callback I just passed into another function is doing when I can provide that information exactly where and when they need it.
Finally, as I said before Boost.Local allows me to "hide" a callback inside the function which uses it, which plain functions/function objects don't in the same way.
All three of these features together are worth it in my mind, even if the actual typing were slightly more.
So in short, no, Boost.Local is not really about just saving a little typing; I could understand, though, how you would be perplexed if that is what you thought it was primarily meant to do. :-)
Thanks. --Lorenzo
On Sun, Nov 20, 2011 at 11:16 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/20/2011 09:27 PM, Gregory Crosswhite wrote:
On Nov 21, 2011, at 11:36 AM, Thomas Heller wrote:
I vote to not include Boost.Local into Boost for the following reason. I obviously can not appreciate the value of local functions in C++.
I similarly have trouble seeing the value of the higher-order functional programming constructs provided by Phoenix. Personally when I want to program in a functional programming style I write my code in a language Haskell rather than shoehorning functional programming into C++ constructs that were never designed for such a purpose. When I translate my Haskell code into C++ (as I have done for a numerical code I once wrote), I would never think of using Phoenix, but instead I would change the idioms that I was using to match C++'s stateful, imperative nature.
You should definitely take a look at spirit. Try to use Local in semantic actions.
OK but I'm not at all a Spirit or Phoenix expert so I'll really need your help and guidance. Please note that I'm no Phoenix expert and I know almost nothing about Spirit so this is not aimed to show that Boost.Local could be useful in this context (I have literally no way to predict or assess such usefulness upfront given my sever lack of expertise with Spirit). I'm honestly intellectually interested in discovering the outcome of this exercise together :) Let me start by using Boost.Local to program the semantic action's example from the Spirit docs: http://www.boost.org/doc/libs/1_48_0/libs/spirit/example/qi/actions.cpp #include <boost/local/function.hpp> #include <boost/config/warning_disable.hpp> #include <boost/spirit/include/qi.hpp> #include <iostream> namespace client { namespace qi = boost::spirit::qi; struct print_action { void operator()(int const& i, qi::unused_type, qi::unused_type) const { std::cout << i << std::endl; } }; } // namespace int main() { using boost::spirit::qi::int_; using boost::spirit::qi::parse; using boost::spirit::qi::unused_type; using client::print_action; { // example using simple function object char const *first = "{43}", *last = first + std::strlen(first); parse(first, last, '{' >> int_[print_action()] >> '}'); } { // example using Boost.Local void BOOST_LOCAL_FUNCTION_PARAMS(int const& i, unused_type, unused_type) { std::cout << i << std::endl; } BOOST_LOCAL_FUNCTION_NAME(print_local) char const *first = "{40}", *last = first + std::strlen(first); parse(first, last, '{' >> int_[print_local] >> '}'); } return 0; } This compiles and runs (I can say that much :) ) but a part from that I can't quite comment given my Spirit ignorance. I honestly ask you: What do you think? Notes: 1) Is this too easy of an example? If so, where can I find a more complex one? Maybe in other points of the docs? 2) I removed the Bind, Lambda, and global function versions thinking/assuming that the comparison with global functor was the most interesting one... was that a mistake? Thanks. --Lorenzo
On 11/21/2011 7:41 PM, Lorenzo Caminiti wrote:
On Sun, Nov 20, 2011 at 11:16 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/20/2011 09:27 PM, Gregory Crosswhite wrote:
On Nov 21, 2011, at 11:36 AM, Thomas Heller wrote:
I vote to not include Boost.Local into Boost for the following reason. I obviously can not appreciate the value of local functions in C++.
I similarly have trouble seeing the value of the higher-order functional programming constructs provided by Phoenix. Personally when I want to program in a functional programming style I write my code in a language Haskell rather than shoehorning functional programming into C++ constructs that were never designed for such a purpose. When I translate my Haskell code into C++ (as I have done for a numerical code I once wrote), I would never think of using Phoenix, but instead I would change the idioms that I was using to match C++'s stateful, imperative nature.
You should definitely take a look at spirit. Try to use Local in semantic actions.
OK but I'm not at all a Spirit or Phoenix expert so I'll really need your help and guidance. Please note that I'm no Phoenix expert and I know almost nothing about Spirit so this is not aimed to show that Boost.Local could be useful in this context (I have literally no way to predict or assess such usefulness upfront given my sever lack of expertise with Spirit). I'm honestly intellectually interested in discovering the outcome of this exercise together :)
Let me start by using Boost.Local to program the semantic action's example from the Spirit docs: http://www.boost.org/doc/libs/1_48_0/libs/spirit/example/qi/actions.cpp
#include <boost/local/function.hpp> #include <boost/config/warning_disable.hpp> #include <boost/spirit/include/qi.hpp> #include <iostream>
namespace client {
namespace qi = boost::spirit::qi;
struct print_action { void operator()(int const& i, qi::unused_type, qi::unused_type) const { std::cout << i << std::endl; } };
} // namespace
int main() { using boost::spirit::qi::int_; using boost::spirit::qi::parse; using boost::spirit::qi::unused_type; using client::print_action;
{ // example using simple function object
char const *first = "{43}", *last = first + std::strlen(first); parse(first, last, '{' >> int_[print_action()] >> '}'); }
{ // example using Boost.Local void BOOST_LOCAL_FUNCTION_PARAMS(int const& i, unused_type, unused_type) { std::cout << i << std::endl; } BOOST_LOCAL_FUNCTION_NAME(print_local)
char const *first = "{40}", *last = first + std::strlen(first); parse(first, last, '{' >> int_[print_local] >> '}'); }
return 0; }
This compiles and runs (I can say that much :) ) but a part from that I can't quite comment given my Spirit ignorance. I honestly ask you: What do you think?
Notes: 1) Is this too easy of an example? If so, where can I find a more complex one? Maybe in other points of the docs?
Yes, go further down the docs where Phoenix is used.
2) I removed the Bind, Lambda, and global function versions thinking/assuming that the comparison with global functor was the most interesting one...
Really? Read what you quoted above. It's about Phoenix, function composition and higher order functions.
was that a mistake?
Definitely, yes. You can't really be seriously dabbling locals into FP land now, are you? :-) Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Mon, Nov 21, 2011 at 8:07 AM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 11/21/2011 7:41 PM, Lorenzo Caminiti wrote:
On Sun, Nov 20, 2011 at 11:16 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/20/2011 09:27 PM, Gregory Crosswhite wrote:
On Nov 21, 2011, at 11:36 AM, Thomas Heller wrote:
I vote to not include Boost.Local into Boost for the following reason. I obviously can not appreciate the value of local functions in C++.
I similarly have trouble seeing the value of the higher-order functional programming constructs provided by Phoenix. Personally when I want to program in a functional programming style I write my code in a language Haskell rather than shoehorning functional programming into C++ constructs that were never designed for such a purpose. When I translate my Haskell code into C++ (as I have done for a numerical code I once wrote), I would never think of using Phoenix, but instead I would change the idioms that I was using to match C++'s stateful, imperative nature.
You should definitely take a look at spirit. Try to use Local in semantic actions.
OK but I'm not at all a Spirit or Phoenix expert so I'll really need your help and guidance. Please note that I'm no Phoenix expert and I know almost nothing about Spirit so this is not aimed to show that Boost.Local could be useful in this context (I have literally no way to predict or assess such usefulness upfront given my sever lack of expertise with Spirit). I'm honestly intellectually interested in discovering the outcome of this exercise together :)
Let me start by using Boost.Local to program the semantic action's example from the Spirit docs: http://www.boost.org/doc/libs/1_48_0/libs/spirit/example/qi/actions.cpp
#include <boost/local/function.hpp> #include <boost/config/warning_disable.hpp> #include <boost/spirit/include/qi.hpp> #include <iostream>
namespace client {
namespace qi = boost::spirit::qi;
struct print_action { void operator()(int const& i, qi::unused_type, qi::unused_type) const { std::cout << i << std::endl; } };
} // namespace
int main() { using boost::spirit::qi::int_; using boost::spirit::qi::parse; using boost::spirit::qi::unused_type; using client::print_action;
{ // example using simple function object
char const *first = "{43}", *last = first + std::strlen(first); parse(first, last, '{' >> int_[print_action()] >> '}'); }
{ // example using Boost.Local void BOOST_LOCAL_FUNCTION_PARAMS(int const& i, unused_type, unused_type) { std::cout << i << std::endl; } BOOST_LOCAL_FUNCTION_NAME(print_local)
char const *first = "{40}", *last = first + std::strlen(first); parse(first, last, '{' >> int_[print_local] >> '}'); }
return 0; }
This compiles and runs (I can say that much :) ) but a part from that I can't quite comment given my Spirit ignorance. I honestly ask you: What do you think?
Notes: 1) Is this too easy of an example? If so, where can I find a more complex one? Maybe in other points of the docs?
Yes, go further down the docs where Phoenix is used.
2) I removed the Bind, Lambda, and global function versions thinking/assuming that the comparison with global functor was the most interesting one...
Really? Read what you quoted above. It's about Phoenix, function composition and higher order functions.
was that a mistake?
Definitely, yes. You can't really be seriously dabbling locals into FP land now, are you? :-)
Well, I am seriously asking for your guidance entering the Spirit/Phoenix domain. Would this be a good example or is there a better one? http://www.boost.org/doc/libs/1_48_0/libs/spirit/example/qi/num_list4.cpp There are quire a few examples in the docs even after num_list4 (which is good) so I can use your help in selecting the best one for this exercise. Thanks for the input. --Lorenzo
On 11/21/2011 9:17 PM, Lorenzo Caminiti wrote:
Definitely, yes. You can't really be seriously dabbling locals into FP land now, are you? :-)
Well, I am seriously asking for your guidance entering the Spirit/Phoenix domain.
Why?
Would this be a good example or is there a better one? http://www.boost.org/doc/libs/1_48_0/libs/spirit/example/qi/num_list4.cpp There are quire a few examples in the docs even after num_list4 (which is good) so I can use your help in selecting the best one for this exercise.
Are you really serious :-) ? Ok, try the simplest calculator that uses phoenix: http://www.boost.org/doc/libs/1_48_0/libs/spirit/example/qi/compiler_tutoria... That's the "hello world" for parsers, so to speak. See the code in square brackets. Those are your phoenix semantic actions. E.g.: term [_val = _1] Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Sun, Nov 20, 2011 at 8:36 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Please let me clarify my vote again. Please also note, that the quality of the implementation and documentation is out of doubt.
I vote to not include Boost.Local into Boost for the following reason. I obviously can not appreciate the value of local functions in C++. C++, IMHO, has better ways to structure code (namespaces and classes). Therefor, Boost.Local is not an alternative to C++-Lambdas, Boost.Bind, Boost.Lambda or Boost.Phoenix,
That is true for a very simple reason: Boost.Local functions cannot be defined with an expression. Boost.Local don't define lambdas but they define closures at local scope.
but to regular C++ functions That being said, I don't see Boost.Local as a complement to the other already existing functional programming style libraries. I see it as a complement to regular functions. It is true that this implementation of local functions leads to function objects that can be used as callbacks where something callable is expected. However, that does not mean that they are higher order functions, which in turn means that they are not easily composable (that means, there is no way to compose two local functions to form another local function other than calling the macros again). Which has
You can bind a local function to another local function thus composing them together without needing to call the macros again: https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... int x = 10; int BOOST_LOCAL_FUNCTION(int y, const bind x) { return x + y; } BOOST_LOCAL_FUNCTION(g) int BOOST_LOCAL_FUNCTION(int y, const bind g) { return g(y); } BOOST_LOCAL_FUNCTION_END(f) f(5); This composition is "easy" (if by "easy" we mean "without calling the macros again").
another side effect that they are completely incompatible with the existing solutions (ok, you can bind them again).
Yes, you can compose them by "binding them again" as shown by the example above.
Also, i think the verbosity of the macros defeat their initial purpose of "avoiding cluttering other parts of the program with functions, variables, etc unrelated to those parts". Additionally I can not see how local functions hide any information at all, the information is still visible
Local functions hide information in same sense as explained by N2511. Mainly: 1) Local functions are declared locally, next to the code where they are directly needed (the std::for_each call, etc). 2) Local functions are only visible by the scope that needs to call them (another part of the program can't call them by "mistake" because they are not accessible outside the enclosing function). If you open up the source file, you can of course read the local function code as you can read any other implementation code. However, both local function and implementation code is hidden because it is not accessible from outer scopes within your program.
(although hidden behind two enclosing, quite verbose, macro calls).
Nope, local functions are /not/ hidden by the wrapping macros at all (they are declared by the wrapping macros). They are instead hidden by the enclosing function scope which makes local functions not accessible/visible from outside the enclosing function (even if they are still readable as any non-obfuscated code is). Does this make sense?
I admit the fact, that we as C++ Programmers need Macros to avoid repeating ourselves (aka boilerplate code). A lot of libraries do exactly this, and it is considered good practice. My main point of critique was that the Boost.Local seem to implement a quite complex logic and, basically, their own language. I would not consider the preprocessor the proper place to implement a DSEL.
On the contrary, there's no way to use macros at all without introducing a DSEL by definition. Oops, this might be a controversial point (and I don't want to speak for Paul) but it has been argued that the moment you use a macro you are introducing a DSEL by #definition ;) That is because the pp is a code generator and it has nothing to do with the underling language (C or C++) by definition. Even in simple "function-like" (whatever that means) usage of macros: #define M(x, y) (x-y) // (1) You are introducing a DSEL because the following: M( , 2) // expands to -2 Would not be legal if M was a C++ function but it is legal given that M is a macro! That shows that even the super-simple (1) defines a DSEL with respect to the underlining C++ language. All of that said, I definitively think that you can draw different degrees of complexity of the DSEL you /have to/ define with your macros. You can say that the DSEL introduced by (1) is trivial and acceptable but the DSEL introduced by Boost.Local is too complex and not acceptable (which I think it is your point) [we'll leave out the DSEL introduced by Boost.Contract because on this scale it would be pure madness :) ]. However, they all are DSEL (and not C++) by definition simply because they use macros.
Again, Lorenzo's effort and ingenuity of creating this very library is out of question,
Thanks :)
I solely believe that it is not the proper solution to this problem.
--Lorenzo
On 11/19/2011 6:44 AM, Lorenzo Caminiti wrote:
As boost is trying to not only push the boundaries of C++ it should also try
to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice. I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
I share Thomas' sentiment. But IMO, it's really not about using macros. it's about the syntax. Boost.Foreach looks exactly like a for loop. The proposed boost locals syntax is arcane. IMO, bottomline: it's about the design of the DSEL regardless how it is implemented. And, the syntax proposed is, well, utterly ugly. Anything more complex than foreach, done using the PP will be a mess. An ugly syntax can be acceptable IFF there is no other way to do it. In this case (locals), there are better ways of doing the same thing. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Fri, Nov 18, 2011 at 10:19 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 11/19/2011 6:44 AM, Lorenzo Caminiti wrote:
As boost is trying to not only push the boundaries of C++ it should also try
to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice. I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
I share Thomas' sentiment. But IMO, it's really not about using macros. it's about the syntax. Boost.Foreach looks exactly like a for loop. The proposed boost locals syntax is arcane. IMO, bottomline: it's about the design of the DSEL regardless how it is implemented. And, the syntax proposed is, well, utterly ugly. Anything more complex than foreach, done using the PP will be a mess. An ugly syntax can be acceptable
That is fine but the definition of "ugly" is subjective so IMO it should be left up to the reviewers (and Thomas' point is clear, the syntax is ugly for him as one of the reviewers).
IFF there is no other way to do it. In this case (locals), there are better ways of doing the same thing.
But there is no other way to do locals allowing to use statement syntax for the function definition. Thanks. --Lorenzo
On 11/19/2011 11:44 AM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 10:19 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 11/19/2011 6:44 AM, Lorenzo Caminiti wrote:
As boost is trying to not only push the boundaries of C++ it should also try
to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice. I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
I share Thomas' sentiment. But IMO, it's really not about using macros. it's about the syntax. Boost.Foreach looks exactly like a for loop. The proposed boost locals syntax is arcane. IMO, bottomline: it's about the design of the DSEL regardless how it is implemented. And, the syntax proposed is, well, utterly ugly. Anything more complex than foreach, done using the PP will be a mess. An ugly syntax can be acceptable
That is fine but the definition of "ugly" is subjective so IMO it should be left up to the reviewers (and Thomas' point is clear, the syntax is ugly for him as one of the reviewers).
IFF there is no other way to do it. In this case (locals), there are better ways of doing the same thing.
But there is no other way to do locals allowing to use statement syntax for the function definition.
Why can't we C++ programmers just accept the fact that we don't have *true* local functions like those in Pascal and Modula? Challenge: try to port one moderately sized Pascal code (I suggest a recursive descent calculator parser) using your proposed library. I know the result will be utterly ugly and unreadable compared to the real thing, but let the readers be the judge. I can write the same using non-local functions and/or offline function objects using only straight C++. The result will be very readable. Not as elegant as the one using Modula's local functions, but certainly better than using your proposed locals library. That is what I mean by "better ways of doing the same thing". Again, I share Thomas' sentiment. It's not worth the additional complexity and arcane syntax. There are two scenarios here: 1) simple local functions --for those, you don't want the complexity of using a macro DSEL, just use plain functions and function objects. 2) Moderate to complex programs --for those, the macro syntax will get in the way of clarity of code -- for those, just use plain functions and function objects. And I am not even mentioning c++ lambda. You can nitpick all you want, but it's still nitpicking. Nitpicks are not valid rationale for justifying a library for boost. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Nov 19, 2011, at 2:23 PM, Joel de Guzman wrote:
Why can't we C++ programmers just accept the fact that we don't have *true* local functions like those in Pascal and Modula? [...] Again, I share Thomas' sentiment. It's not worth the additional complexity and arcane syntax.
Please forgive me, but it is very difficult to believe that you are seriously making these kinds of criticisms given that you are one of the authors of Boost.Phoenix which itself uses an incredibly complex and arcane syntax to attempt to give C++ features that it doesn't "truly" have. Cheers, Greg
On 11/19/2011 12:54 PM, Gregory Crosswhite wrote:
On Nov 19, 2011, at 2:23 PM, Joel de Guzman wrote:
Why can't we C++ programmers just accept the fact that we don't have *true* local functions like those in Pascal and Modula? [...] Again, I share Thomas' sentiment. It's not worth the additional complexity and arcane syntax.
Please forgive me, but it is very difficult to believe that you are seriously making these kinds of criticisms given that you are one of the authors of Boost.Phoenix which itself uses an incredibly complex and arcane syntax to attempt to give C++ features that it doesn't "truly" have.
Phoenix is doing something that needs to be done and cannot be done any other way. It is not the same with locals where there are other ways to do the same thing and the only advantage I can see is a bit of extra PP-based sugar. We can live without local functions, but we cannot live without FP as a means for programming with anonymous functions, lazy evaluation, partial function application, etc. See the big difference? Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Sat, Nov 19, 2011 at 5:37 AM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 11/19/2011 12:54 PM, Gregory Crosswhite wrote:
On Nov 19, 2011, at 2:23 PM, Joel de Guzman wrote:
Why can't we C++ programmers just accept the fact that we don't have *true* local functions like those in Pascal and Modula? [...] Again, I share Thomas' sentiment. It's not worth the additional complexity and arcane syntax.
Please forgive me, but it is very difficult to believe that you are seriously making these kinds of criticisms given that you are one of the authors of Boost.Phoenix which itself uses an incredibly complex and arcane syntax to attempt to give C++ features that it doesn't "truly" have.
Phoenix is doing something that needs to be done and cannot be done any other way. It is not the same with locals where there are other ways to do the same thing and the only advantage I can see is a bit of extra PP-based sugar. We can live without local functions, but we
Well, we can but we don't have to live without local functions, we can use Boost.Local (I think I'm starting to repeat myself... I should stop that :) ).
cannot live without FP as a means for programming with anonymous functions, lazy evaluation, partial function application, etc.
See the big difference?
This is a bit off topic... but how do lambdas and other C++11 features change this landscape for FP? (They obviously provide anonymous functions and closures... how about the rest?) Thanks. --Lorenzo
On 11/19/2011 7:03 PM, Lorenzo Caminiti wrote:
On Sat, Nov 19, 2011 at 5:37 AM, Joel de Guzman
This is a bit off topic... but how do lambdas and other C++11 features change this landscape for FP? (They obviously provide anonymous functions and closures... how about the rest?)
Surely off topic. Anyway: Phoenix will still be relevant because 1) it is more than just lambda 2) c++11 lambda is monomorphic (*). Until such time when c++ lambda becomes polymorphic, there's still no substitute for phoenix lambda. (* parametric polymorphism) Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Fri, Nov 18, 2011 at 11:23 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 11/19/2011 11:44 AM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 10:19 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 11/19/2011 6:44 AM, Lorenzo Caminiti wrote:
As boost is trying to not only push the boundaries of C++ it should also try
to set examples of best C++ practice. I would not consider Macro calls to interface with a library not good practice. I understand. I disagree but I understand your point. I disagree because the use of macros in this context saves the user from writing verbose boiler-plate code.
IMO, it is similar to Boost.ScopeExit: Instead of providing the SCOPE_EXIT macros we could have said that the user writes the code to bind the variables deducing their types and it programs the local class with the exit code in the desstructor all of that by hand. However, using macros saves the user to write all of that and as I user I personally appreciate that. It is true that there is a trade off between the readability cost of using macros and the benefit of not having to write boiler-plate code. I find that trade off acceptable for libraries like Boost.SopeExit or even ForEach and (of course ;) ) also for my library so I don't have to write the boiler-plate code for the binding and the casting to pass the local class as a template parameter.
I share Thomas' sentiment. But IMO, it's really not about using macros. it's about the syntax. Boost.Foreach looks exactly like a for loop. The proposed boost locals syntax is arcane. IMO, bottomline: it's about the design of the DSEL regardless how it is implemented. And, the syntax proposed is, well, utterly ugly. Anything more complex than foreach, done using the PP will be a mess. An ugly syntax can be acceptable
That is fine but the definition of "ugly" is subjective so IMO it should be left up to the reviewers (and Thomas' point is clear, the syntax is ugly for him as one of the reviewers).
IFF there is no other way to do it. In this case (locals), there are better ways of doing the same thing.
But there is no other way to do locals allowing to use statement syntax for the function definition.
Why can't we C++ programmers just accept the fact that we don't have *true* local functions like those in Pascal and Modula?
Because now we have Boost.Local so we can program true local functions with all their functionality :) (If not, which local function functionality is missing from Boost.Local?)
Challenge: try to port one moderately sized Pascal code (I suggest a recursive descent calculator parser) using your proposed library. I know the result will be utterly ugly and unreadable compared to the real thing, but let the readers be the judge.
Sure, after the review I'll program such an example and add it to the Examples section https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... BTW, do you have some code (possibly Pascal) to start from? Do you have such a code programmed in Phoenix already?
I can write the same using non-local functions and/or offline function objects using only straight C++. The result will be very readable. Not as elegant as the one using Modula's local functions, but certainly better than using your proposed locals library. That is what I mean by "better ways of doing the same thing".
Well, we'll have to wait and see the code example to judge. Please note that had this request come ~1year ago when I started collecting feedback, I'd be happy to program the example just using the proposed library syntax even before starting to implementing it. Too bad no one thought about it back then... but it's never too late. To the end of exercising my library, I did program /all/ the examples I found from the references I looked at (N2511: https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... Including the examples from GCC extension nested functions: http://gcc.gnu.org/onlinedocs/gcc/Nested-Functions.html https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_loca... But these are short examples.
Again, I share Thomas' sentiment. It's not worth the additional complexity and arcane syntax. There are two scenarios here:
What do you mean by "complexity" here? As for the syntax, with respect the the normal C++ syntax: // some local scope int f(int x) { ... // lots of instructions here } It requires you to do two things: 1) Wrap the function name and parameter list in two macros. 2) And, move the function name after the function body. // some local scope int PARAMS(int x) { ... // lots of instructions here } NAME(f) To me and others this does not look arcane but I understand this is a subjective. BTW, why would you consider the above syntax arcane and the one below "nice"? (I am honestly asking such a question, it is not that people cannot change their minds, there would be no improvement otherwise, but if that is the case I would sincerely like to know what prompted such a change.) -- from: http://lists.boost.org/Archives/boost/2011/02/176638.php -- On Fri, Feb 4, 2011 at 5:43 PM, Joel de Guzman <joel@boost-consulting.com> wrote:
On 2/5/11 6:03 AM, Alexander Nasonov wrote:
As far is a am concerned, I still find this syntax overly verbose. I do realize though that this is just a toy example. For better comparison, Here is how the same thing would like in Boost.Phoenix:
Steven and I were playing with different syntaxes few years ago
We came up with something like this:
void BOOST_LOCAL_FUNCTION ( BOOST_BIND((factor)(&sum)), double num ) { sum += factor * num; std::clog<< "Summed:"<< sum<< std::endl; } BOOST_LOCAL_FUNCTION_DECL(add)
http://thread.gmane.org/gmane.comp.lib.boost.devel/168612/focus=168694
That is nice! -- end --
1) simple local functions --for those, you don't want the complexity of using a macro DSEL, just use plain functions and function objects. 2) Moderate to complex programs --for those, the macro syntax will get in the way of clarity of code -- for those, just use plain functions and function objects.
IMO, what is moderate or complex programs and which tool to use to program them (Boost.Local, Phoenix, Lambda, etc) is a decision that should be left to our library users.
And I am not even mentioning c++ lambda. You can nitpick all you want, but it's still nitpicking. Nitpicks are not valid rationale for justifying a library for boost.
I am sorry but I don't understand this. Can you please re-word it? Thanks a lot. --Lorenzo
On 11/19/2011 6:08 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 11:23 PM, Joel de Guzman To me and others this does not look arcane but I understand this is a subjective.
BTW, why would you consider the above syntax arcane and the one below "nice"? (I am honestly asking such a question, it is not that people cannot change their minds, there would be no improvement otherwise, but if that is the case I would sincerely like to know what prompted such a change.)
[snip]
As far is a am concerned, I still find this syntax overly verbose.
I do realize though that this is just a toy example. For better comparison, Here is how the same thing would like in Boost.Phoenix:
Steven and I were playing with different syntaxes few years ago
We came up with something like this:
void BOOST_LOCAL_FUNCTION ( BOOST_BIND((factor)(&sum)), double num ) { sum += factor * num; std::clog<< "Summed:"<< sum<< std::endl; } BOOST_LOCAL_FUNCTION_DECL(add)
http://thread.gmane.org/gmane.comp.lib.boost.devel/168612/focus=168694
That is nice!
Pardon me for the confusion. No change of mind here. If I were allowed to reword that I would say "nicer". It's still too noisy. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Sat, Nov 19, 2011 at 5:17 AM, Joel de Guzman <joel@boost-consulting.com> wrote:
As far is a am concerned, I still find this syntax overly verbose.
I do realize though that this is just a toy example. For better comparison, Here is how the same thing would like in Boost.Phoenix:
Steven and I were playing with different syntaxes few years ago
We came up with something like this:
void BOOST_LOCAL_FUNCTION ( BOOST_BIND((factor)(&sum)), double num ) { sum += factor * num; std::clog<< "Summed:"<< sum<< std::endl; } BOOST_LOCAL_FUNCTION_DECL(add)
http://thread.gmane.org/gmane.comp.lib.boost.devel/168612/focus=168694
That is nice!
Pardon me for the confusion. No change of mind here. If I were allowed to reword that I would say "nicer". It's still too noisy.
Oh but you are allowed! Thanks a lot for the clarification. --Lorenzo
On 11/19/2011 6:08 PM, Lorenzo Caminiti wrote:
On Fri, Nov 18, 2011 at 11:23 PM, Joel de Guzman
Why can't we C++ programmers just accept the fact that we don't have *true* local functions like those in Pascal and Modula?
Because now we have Boost.Local so we can program true local functions with all their functionality :) (If not, which local function functionality is missing from Boost.Local?)
It's very different. See, for example: function E(x: real): real; function F(y: real): real; begin F := x + y end; begin E := F(3) end; There's no need to "bind" x. F can see x. That's the feature I want to see in a local function that *cannot* be done in any way in C++. *True* local functions should not require you to bind a local variable to make it visible in a nested function. Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Hi all, Hello Thomas and thank you very much for your review.
Following is my Review of the upcoming Boost.Local library. I will as others split my review into the three parts that make up this library: local functions, local blocks, scoped exit.
Let me first start with some general comments.
nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just Which part of the documentation are you referring too? After both syntaxes are introduced:
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/index.html...
The rest of the docs just explain the functionality. Even if I provide all examples in both syntaxes, I don't think it distracts the user-- do you? Can you point out the part of the documentation that are confusing for the user because of the "double" syntax? (Maybe I can improve them.)
stick with one and concentrate on the functionality? Because having the sequencing syntax comes at no maintenance or implementation cost and it allows to use the library on compilers without variadic support. I agree that it will be better if the documentation used the variadic macro forms and introduce the sequence form as a workaround, as I suggested in my review.
Local Functions:
The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter. Others have made the same comments (with you a total of 3 people). However, others have made the opposite comment (2 liked default, 5+ didn't say anything about it). In my experience, this quickly becomes a non-issue after the 1st week that you start using the library. I will accept that Boost.Local doesn't default parameters, as I don't
Le 18/11/11 23:44, Lorenzo Caminiti a écrit : think default parameter to be of high value for local functions.
I would consider binding variables from the enclosing scope one of the major weak points of Boost.Local. The need to binding those variables is obvious. However, how it is done clutters the arguments list of the BOOST_LOCAL_FUNCTION_PARAMS to an extend that it is not immediately clear what the arity of the local function is. It would make things clearer to have something like: BOOST_LOCAL_FUNCTION_PARAMS(...) BOOST_LOCAL_FUNCTION_CAPTURE(...) You are the first one to make this comment. I think that if you follow the convention to list the function parameters first and the binds last, you can easily see the function arity (at least that is my experience). The library does not enforce this order, but that is the convention I use in the docs. With that you look at the parameters from left to right until you find an parameter that said `bind` and there it is the function arity. I was thinking also about that, but I didn't see how the split could be done. Thinking a little bit more on this issue, what about using a keyword 'capture/bind' to split them
BOOST_LOCAL_FUNTION(T1 p1, ..., Tn pn, capture, v1, ... vn) Could this syntax be implemented?
Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations. I don't think so... here bind is clearly used in a different context. 'capture' seams OK to me. See above.
WRT to your note in the documentation that C++11 can not bind const reference. I suggest the following workaround:
int i = 0; auto f = [&](){ auto const& icr = i; ... }; Yes, I will add that to the docs ad suggested by Vicente. This has the down side of (1) requiring to use a different name for the constant variabe `icr` and (2) write the extra code to declare the const variable. I agree that these are both minor points (but in some specific domains like Contract Programming however (1) is a big issue because you will have to explain the user that they need to use the magic name `icr` to refer to the variable in programming their assertions).
I agree that this and _this should be used always, instead of having both _this and this. Yes, I will always use this_ (because it's a keyword in the context where it is used in Boost.Local).
Even if this_ is not a placeholder, I will accept that Boost.Local uses _this, to be more homogeneous.
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does). I am sure there are lots of people that will never use a local blocks. That said, I personally needed to program (to ensure the constant-correctness requirement of assertions in Contract Programming):
int x = 0; const { assert( x == 0 ); }
You can do that with local blocks:
int x = 0; BOOST_LOCAL_BLOCK(const bind x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END
As we said, using lambdas or other Boost libraries there are other ways to do that but in my case I needed to keep the `assertion( x == 0 )`;` unchanged (including the variable name `x`) and these other ways (including C++11 lambdas) don't allow for that.
More in general, local blocks allow you to specify the sematic that a variable should be thread as const by the next bunch of instruction. I personally find that useful, others have made the same comment (Vicente, Pierre, Jeff, and another couple of people-- see their review to Boost.Local). Local blocks could be included internally to Boost.Contract as this has no general use.
Local Exit: Why do we need two libraries doing the exact same thing with neither of the adding any functionality. I suggest to improve ScopedExit instead. I agree. The ScopeExit improvements alone are not sufficient to motivate Boost.Local (I'd just add const bindings and this binding to ScopeExit for that). Boost.Local is about local functions, the improved scope exits (i.e., local exits) should be somehow merged with the existing ScopeExit (in fact that's a question to the reviewers).
I will also accept to maintain separated Boost.Local and Boost.ScopedExit with the accepted improvements. The macros needed in both libraries could be located in a specific detail directory. Best, Vicente
On Sat, Nov 19, 2011 at 11:59 AM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:
Le 18/11/11 23:44, Lorenzo Caminiti a écrit :
On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller <thom.heller@googlemail.com> wrote:
Hi all,
Hello Thomas and thank you very much for your review.
Following is my Review of the upcoming Boost.Local library. I will as others split my review into the three parts that make up this library: local functions, local blocks, scoped exit.
Let me first start with some general comments.
nightmare. Also, reading through the documentation, most of the time is spent on explaining, the difference between the two syntaxes. Why not just
Which part of the documentation are you referring too? After both syntaxes are introduced:
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/index.html...
The rest of the docs just explain the functionality. Even if I provide all examples in both syntaxes, I don't think it distracts the user-- do you? Can you point out the part of the documentation that are confusing for the user because of the "double" syntax? (Maybe I can improve them.)
stick with one and concentrate on the functionality?
Because having the sequencing syntax comes at no maintenance or implementation cost and it allows to use the library on compilers without variadic support.
I agree that it will be better if the documentation used the variadic macro forms and introduce the sequence form as a workaround, as I suggested in my review.
Yes, I will leave just the variadic macro syntax throughout the docs and examples while mentioning the sequencing syntax only in an Annex.
Local Functions:
The default parameter syntax is confusing, it looks like it is a completely new parameter, and not belonging to the previous argument. This gets confusing once you have more than one parameter with a default parameter. The WITH_DEFAULT macro should be the default, as it clearly states the relationship of the default value to the parameter.
Others have made the same comments (with you a total of 3 people). However, others have made the opposite comment (2 liked default, 5+ didn't say anything about it). In my experience, this quickly becomes a non-issue after the 1st week that you start using the library.
I will accept that Boost.Local doesn't default parameters, as I don't think default parameter to be of high value for local functions.
I'd be OK with removing default parameter support from Boost.Local because they are not a very important feature for local functions. However, I would personally prefer to leave default parameter there and if a user doesn't want to use their ", default" syntax, he/she will simple don't use default parameter (this way if a user really has to use a default parameter, he/she has the option to do so). I will of course comply with the final decision from the review.
I would consider binding variables from the enclosing scope one of the major weak points of Boost.Local. The need to binding those variables is obvious. However, how it is done clutters the arguments list of the BOOST_LOCAL_FUNCTION_PARAMS to an extend that it is not immediately clear what the arity of the local function is. It would make things clearer to have something like: BOOST_LOCAL_FUNCTION_PARAMS(...) BOOST_LOCAL_FUNCTION_CAPTURE(...)
You are the first one to make this comment. I think that if you follow the convention to list the function parameters first and the binds last, you can easily see the function arity (at least that is my experience). The library does not enforce this order, but that is the convention I use in the docs. With that you look at the parameters from left to right until you find an parameter that said `bind` and there it is the function arity.
I was thinking also about that, but I didn't see how the split could be done. Thinking a little bit more on this issue, what about using a keyword 'capture/bind' to split them
BOOST_LOCAL_FUNTION(T1 p1, ..., Tn pn, capture, v1, ... vn)
Could this syntax be implemented?
Yes but it does not support binding by const/ref one variable but not another and specifying the type for the bind (instead of using typeof). This could be done (using capture, or bind, or any other word we wish): BOOST_LOCAL_FUNCTION(T1 p1, ..., Tn pn, capture(v1, &v2, const v3, const &v4, (int)v5)) I personally don't find this more readable than the existing Boost.Local syntax (especially when the type is explicitly specified like for v5). I'll comply with the decision from the review.
Additionally as binding is probably the right term to use here, it might confuse people with the already existing solutions of the various bind implementations.
I don't think so... here bind is clearly used in a different context.
'capture' seams OK to me. See above.
WRT to your note in the documentation that C++11 can not bind const reference. I suggest the following workaround:
int i = 0; auto f = [&](){ auto const& icr = i; ... };
Yes, I will add that to the docs ad suggested by Vicente. This has the down side of (1) requiring to use a different name for the constant variabe `icr` and (2) write the extra code to declare the const variable. I agree that these are both minor points (but in some specific domains like Contract Programming however (1) is a big issue because you will have to explain the user that they need to use the magic name `icr` to refer to the variable in programming their assertions).
I agree that this and _this should be used always, instead of having both _this and this.
Yes, I will always use this_ (because it's a keyword in the context where it is used in Boost.Local).
Even if this_ is not a placeholder, I will accept that Boost.Local uses _this, to be more homogeneous.
Local Blocks: I can't see the value in that one. The example in the documentation doesn't help either, as those things are easily detected by any modern compiler, with the appropriate warning levels (ok, not in the assert case, but for other boolean contexts at least gcc does).
I am sure there are lots of people that will never use a local blocks. That said, I personally needed to program (to ensure the constant-correctness requirement of assertions in Contract Programming):
int x = 0; const { assert( x == 0 ); }
You can do that with local blocks:
int x = 0; BOOST_LOCAL_BLOCK(const bind x) { assert( x == 0 ); } BOOST_LOCAL_BLOCK_END
As we said, using lambdas or other Boost libraries there are other ways to do that but in my case I needed to keep the `assertion( x == 0 )`;` unchanged (including the variable name `x`) and these other ways (including C++11 lambdas) don't allow for that.
More in general, local blocks allow you to specify the sematic that a variable should be thread as const by the next bunch of instruction. I personally find that useful, others have made the same comment (Vicente, Pierre, Jeff, and another couple of people-- see their review to Boost.Local).
Local blocks could be included internally to Boost.Contract as this has no general use.
Local blocks are trivial to implement once you have local function so I'm also OK removing them from the library: int x = 0; int y = x; void BOOST_LOCAL_FUNCTION(const capture& x, const capture& y) { assert( x == y ); } BOOST_LOCAL_FUNCTION_END(block1) block1(); That said I would personally prefer to leave local blocks in the library so a user has them "out of the box" if he/she needs them. I'll of course comply with the decision from the review.
Local Exit: Why do we need two libraries doing the exact same thing with neither of the adding any functionality. I suggest to improve ScopedExit instead.
I agree. The ScopeExit improvements alone are not sufficient to motivate Boost.Local (I'd just add const bindings and this binding to ScopeExit for that). Boost.Local is about local functions, the improved scope exits (i.e., local exits) should be somehow merged with the existing ScopeExit (in fact that's a question to the reviewers).
I will also accept to maintain separated Boost.Local and Boost.ScopedExit with the accepted improvements. The macros needed in both libraries could be located in a specific detail directory.
That sounds good to me. Thanks a lot. --Lorenzo
On Nov 19, 2011, at 4:49 AM, Thomas Heller wrote:
For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions.
In no way are Boost.Bind, Boost.Lamda, and Boost.Phoenix equivalent to Boost.Local in functionality in practice. In Boost.Local, you have to learn some new (relatively straightforward) syntax for declaring the local function, but the function body is plain C++. For the other libraries by contrast you have to learn a completely new syntax for writing *all* of the code in your function, and if you get something wrong you get literally pages of error messages that give you no insight as to what happened. The learning curve for Boost.Local is lightyears ahead of the competitors. I personally don't like the verbosity of Boost.Local so I often prefer to start by using Boost.Lambda instead for small functions instead, but I can't tell you how many times I have eventually decided to give up and just use Boost.Local because it was becoming a waste of my time to figure out what the obscure error messages were telling me about what went wrong. Cheers, Greg
For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions.
In no way are Boost.Bind, Boost.Lamda, and Boost.Phoenix equivalent to Boost.Local in functionality in practice. In Boost.Local, you have to learn some new (relatively straightforward) syntax for declaring the local function, but the function body is plain C++. For the other libraries by contrast you have to learn a completely new syntax for writing *all* of the code in your function, and if you get something wrong you get literally pages of error messages that give you no insight as to what happened. The learning curve for Boost.Local is lightyears ahead of the competitors.
I personally don't like the verbosity of Boost.Local so I often prefer to start by using Boost.Lambda instead for small functions instead, but I can't tell you how many times I have eventually decided to give up and just use Boost.Local because it was becoming a waste of my time to figure out what the obscure error messages were telling me about what went wrong.
Sorry, I still don't get it - so please excuse my ignorance. What exactly is missing from C++11 lambdas for them to be useful as local functions? void foo() { auto f = [](){ cout << "Hey, I'm a local function!\n"; } f(); } Why do we need Boost.Local instead? Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu
On Mon, Nov 21, 2011 at 10:44 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions.
In no way are Boost.Bind, Boost.Lamda, and Boost.Phoenix equivalent to Boost.Local in functionality in practice. In Boost.Local, you have to learn some new (relatively straightforward) syntax for declaring the local function, but the function body is plain C++. For the other libraries by contrast you have to learn a completely new syntax for writing *all* of the code in your function, and if you get something wrong you get literally pages of error messages that give you no insight as to what happened. The learning curve for Boost.Local is lightyears ahead of the competitors.
I personally don't like the verbosity of Boost.Local so I often prefer to start by using Boost.Lambda instead for small functions instead, but I can't tell you how many times I have eventually decided to give up and just use Boost.Local because it was becoming a waste of my time to figure out what the obscure error messages were telling me about what went wrong.
Sorry, I still don't get it - so please excuse my ignorance.
What exactly is missing from C++11 lambdas for them to be useful as local functions?
void foo() { auto f = [](){ cout << "Hey, I'm a local function!\n"; } f(); }
Why do we need Boost.Local instead?
As it was clarified in a couple of other reviews, if you have C++11 lambdas, you'd probably use them instead of Boost.Local (the only plus of Boost.Local is that you don't need extra variables to bind by const but that's a minor advantage). If instead you need to write local functions that will work on both C++03 and C++11, you can use Boost.Local and on C++11 the local functions programmed with Boost.Local will have the same performance as lambdas. --Lorenzo
On Mon, Nov 21, 2011 at 10:44 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com> wrote:
For everything else, there already exist a couple of solutions (Boost.Bind, Boost.Lambda and Boost.Phoenix). I would like to see improvements in ease of use in these libraries instead of advertising a completely macro based solutions.
In no way are Boost.Bind, Boost.Lamda, and Boost.Phoenix equivalent to Boost.Local in functionality in practice. In Boost.Local, you have to learn some new (relatively straightforward) syntax for declaring the local function, but the function body is plain C++. For the other libraries by contrast you have to learn a completely new syntax for writing *all* of the code in your function, and if you get something wrong you get literally pages of error messages that give you no insight as to what happened. The learning curve for Boost.Local is lightyears ahead of the competitors.
I personally don't like the verbosity of Boost.Local so I often prefer to start by using Boost.Lambda instead for small functions instead, but I can't tell you how many times I have eventually decided to give up and just use Boost.Local because it was becoming a waste of my time to figure out what the obscure error messages were telling me about what went wrong.
Sorry, I still don't get it - so please excuse my ignorance.
What exactly is missing from C++11 lambdas for them to be useful as local functions?
void foo() { auto f = [](){ cout << "Hey, I'm a local function!\n"; } f(); }
Why do we need Boost.Local instead?
As it was clarified in a couple of other reviews, if you have C++11 lambdas, you'd probably use them instead of Boost.Local (the only plus of Boost.Local is that you don't need extra variables to bind by const but that's a minor advantage).
Yes, IMHO it's not worth creating a full library just for this minor case.
If instead you need to write local functions that will work on both C++03 and C++11, you can use Boost.Local and on C++11 the local functions programmed with Boost.Local will have the same performance as lambdas.
Ok. However this raises a more serious question. Should we as the Boost community still encourage solutions and libraries solely for portability with ancient compilers? I'd say no, but YMMV. Boost will be still around 2, 5, or 10 years from now. What's the utility of adding such a _solely_ backwards oriented library from this POV? Based on your arguments I'd suggest to make (Non-)Boost.Local available somewhere for other people to use and for you to maintain in the long term. You don't have to have it in Boost just for it to be used and useful for others. What's the point in cluttering Boost with this? I'm sure, any review manager interested in keeping Boost healthy and alive would agree with me. Adding each and every library which seems to be cool just for the sake of it - without thinking about the long term consequences - hurts Boost. Badly. But I better shut up on this before I'm going to become wind up too much... Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu
On Mon, Nov 21, 2011 at 8:30 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com>wrote: [...]
Ok. However this raises a more serious question. Should we as the Boost community still encourage solutions and libraries solely for portability with ancient compilers? I'd say no, but YMMV. Boost will be still around 2, 5, or 10 years from now. What's the utility of adding such a _solely_ backwards oriented library from this POV?
Based on your arguments I'd suggest to make (Non-)Boost.Local available somewhere for other people to use and for you to maintain in the long term. You don't have to have it in Boost just for it to be used and useful for others. What's the point in cluttering Boost with this?
I'm sure, any review manager interested in keeping Boost healthy and alive would agree with me. Adding each and every library which seems to be cool just for the sake of it - without thinking about the long term consequences - hurts Boost. Badly.
But I better shut up on this before I'm going to become wind up too much...
Since this indirectly references myself, I'd like to point out that yes, I agree with your overall sentiment: "Adding each and every library which seems to be cool just for the sake of it [...] hurts Boost." To answer your initial question, though ("Should we as the Boost community still encourage solutions and libraries solely for portability with ancient compilers?"), I don't know. You seem to acknowledge that the Boost community, at one time, did encourage libraries solely for portability; indeed, Boost.Move comes to mind, and I think Boost.Atomic (just proposed? or also accepted? I forget) fits there as well. And now, presumably with the advancement of C++11 (at least partially) compilers, you don't think we should. Or, perhaps, the added value that Boost.Local provides is significantly less than that for Boost.Move and Boost.Atomic (and whatever else), hence doesn't meet the bar for acceptance given that it's almost entirely a portability solution. Both of those are, I believe, justifiable opinions. It's fortunate that the quality of the library under consideration has never really been questioned, and, indeed, it has been emphasized even among those who have voted against inclusion. I will have to go back through all the threads (ugh!), but it thus seems that the two primary arguments against inclusion are "Is this actually solving a real problem?" (together with all the discussion about whether the solution that Boost.Local provides is actually better than existing alternatives) and "Does a library providing a portable interface in C++03 for C++11 features belong in Boost?". Regarding the former issue, it *seems* there's been enough actual use to deem the library sufficiently "useful"; but, again, I will have to review the discussion. I'm now thinking, though, that coming to a consensus on this latter issue (together with any qualifications) is too important for me to make a premature ruling on Boost.Local and risk setting an individual-decided precedent. Am I making too big a deal out of this? Or do you think this is misguided? - Jeff
On Mon, Nov 21, 2011 at 8:30 AM, Hartmut Kaiser <hartmut.kaiser@gmail.com>wrote: [...]
Ok. However this raises a more serious question. Should we as the Boost community still encourage solutions and libraries solely for portability with ancient compilers? I'd say no, but YMMV. Boost will be still around 2, 5, or 10 years from now. What's the utility of adding such a _solely_ backwards oriented library from this POV?
Based on your arguments I'd suggest to make (Non-)Boost.Local available somewhere for other people to use and for you to maintain in the long term. You don't have to have it in Boost just for it to be used and useful for others. What's the point in cluttering Boost with this?
I'm sure, any review manager interested in keeping Boost healthy and alive would agree with me. Adding each and every library which seems to be cool just for the sake of it - without thinking about the long term consequences - hurts Boost. Badly.
But I better shut up on this before I'm going to become wind up too much...
Since this indirectly references myself, I'd like to point out that yes, I agree with your overall sentiment: "Adding each and every library which seems to be cool just for the sake of it [...] hurts Boost."
To answer your initial question, though ("Should we as the Boost community still encourage solutions and libraries solely for portability with ancient compilers?"), I don't know. You seem to acknowledge that the Boost community, at one time, did encourage libraries solely for portability; indeed, Boost.Move comes to mind, and I think Boost.Atomic (just proposed? or also accepted? I forget) fits there as well. And now, presumably with the advancement of C++11 (at least partially) compilers, you don't think we should. Or, perhaps, the added value that Boost.Local provides is significantly less than that for Boost.Move and Boost.Atomic (and whatever else), hence doesn't meet the bar for acceptance given that it's almost entirely a portability solution. Both of those are, I believe, justifiable opinions.
Boost.Move is an infrastructure library needed by Boost itself and Boost.Atomics has no MACRO_BASED_INTERFACE. I might sound inconsistent, but it's probably the combination of all three which trips me off: 'solves no real problem', 'just for the sake of portability', and 'MACROS'.
It's fortunate that the quality of the library under consideration has never really been questioned, and, indeed, it has been emphasized even among those who have voted against inclusion. I will have to go back through all the threads (ugh!), but it thus seems that the two primary arguments against inclusion are "Is this actually solving a real problem?" (together with all the discussion about whether the solution that Boost.Local provides is actually better than existing alternatives) and "Does a library providing a portable interface in C++03 for C++11 features belong in Boost?". Regarding the former issue, it *seems* there's been enough actual use to deem the library sufficiently "useful"; but, again, I will have to review the discussion. I'm now thinking, though, that coming to a consensus on this latter issue (together with any qualifications) is too important for me to make a premature ruling on Boost.Local and risk setting an individual-decided precedent.
Am I making too big a deal out of this? Or do you think this is misguided?
Jeff, I fully trust you will make the right decision, and no, you don't make a big deal out of nothing, IMHO. Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu
Hartmut Kaiser wrote:
What exactly is missing from C++11 lambdas for them to be useful as local functions?
void foo() { auto f = [](){ cout << "Hey, I'm a local function!\n"; } f(); }
Why do we need Boost.Local instead?
I've looked a a few posts on this thread and I think I see what the issue is. a local function is something that almost any C program can understand without embarking on a significant amount of study to understand a whole new syntax. The functionality is transparent and verifiable to anyone who looks it. This is true even though its more verbose. (or maybe its true BECAUSE it's more verbose.) When I see auto f = [](){ cout << "Hey, I'm a local function!\n"; } Now I have to embark upon a mini-research project to learn of bunch of new stuff that I didn't know before in order to do something that could easily be done the "old fashioned" way. OK you say I should bone up on the new stuff anyway. Maybe, but in reality there's a couple of other things going on. a) in my company we might not be using a compiler which supports these latest features. b) I maybe under huge pressure to address something before business opens tomorrow (this is common for me). So I don't have time for an un-anticipated diversion. c) When In insert something like this, it will eventually get reviewed by other programmers who really need to be able to understand something right away when they look at it. So I don't seen any real conflict between the local function lambda one. I've I'm crafting the next greatest boost library where I have as much time as necessary to get it exactly optimal I might use one while under other circumstances I might prefer something that everyone can swallow without problem. Robert Ramey
On 11/19/2011 2:49 AM, Thomas Heller wrote:
- What is your evaluation of the documentation?
The documentation is very good!
Despite my criticisms, I have to agree with Thomas. The documentation is very good and is one of the best I've seen for a long time here in Boost. Regards, -- Joel de Guzman http://www.boostpro.com http://spirit.sf.net
participants (8)
-
Gregory Crosswhite -
Hartmut Kaiser -
Jeffrey Lee Hellrung, Jr. -
Joel de Guzman -
Lorenzo Caminiti -
Robert Ramey -
Thomas Heller -
Vicente J. Botet Escriba