"Pavel Vozenilek" <pavel_vozenilek@hotmail.com> writes:
"Doug Gregor" wrote:
The formal review of the Named Parameters library by David Abrahams and Daniel Wallin starts today
I vote weak YES to accept this library.
- Its documentation is very weak and should be expanded *significantly*. More on it in note [8] bellow.
- There should be more tests. - Problem with Intel C++ should be investigated.
/Pavel
_____________________________________________________ 1. Headers should contain
#if (defined _MSC_VER) && (_MSC_VER >= 1200) # pragma hdrstop #endif
on the beginning.
Why?
It has effect on VC and Intel C++ users.
What effect does it have?
_____________________________________________________ 2. named_params.hpp:
Shouldn't the default for BOOST_NAMED_PARAMS_MAX_ARITY be more than 5?
Why not >= 10.
It's arbitrary. We could do either.
_____________________________________________________ 3. named_params.hpp:
Shouldn't the sub-namespace be named "named_params_detail" instead of just "detail"?
Yeah, something like that. I guess a pass over this code to strengthen it for Boost needs to be done.
Names like "nil" inside look quite collision prone.
_____________________________________________________ 4. docs: the line
window* w = new_window("alert2", movability); // error!
should say "logical error" or "coding error" or something like that.
It's a type error. Thanks.
_____________________________________________________ 5. docs: the chapter "Introduction" may be better named "Problem" but YMMV.
Noted.
_____________________________________________________ 6. could the code snippets be visually separated from the rest, e.g being in boxes and color syntax highlighted?
They are already in gray boxes. As for color syntax highlighting, that's not easy for us to do unless we convert the documentation source format... which is a possibility.
_____________________________________________________ 7. Question: is there possibility BCB could be convinced to compile (part) if the library or is BCB completely without any chance?
Anything is possible. I personally won't waste any time on BCB.
_____________________________________________________ 8. What I miss in documentation:
- Information about overhead: - numbers/graphs for at least two compilers - estimate how the overhead changes with # of parameters and their type
Are you curious about compile time or runtime?
- What happen when I change foo_impl() signature? Will it quiely comoile or what kind of error will I get. Is the error understandable? Example.
I am not convinced this belongs in the documentation, unless we are going to change our documentation standards drastically. Few other libraries discuss the quality of error message you get when they are misused.
- Few trivial and non-trivial overcommented complete examples.
Agreed.
- Info whether there are any additional constructors or assignements called for object as parameters.
?? It seems to me that section 3 quite clearly describes how the objects are passed. Can you help me understand why the answer to yoiur question wasn't clear from the text?
- Info about exception safety.
What, specifically, are you curious about? There's nothing much to say; all the library does is introduce a forwarding layer.
- Whether it is possible to have both "named params" function foo() and the original signature function "foo" and whether there could be any problems. Can the original signature foo() be externs "C"?
I'm confused by the question. Except for the need to write ref(), the "named params" function foo supports the original calling signature. I concede that if you have questions the docs need to be beefed up, regardless of the questions' validity.
- How do the usual conversion rules apply? Exceptions, examples of these, tricks to make it as safe as possible.
It should be clear from the fact that the forwarding functions are templated that no conversions apply at that point. However, the usual conversions do apply when the forwarding functions pass their results on to the actual implementation function.
- Do overloads of "named params" functions work? Example if they do.
Yes, that's the whole point of section 4, "Controlling Overload Resolution." Can you help me understand why that wasn't clear from the text?
- How it is with exception specifications - suported, unsupported, dangerous here?
Just as dangerous as ever. Supported? I have no idea what you mean. Libraries don't support exception specifications; that's done by the language.
- Does "named params" allocate heap memory?
No.
If so, in single block?
- Who are expected end users of the library, examples.
This is a general-purpose library. I don't think it's fair to demand that docs answer this question any more than you would ask who the users of an iterator library are. The answers are tautological: anyone who wants to provide a named parameter interface would use it.
- I would welocme detailed table what features do not work with this or that compiler (more than the single line of text now).
On the compilers we tested, all the detail is given by the simple list in the doc. Would it really help to draw boxes around those lines?
- What is overhead of: namespace { boost::keyword<name_t> name; boost::keyword<value_t> value; } being in header?
Compile-time overhead? These generate trivial objects with no data members, constructor, or destructor. It ought to be essentially free.
- Is it possible to reuse tags for different functions?
Yes, that's part of the point of the library.
Any problems can happen?
I suppose anything can have problems, but the library is designed to be used this way.
- Does it work with Boost.Any as parameter?
I don't see why not. Specific example, please? Again, this seems like an unfair question to demand that the docs answer. We could ask the same question about every other boost component. Does it work with Boost.Tuple parameters? Yes. etc., etc.
- Does it work when one used non-default calling conventions as __fastcall?
Yes, and that should be obvious from the documentation. There are no function pointers flying around; it just uses simple forwarding. Another unfair question to demand that the docs answer, IMO.
- Is it possible to export "named params" function from DLL? Any problems?
You wouldn't do that. You'd export the implementation function and leave the forwarding function templates in the header. But again, this should be obvious from the documentation if you know how to write a dynamic library that presents a templated interface.
- Does it work with bind/boost.function/etc?
What do you have in mind? Of course it "works with" any component in Boost for some definition of "with."
- If I use "named params" for the same member of both base and derived class: what defaults would be used? Is it consistent with default handling or 'normal' members?
Because the forwarding interface mechanism is completely exposed, the answers should be obvious to anyone who knows C++, it seems to me. These are not magical functions; they're just ordinary templates.
- Would it work to provide "named params" overload of existing C functions? E.g. Win32 API functions?
Again, that overloading works is the point of section 4. Yes.
- Is it possible to use usual concept checks with "named" function parameters? Will it work?
Because the forwarding interface mechanism is completely exposed, the answers should be obvious to anyone who knows C++, it seems to me.
- The macros is underdocumented and never used in tests.
Good point.
- How the library works with namespaces: can I put the foo_impl into its own inner namespace? Example.
Come on, of course you can. This is just standard C++.
- What does need to be in header and what can be put in implementation file? Example(s).
Everything goes in the header. You could use extern declarations for the keyword objects and stick them in an implementation file instead of putting them in an unnammed namespace, but it will be more inconvenient. Again, this is just standard C++; why is there any mystery here?
- Is there way to make definition of pImpled member functions easier?
Specifics, please?
- Iteration over parameters is undocumented.
Why should we document iteration over parameters? Maybe I don't understand what you have in mind.
No examples.
- There should be info whether the library headers can be part of precompiled headers (= has any compiler problems?)
It's standard C++, and subject to the same problems that any other library might have with certain precompiled header implementations, if any such problems exist. I don't use PCHs, so lack the expertise to find the specific information, but would be happy to include it in the docs if someone else supplies it.
- How does the library deals with std::auto_ptr parameter? Could there be test for it?
It's a good question, but I think this falls again under the common rules for C++ and auto_ptrs. You pass ref(x) or you use the named interface (kw = x), because auto_ptr can't be passed by const reference.
- typedef void (* foo_t)(const char*, float); foo_t var = &foo_with_named_params; Is this possible/correct/error prone/caught by compiler?
I don't see a problem. If it compiles, it's correct. It doesn't make sense to me to mention all the things that just work normally in the docs.
- Is it possible to call function with "named params" from another function with "named params" and the same signature and pass these params without need to extract them?
void foo_impl(....) { cout << "foo_impl called"; foo2(???); }
Good question. The answer is no; you have to call foo2_impl if you want to avoid extraction.
- Does the library work with <cstdarg>?
I'm not sure what, specifically, you have in mind, but because the forwarding interface mechanism is completely exposed, the answer should be obvious to anyone who knows C++, it seems to me.
- Can or can not this library be used in a ScopeGuard implementation? (I do not have exact idea how it would look like.)
No offense intended, and I really appreciate your interest in the library, but this seems typical of your questions. The docs shouldn't be expected to answer every vague question that pops into your head.
_____________________________________________________ 9. Number of test should be much higher. These should range from very simple ones to complex.
I agree.
All possibilities asked in [8] should be covered.
I disagree.
_____________________________________________________ 10. named_params.hpp: are the yes_t/no_t needed? Can similar types from TypeTraits be used instead?
Probably. I don't think it would be an advantage, though.
_____________________________________________________ 11. named_params.hpp: the #endif and #else should have comment to what they relate. The code blocks are quite large and its too easy to lost track.
I have mixed feelings about that. Those kind of comments get out-of-date really quickly and my editor, at least, can automatically find the matching #if. Maybe comments should be saved for information that can't be deduced mechanically. Also, it's hard to know whether the comment at the beginning and end of an else clause should match the #if condition or should negate it.
# endif // __EDG_VERSION__ > 300
_____________________________________________________ 12. named_params.hpp: #include <boost/detail/workaround.hpp> should be there listed explicitly.
Good catch.
_____________________________________________________ 13. debugging support: could there be simple function/macro which iterates over all named parameters and puts them (in readable, pretty form) into single string? Such string can then be printed, shown in IDE tooltip for examination etc.
Details, please? -- Dave Abrahams Boost Consulting http://www.boost-consulting.com