[boost] Re: [Review] Named parameters library

From: Daniel Wallin (dalwan01_at_[hidden])
Date: 2004-11-13 09:27:11

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