Implementation issues comments: > > 2. is_static_visitable description states that this metafunction will > > check whether the specified type is visitable by a static visitor. > > This statement is misleading, cause you do not (and could not ) check > > this. What you check is that type could be visited using > > static_visitable_traits interface. You never mention static_visitor > > here. > > The only way visitation by a static visitor can happen is if the > specified type derives static_visitable directly OR if it appropriately > specializes static_visitable_traits. Accordingly, what > is_static_visitable determines is whether objects of the specified type > derives static_visitable OR if static_visitable_traits is specialized. > > Please elaborate if you feel something is wrong in my approach.
In my interpretation static visitable for wide concept that allows type T to be visited not only by static visitor but any other type. Look onto test_static_visitable test I supplied. > > extractable.hpp > > 2. Extractible is the concept and should contain is_extractable > > check, which is for some reason located in details of extract > > header and named is_directly_extractable (this name even > > contradicts the comment in it's definition: it's either > > directly extractable or through custom traits implementation) > > If you looked more closely, you would notice that the > is_directly_extractable is in a detail namespace. That said, I'm not > sure what you mean when you say the name "contradicts" its comment. > Please explain. Here is your comment in is_directly_extractible implementation // directly-extractable || NOT default-traits So it's not only directly extractible but also ... What I propose is to move it out of detail namespace and make part of concept definition. > > 2. From what I view in this header there are 3 ways to make type T > > work with extract<T> interface > > a) Make T "directly extractible", iow inherit from extractable > > b) Make T "indirectly extractable" by means of specializing > > extractable_traits > > c) Make T visitable by extractor_visitor, by means of specializing > > static_visitable_traits for T > > Regarding a) and b), you are correct. And while your comment in c) is > technically, your wording suggests to me you might not understand > visitation completely. The type T you describe does not support the > given visitor explicitly. Rather it takes visitors via templatized > parameters. Look into test_extract test. Here I made struct VisitableByExtractor to go third road. It's not directly nor indirectly extractible. But still I a able to use boost::extract interface. > > BTW it may worth mentioning in docs. Now it appears that having > > extract::visitor in private part of the class definition make third > > way less usable cause I could not mention Visitor name > > I'm not sure what you mean here. The visitor is _not_ intended to be > used directly. I believe this concept deserve it's own place out of boost::variant details > > 3. Defining result type in extract::visitor definition is unnecessary > > and in fact prohibit compilation on MSVC6.5 > > Please explain. I have not attempted compilation on MSVC prior to > version 7. struct visitor : static_visitor<> { public: // typedefs typedef void result_type; Why do you need this typedef? static_visitor<> will do this for you. MSVC6.5 will require you to use void_ type cause it does not support return void semantic. > > > [snip] > > 5. Name for the visitor could be more specific. For example the one > > you have in it's description > > I don't understand why it needs to be more specific. It is a private > nested class. If you disagree, please explain why. > I could use this visitor to implement third way to support extract interface. > > define_forwarding_func.hpp > > This file contain mix of Unix/Windows EOLs. > > Please explain this problem and tell me how to fix it. Thanks. Try to print this file on Windows. Some lines disappear or became 1 line. Run something like unix2dos for example. > > delayed_apply_visitor.hpp > > I did not touch binary side but once you fix the traits > > implementation unary function operators became much more simple. > > You don't even need PP anymore, but straightforward: > > template <typename Visitable> > > result_type operator()( Visitable& visitable ) > > { > > return apply_visitor(visitor_, visitable); > > } > > Again, the code you provide does not allow temporaries to be passed to > operator(). Right. I expected Visitable to became 'T const' for temporaries. I will think more about it. BTW in your version you are missing return keywords. > > incomplete.hpp > > This header presents yet another implementation of smart pointer > > idiom. In this case with deep copy semantic. Do we need one? If > > yes, lets name it deep_copy_ptr. > > boost::incomplete is _not_ a smart pointer. It is a value type that's > allocates its content on the heap. > > So no, deep_copy_ptr is not an appropriate name. This semantic is very close to what "real" deep copy smart pointer would have and could easily be generated using smart_ptr framework. The only difference is that incomplete allocate memory itself > > 2. aligned_storage component is not that generic as it may look from > > the perfunctory look. It only works of size is 2^N. May be we should > > check this explicitly for more clear error notification. > > Please explain. Try to define aligned_storage<5>. Do you see an error message? > > > 3. Why is it noncopyable?? > > The intention is to prevent blind copying of aligned_storage since it > is not possible to invoke the copy constructor corresponding to the > storage's content (if any). > > So I ask, why *not* noncopyable? What If I do want to make a copy and I now that all types have trivial copy constructor? > > visitor_ptr.hpp > > 1. why visitor_t is not function1<>? > > I'm not well accustomed to the Boost.Function library, but my limited > understanding suggests the "catch-all" behavior of visitor_ptr_t is not > implementable. Could you clarify above please? > Further, I am working on a larger Boost.Visitor library. That is, > visitor_ptr will need to return a function object that serves as both a > static and dynamic visitor. (See the code in the sandbox for further > explanation.) So how this prevent you from using function<1> for implementation in creation interface > > * I would prefer if you would follow boost recommendation with class > > data member naming with leading m_ instead of trailing _. > > Where are these recommendations? Funny, I couldn't find it. Does anybody know where is it now? _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost