September 2012 - Boost - lists.preview.boost.org

[Review:Contract] Very late review
by John Bytheway 15 Sep '12

15 Sep '12

I was out of touch for most of the review period, but I wanted to submit a review of Contract anyway. The review manager is of course welcome to ignore my comments or otherwise give them short shrift as he sees fit as a consequence of this delay, but I hope Lorenzo will welcome more feedback :). Firstly, I would like to add my voice to those who are congratulating Lorenzo on the quite remarkable feat this library represents. It has obviously required extensive and careful thought and effort … [View More]to reach this state. It is primarily because of this evident conscientiousness that I will vote YES, I think the library should be accepted into Boost. I concur with previous reviewers that it is unlikely to suit industrial use, but it is an important experiment, and one which will benefit from the additional exposure granted by inclusion in Boost. My review follows. Problems using the library ========================== I only experimented very briefly, but found one problem (which may well be a clang bug, but you should probably investigate). The following program I tried works with gcc-4.7.1 but not with clang r163674 (both in C++11 mode): #include <contract.hpp> CONTRACT_FUNCTION( int (postinc) ( (int&) value ) precondition( value < std::numeric_limits<int>::max() ) ) { return value++; } int main() { int i = std::numeric_limits<int>::max(); postinc(i); return 0; } The first few errors with clang all look like: In file included from contract-failure-test.cpp:1: In file included from .../contractpp_0_4_1/include/contract.hpp:14: .../contractpp_0_4_1/include/contract/broken.hpp:359:7: error: call to object of type 'thread_variable<broken_contract_handler>' is ambiguous { aux::broken_handlers<>::precondition(context); } ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ .../contractpp_0_4_1/include/contract/aux_/thread_variable.hpp:42:5: note: conversion candidate of type 'void (*)(const contract::from &)' operator T() const { // Sync so must return object copy (by value). ^ .../contractpp_0_4_1/include/contract/aux_/thread_variable.hpp:49:5: note: conversion candidate of type 'void (*)(const contract::from &)' operator T() const volatile { // Sync so must return object copy (by value). ^ Minor issues in the docs ======================== (I'm not mentioning issues already raised in another review) Tutorial -------- - I think it's slightly unfortunate that your example function name ("postinc") shares a four-character prefix with "postcondition". This threw me a couple of times when I was reading too fast. - "specify both its type and its name (parameter names can instead by omitted in usual C++ declarations)" -- I found this phrasing confusing. I think you were planning to remove this constraint anyway so it doesn't matter, but FWIW I would recommend something like "specify both its type and its name (unlike conventional C++ declarations, where parameter names may by omitted)". - "and assigning it to the return keyword" -- also confusing. Perhaps "and assigning to it from the return keyword" - "destructors cannot specify neither preconditions nor postconditions" -- double negative; should be "destructors can specify neither preconditions nor postconditions" - The whole "Forward Declarations and Body Definitions" section is a bit odd. The code example doesn't seem to match up with the text. For example, you say "less and natural::get are defined together with their declarations", but in fact less is forward-declared at the top of the file with no definition. Advanced topics --------------- - "the postcondition result = n * factorial(n - 1) has factorial computation complexity" -- this is not true; the factorial function as defined here has linear complexity, not factorial complexity. (If it were using arbitrary precision integers it would be something like O(n^2 log(n)^2) but still certainly polynomial, not super-exponential!) - "preconditions cannot only be weakened" -- again s/cannot/can/ Named parameters ---------------- - By using "requires" here in the pseudocode example with a different meaning from the rest of the library you are further muddying the waters. If you change the "requires" keyword that is used by the library to something else (as discussed below) then this will no longer be a problem. - In footnote 61 (Boost.Parameter bug) could you please link to the bug so that future readers will know when it is fixed. Doxygen ------- You've mentioned a few times the the Doxygen preprocessor can't cope with your preprocessor metaprogramming. Have you submitted bugs to Doxygen? Do you know how receptive they are to improving their preprocessor conformance? This library might be a compelling incentive for them to do so. Design issues ============= The vast majority of the design is clearly well-grounded in existing practice and practicality. Nevertheless, I have a few issues to raise. Concepts -------- I agree with the earlier thread that you should not use the term "requires" in the way you currently are, because it has a different meaning in the Concepts proposal. Alternatives as discussed there such as "check" would be an improvement. Contract broken message ----------------------- The above example program generates the following broken contract report: precondition number 1 "value < std::numeric_limits<int>::max()" failed: file "contract-failure-test.cpp", line 8 I would like my editor (Vim, though this comment applies equally to others) to automatically take me to the line where the failure occurred. I could of course configure it appropriately, but it might make your users lives easier if you used a standard error format which editors will recognize by default, such as: contract-failure-test.cpp:8:precondition number 1 "value < std::numeric_limits<int>::max()" failed Furthermore, I suspect that it is quite likely that these contract broken messages might occur at odd times, such as when the program is in the midst of printing something else to stdout/stderr, so it might be a good idea to prefix all your messages with a newline. Best of all, perhaps, would be to provide a customization point for the message format (which doesn't entail changing all five broken handlers). This would allow me to make these changes to the messages without disturbing other users. Contract broken handlers ------------------------ Based on your example, I presume that if I install a custom handler for broken contracts, and that handler returns without throwing, then execution will simply continue merrily without regard to the detected contract violation. I can see why you might feel the need to provide such functionality ... but my first instinct is that if a custom handler returns the library should take over and call std::terminate anyway (this is similar to the way that a constructor function-try block will not allow you to swallow an exception thrown by a base or member constructor). But I don't feel strongly about this. At the least, you should document this turn of events more clearly; I only figured it out from the very last footnote in advanced topics. Configuration macros -------------------- This is my biggest concern with the library at present. You provide the various macros for configuration (CONTRACT_CONFIG_NO_PRECONDITIONS etc. as in <contract/config.hpp>). However, they are all essentially global constants. Suppose I am writing an application that depends on a library, and they both use contracts in their declarations. I would like to have only preconditions active for the library, but all contracts checked for the application. There is no way to achieve this. CONTRACT_CONFIG_DO_NOT_SUBCONTRACT_PRECONDITIONS causes a similar but different problem. If the library subcontracts preconditions then I can't disable them for my application. I think you need to provide a means to turn the various features on and off on a per-project basis. As one suggestion, suppose that you took extra arguments to your macros, so that the above example became: CONTRACT_FUNCTION( MYAPP_NO_PRECONDITIONS, MYAPP_NO_POSTCONDITIONS, int (postinc) ( (int&) value ) precondition( value < std::numeric_limits<int>::max() ) ) { return value++; } and then I define a wrapper macro for my app which substitutes these arguments for me, and therefore the example becomes: MYAPP_CONTRACT_FUNCTION( int (postinc) ( (int&) value ) precondition( value < std::numeric_limits<int>::max() ) ) { return value++; } Or perhaps rather than many variables it would be better to have a single MYAPP_CONTRACT_CONFIG value which contained all of them (as a preprocessor tuple, say) which was passed in similarly. Anyway, I'm sure you can think of something appropriate. Philosophical issues ==================== Finally, I'd like to go off on a bit of a tangent and talk about a philosophical problem with the design. It has been touched on by other reviewers but I'd like to go into more detail. Your library design is very syntactically invasive. As the docs indicate, this makes it unfamiliar, slow to compile, and potentially difficult to debug. But none of these is the worst problem. To worst problem is that only one library can do this. You've already seen this, and done your best to integrate the other Boost libraries that have similar requirements (Parameter and Concept) but this approach doesn't scale. I'm sure at least one of the many reflection libraries proposed to Boost does something similar. Will you integrate that too? What about non-Boost libraries. The only scalable solution is to write a framework for customized definitions. You've mentioned the possibility of using the knowledge your alternate syntax has to expose extra data (e.g. function access level) at preprocessor time. But that's not enough. It would have to allow other library authors to inject code into definitions in the way Contract or Parameter needs to; even provide whole new definition syntaxes as Property does. Given such a framework, each other library (Contract, Parameter, a reflection library, and whatever else) would be written using the framework. It should then be possible to use whichever a user desires, at the same time, for the same declaration, without those libraries having any knowledge of each other. Such a framework would be akin to the language customization features that some other languages have (and which have also been mentioned in other threads of this review). To be clear, I am certainly not demanding, nor even proposing, that you implement such a framework. I think it would be extremely difficult -- more trouble than it's worth. It would also be very dangerous (for all the same reasons that it is dangerous to be able to customize the language on the fly). So long as Contract remains a primarily experimental library the current design is perfectly adequate. I just think that this direction is the logical conclusion of your efforts, and wanted to bring it to your attention (though you've probably already had similar thoughts). Standard review questions ========================= * What is your evaluation of the design? Barring issues mentioned above, it is an excellent attempt to solve a very difficult problem. * What is your evaluation of the implementation? I did not examine it. * What is your evaluation of the documentation? I did not read the reference; the rest was very good, with minor nits as above. * What is your evaluation of the potential usefulness of the library? It has significant potential for use in small projects, to spread the idea of contract programming, serve as a proof of concept and influence future standardisation efforts. It is unlikely to see use in large scale industrial programming. * Did you try to use the library? With what compiler? Did you have any problems? Very briefly, with compilers and problems mentioned above. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I spent three evenings reading the docs and writing my review, and have been following the development of the library on the list since its first appearance. I would have liked to do more experiments, but my review was already late enough! * Are you knowledgeable about the problem domain? Not to any notable degree. Congratulations again to Lorenzo on achieving something I would have thought impossible; with luck it might inspire me to actually try contract programming. John Bytheway [View Less]

2 1

[preprocessor] automatic recursion curiosity
by Lorenzo Caminiti 14 Sep '12

14 Sep '12

Hello all, I have a curiosity about an implementation detail of Boost.Preprocessor: How does the lib implement Automatic Recursion? >From the docs under Topic > Reentrancy > Automatic Recursion: http://www.boost.org/doc/libs/1_51_0/libs/preprocessor/doc/index.html `` Using automatic recursion to reenter various sets of macros is obviously much simpler. It completely hides the underlying implementation details. So, if it is so much easier to use, why do the state parameters still … [View More]

2 1

[Review.Coroutine] Vicente's review
by Vicente J. Botet Escriba 13 Sep '12

13 Sep '12

Hi, I understand that Oliver is busy as all of us and that it is better to provide first the minimal and well designed scope he is confident with. I expect however that Oliver will add soon symmetric coroutines based on the work done by Giovanni in a near future. - What is your evaluation of the design? I have some suggestions about changing the interface, but I'm not sure all of them could be implemented in an efficient way. * The use of thecoro_t::self_t & self parameter makes … [View More]too much noise. This parameter must be forwarded to any function that could yield the coroutine. An alternative design using a thread specific pointer could allow to access the current coroutine int f20() { typedef this_coroutine<int(void)> this_coro; this_coro::yield(5); } Is the rationale for using self_t to get better performances? Maybe both interfaces could be provided. * There is an asymmetry between the value returned the last time and the others. self.yield( i); // OTHERS return i; // LAST This asymmetry forces the user to manage the last return in a specific case, changing the readability of the code. I would prefer coroutine function returns void. voidfn( coro_t::self_t & self) { self.yield(1); self.yield(2); } * If this is not adopted, the implementation should check at compile time that the return type of the coroutine function is the one of the signature. * The access to the actual coroutine parameters is asymmetric, letting access to old actual parameters that can point to objects that have already been destroyed. Maybe the self_t object could take care of this (or this_coroutine). We can use a get<> function to give access to the actual parameters. int f20(coro_t::self_t & self) { self.yield(2*self.get<0>()); } int f20() { typedef this_coroutine<int(int)> this_coro; this_coro::yield(2*this_coro::get<0>()); } Or even use some kind of bind parameter int f20(coro_t::self_t & self) { int i; int j; self::bind(i, j); self.yield( i + j ); } int f20() { int i; int j; typedef this_coroutine<int(int)> this_coro; this_coro::bind(i, j); this_coro::yield( i + j ); } If bind is adopted, the implementation could use it to store directly the actual parameters, instead of needing to store them in the self_t instance and them returning them when yield is called. */As// Boost.Coroutine is able to transfer user exceptions, and there is no difference between calling to coroutine<>::self_t::yield_break()/ and throwing/coroutine_terminated,/both (/coroutine<>::self_t::yield_break()/ and throwing/coroutine_terminated)/be removed from the library. The same applies for generators. * noexcept specifier should be added to class coroutine { // ... coroutine() BOOST_NOEXCEPT; coroutine( coroutine && other) BOOST_NOEXCEPT; coroutine & operator=( coroutine && other) BOOST_NOEXCEPT; explicitoperator bool() const BOOST_NOEXCEPT; bool is_complete() const BOOST_NOEXCEPT; //... }; void swap( coroutine & l, coroutine & r) BOOST_NOEXCEPT; The same applies to generator. * Type-erase on the Allocator parameter as the standard std::packaged_task does. * Make coroutine/generator classes to follow the allocator aware standard protocol adding the constructor with the standard allocator. A specializations of the use_allocator template should be added for this purpose. * Moving some of the coroutine/generator optional parameters to a coro::attributes class could simplify the interface std::size_t size = ctx::default_stacksize(), flag_unwind_t do_unwind = stack_unwind, bool preserve_fpu = true, * The lib shouldn't throw when preconditions are not satisfied, assert should instead. * It is a shame that the library can not support completely move semantics for the coroutine parameters, as the used boost::tuple don't support it yet. I would like this point be addressed before the first release so that there is no break in the interface when this will be solved either using boost::tuple, boost::fusion::tuple or std::tuple. At a last resort the library could either define a macro that allows to parameterize the tuple implementation, or alternatively it could implement a wrapper that forwards to the best implementation. * As others have noted, providing a view of a generator as an output iterator will be welcome. Viewing it as a range will be also appreciated. BTW, I don't like the alternative using optional as the result of a generator. Following the output iterator concept the access is defined only if the generator has not finished. - What is your evaluation of the implementation? I have not a deep opinion as I have not found the time to inspect it. At first glance, it seems that the inheritance hierarchy is deeper than needed and the use of the pre-processor to manage with variadic templates hides a possible good design (as it is usual in those cases). Maybe now that most of the c++ compilers support them, using the preprocessor is less imperative. - What is your evaluation of the documentation? I find the documentation a little bit sparse and contain a lot or typos and minor errors. I would appreciate if the original Boost.Coroutine documentation from Giovannii were not lost, at least for Motivation and Tutorial parts. I would appreciate also if the documentation include the rationale to don't include symmetric coroutines and show how the current library can be used to solve the problems the missing features provide, why not as examples. * showing the signature in the documentation will be more informative. template< typename Signature, typename Allocator = ctx::stack_allocator >class coroutine; template< typename R, typename ...ArgTypes, typename Allocator > class coroutine<R(ArgTypes...)> { public: class self_t { public: tuple<ArgTypes...>yield( R); void yield_break(); }; * documentation for the following constructor is missing in the reference template< typename Fn > coroutine( Fn && fn, std::size_t size = ctx::default_stacksize(), flag_unwind_t do_unwind = stack_unwind, bool preserve_fpu = true, Allocator alloc = Allocator() ); *The example typedef boost::coro::coroutine< void(int) > coro_t; void f( coro_t::self_t & self, int i) { ... self.yield(); ... } coro_t c( boost::bind( f, _1) ); c( 7); doesn't compiles. It should be something like coro_t c( boost::bind( f, _1, _2) ); BTW, I don't see the need to use boost::bind in this case, as coro_t c( f ); works as well. The superfluous uses of bind in the documentation should be removed: - What is your evaluation of the potential usefulness of the library? Very useful. coroutines and generators are some useful constructions that are missing in the language. - Did you try to use the library? With what compiler? Did you have any problems? Yes. I used gcc-4.2.1,4.6.0, 4.7.0,clang2.9 andclang-3.0 on darwin. I had no problem running the provided examples and tests. I have written some examples. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I have read the documentations of Oliver and Giovanii libraries, followed the review and used the library. - Are you knowledgeable about the problem domain? From the theoretical point of view, yes. Never used in practice. Finally, my vote is YES. Even if the current interface has some drawbacks that should be mentioned in the documentation, I think I Boost.Coroutine is already useful and should be accepted into Boost. Best, Vicente P.S. Maybe I have forgotten to include some of the remarks I did on preceding posts. [View Less]

4 11

[optional] using gdb to inspect optional value
by Pedro Larroy 13 Sep '12

13 Sep '12

Hi Is there a way to inspect an optional with gdb? I had to remove the usage if boost::optional as it caused me trouble to inspect the value. Thanks. Pedro.

2 1

[asio] New feature: timeout for poll()
by Max Che 13 Sep '12

13 Sep '12

Hello. My previous letter contained errors. Here is another letter. I want to use win_iocp_io_service::poll() with timeout. poll() + timeout => poll() functional and free waitable timer! GetQueuedCompletionStatus() (and epoll(), kqueue()) allows to use that. Can anyone add (restore) this feature?

1 0

[shared_ptr] Patch adding weak_use_count and weak_from_this
by Brent Lewis 12 Sep '12

12 Sep '12

These are useful for debugging, similarly to shared_ptr::use_count. Specifically, you can check for dangling pointers by replacing ordinary pointers with weak pointers, and checking the count inside a destructor. The patch is for boost 1.49 and is attached.

1 0

[build] 1.49 unable to bootstrap boost
by yuri kilochek 12 Sep '12

12 Sep '12

Not sure if this goes here. On Windows 7, when I try to do bootstrap on a freshly unzipped boost 1.49 at the root directory, I get the following output: Building Boost.Build engine But that is as far as it gets before os terminates whatever it is doing with this (roughly translated from russian): **** STACK SIZE EXCEEDED **** **** WHILE PERFORMING RECURSION IN … [View More]

4 3

[context] Linux x64 / printf
by Daniel Larimer 12 Sep '12

12 Sep '12

I modified the 'jump.cpp' example and added the line marked CRASH in f1() #include <cstdlib> #include <cstring> #include <iostream> #include <vector> #include <boost/assert.hpp> #include <boost/context/all.hpp> namespace ctx = boost::ctx; ctx::fcontext_t fcm, fc1, fc2; void f1( intptr_t) { std::cout<<"hello "<<0; // WORKS printf( "hello %d", 0 ); // <<< CRASH std::cout << "f1: entered" << std::endl; std::cout <&… [View More]

4 9

[mpi] scatterv, gatherv and Boost.MPI status
by Alain O Miniussi 11 Sep '12

11 Sep '12

Hi, Despite the fact that https://svn.boost.org/trac/boost/ticket/5292 has been sitting there for some time, the documentations still states that MPI_Allgatherv => most uses supported by all_gather (from 1.51.1). It seems the ticket wasn't commited nor refused. Is there a reason why a c++ MPI_gatherv equivalent is useless in Boost.MPI ? Overall, there seems to be a few other tickets in the same situation in the mpi lib. Which raises another question: what is the exact status of Boost.MPI ?… [View More]

1 0

FW: [Review] Coroutine review starts today, September 3rd, ends September 12th
by Hartmut Kaiser 11 Sep '12

11 Sep '12

This is just a reminder that the review of Oliver's Boost.Coroutine is ending tomorrow (see below for the initial announcement). Please get your reviews in as soon as possible. If you need more time, please let me know. Regards Hartmut --------------- http://boost-spirit.com http://stellar.cct.lsu.edu > -----Original Message----- > From: Hartmut Kaiser [mailto:hartmut.kaiser@gmail.com] > Sent: Monday, September 03, 2012 6:46 AM > To: boost(a)lists.boost.org > Subject: {Review]… [View More]

1 0