Thank you for your comments. Pavel Vozenilek wrote:
"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.
We are working on reference docs, but didn't complete it in time for the review.
- 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. It has effect on VC and Intel C++ users.
Why? What does it do?
_____________________________________________________ 2. named_params.hpp:
Shouldn't the default for BOOST_NAMED_PARAMS_MAX_ARITY be more than 5?
Why not >= 10.
Sure, why not.
_____________________________________________________ 3. named_params.hpp:
Shouldn't the sub-namespace be named "named_params_detail" instead of just "detail"?
Yes.
_____________________________________________________ 4. docs: the line
window* w = new_window("alert2", movability); // error!
should say "logical error" or "coding error" or something like that.
OK.
_____________________________________________________ 6. could the code snippets be visually separated from the rest, e.g being in boxes and color syntax highlighted?
Don't the gray background and indentation suffice?
_____________________________________________________ 7. Question: is there possibility BCB could be convinced to compile (part) if the library or is BCB completely without any chance?
I am not very familiar with BCB, so I don't know.
_____________________________________________________ 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
It shouldn't. There is no inherent overhead in the library. No copies are made, and no heap storage is allocated.
- Info about exception safety.
Everything is nothrow, except when using lazy-defaults that can throw from operator(). But either way, the library doesn't throw.
- Whether it is possible to have both "named params" function foo() and the original signature function "foo" and whether there could be any problems.
Yes, it should be possible.
- How do the usual conversion rules apply? Exceptions, examples of these, tricks to make it as safe as possible.
- Do overloads of "named params" functions work? Example if they do.
Yes, by disabling overloads using SFINAE.
- How it is with exception specifications - suported, unsupported, dangerous here?
- Does "named params" allocate heap memory?
No.
- I would welocme detailed table what features do not work with this or that compiler (more than the single line of text now).
OK.
- What is overhead of: namespace { boost::keyword<name_t> name; boost::keyword<value_t> value; } being in header?
None..?
- Is it possible to reuse tags for different functions? Any problems can happen?
No, that's the intended use.
- Does it work with Boost.Any as parameter?
Yes.
- Does it work when one used non-default calling conventions as __fastcall?
I guess so.
- Is it possible to export "named params" function from DLL? Any problems?
The forwarding functions need to be templated. The actual implementation can be any function, so yes, it can be exported.
- Does it work with bind/boost.function/etc? Examples.
The forwarding functions are templated, so no. They can't be lazily evaluated unless done manually in a functor.
- Would it work to provide "named params" overload of existing C functions? E.g. Win32 API functions?
Yes.
- Is it possible to use usual concept checks with "named" function parameters? Will it work?
It is possible to disable the overload using SFINAE, as described in the documentation under "Controlling Overload Resolution".
- The macros is underdocumented and never used in tests.
- How the library works with namespaces: can I put the foo_impl into its own inner namespace? Example.
- What does need to be in header and what can be put in implementation file? Example(s).
The forwarding functions goes in the header.
- Iteration over parameters is undocumented. No examples.
It isn't possible to iterate over the parameters. It probably could be. Perhaps some fusion integration would be appropriate here.
- How does the library deals with std::auto_ptr parameter?
If the parameter is passed as a keyword argument, it should just work. If it's passed by position, you'd need to boost::ref() it.
Could there be test for it?
Yes.
- typedef void (* foo_t)(const char*, float); foo_t var = &foo_with_named_params; Is this possible/correct/error prone/caught by compiler?
- 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(???); }
I don't think it is, but it probably could be without much trouble.
- Does the library work with <cstdarg>?
I'm not sure what that means.
_____________________________________________________ 9. Number of test should be much higher. These should range from very simple ones to complex. All possibilities asked in [8] should be covered.
Agreed.
_____________________________________________________ 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.
# endif // __EDG_VERSION__ > 300
OK.
_____________________________________________________ 12. named_params.hpp: #include <boost/detail/workaround.hpp> should be there listed explicitly.
OK.
_____________________________________________________ 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.
Possibly, but I'm not sure how it would work. -- Daniel Wallin