This is not a review. Just some comments on the docs. I have other issues with the design which I'll raise in another post. Hi,

Le 16/08/11 23:17, John Bytheway a écrit : thanks in advance for your useful and constructive comments.

The one long page of docs is a bit intimidating. At least it should be split into reference and non-reference, I think. I could change that. I guess that then there will be a page for each first level section, which is admisible. I will do it if this is considered a better choice.

Overview --------

- s/Tatget/Target/ OK. Done.

- In this:

template< class Source> convertible_from<From> implicitly(Source source) { return convertible_from<Source>(source); }

I guess you mean "convertible_from<Source>" as the return type? Yes. Done. - When first discussing the need for is_constructible and is_assignable (under "Caveats"), could you mention that there is a list of supported compilers just below (to avoid scaring away people who don't know whether their compiler is supported)? Good idea. The fact that I didn't reached to implement these traits on some compilers was quite frustrating. For the moment I have reached to implement it for gcc >= 4.4 and clang. Even if MSVC supports decltype I don't reach to make it work. User's Guide ------------

- You say it needs "compilers supporting decltype" -- would typeof suffice also? At the end I think that what is needed is just SFINAE_EXPR. I'll try TYPEOF and see how this behaves.

- "When you need to make a extrinsic explicit conversion" is ambiguous; I suggest "When you need to perform an extrinsic explicit conversion". OK. - In "they can try with the make assigner to `lvalue' free function" you have bold 'm', 'a', 't'; is this because the function used to be called "mat"? If so, they should probably be un-bolded. Yes. This was the preceding name. I have removed this now. - "Why ADLIt would be great if" looks like a mistake. Done.

- In this:

template<typename T> typename enable_if<is_extrinsically_convertible<int>,void>::type f(T v) { return convert_to<int>(v); }

surely you need to pass T to is_extrinsically_convertible? Yes. Done. - Does is_extrinsically_convertible test for explicit or implicit convertibility? Is there a trait for the other one? It test for implicit conversion. There is a trait is_extrinsically_explicit_convertible. - I presume s/implicit_convert_cp/implicit_converter_cp/ Done.

- What does the "cp" signify? I figured it out eventually but you probably want to say. _cp stands for customization point. I will add a more specific explanation.

- I assume there is also explicit_converter_cp and an implicit_convert_to function you can overload with dummy_type? You should probably at least mention them in passing when introducing implicit_converter_cp and explicit_convert_to. OK. I will do it before the review.

Reference ---------

- The docs of BOOST_CONVERSION_NO_IS_ASSIGNABLE say "Macro stating if the compiler doesn't support...". Please clarify in what way the macro "states" this; I think you mean "Macro defined if the compiler doesn't support..." (and personally I would prefer "Macro defined if and only if the compiler doesn't support..."). The other macros similarly. OK. I will do it before the review. - Please normalise the names of BOOST_CONVERSION_NO_IS_EXPLICIT_CONVERTIBLE and is_explicitly_convertible; the former should have the "LY". Similarly BOOST_CONVERSION_NO_IS_EXTRINSIC_ASSIGNABLE, BOOST_CONVERSION_NO_IS_EXTRINSIC_CONVERTIBLE. OK. I will do it before the review. - You list all these headers providing specializations for standard types, such as "Type traits specializations for<string> types." but it's far from clear what these mean. There are more details lower down, but you wouldn't know that from looking at the short version. Can you point or link to the further details? OK. I will try to do it before the review. - "implicit conversion of the @ Source" -- I guess the @ is a typo? Yes. It should be @c.

- "Defines the free function assignable_to class" -- is it a function or a class? It is a class. Corrected.

- ExtrinsicallyExplicitConvertible should probably be ExtrinsicallyExplicitlyConvertible for consistency. I was wondering how tis should be. If you can confirm that this is the correct way I will change them.

- boost/conversion/convertible_from.hpp is documented as "Defines the free function implicitly.". I have two problems with this: Surely it's more significant that it defines convertible_from? Also, I think implicitly warrants its own header; I'm unlikely to remember that this is what one must include to get it. OK I understand. I will add a implicitly.hpp and a lvalue.hpp headers.

- What are the semantics for the conversions to/from std::string? Currently the library uses lexical cast and I know that this is a controversial issue. I will add it in a more explicit way.

- Do the fusion::tuple conversions really only go up to 3-tuples? What about 1-tuples? I have no taken the time to implement a preprocessor version. I will do it if the library is accepted. - Must all the "trick_*" classes really appear in the docs? This is the first time I have used doxygen to generate the docs. I have find a lot of trouble to ensure that the comments in the source appear completly in the doc, and I know that there are some cases that are missing. Most od the time, this are bugs associated to how boost book takes in account the xml files generated by doxygen, maybe be there are some cases associated to doxygen itself, I don't remember. Unfortunately if I remove these classes, other specializations will disappear from the documentation. I will try to identify a simple case causing the error and make a ticket. John Bytheway Thanks again, Vicente

Le 16/08/11 23:29, John Bytheway a écrit :

...

Hi all,

The review of Vicente Botet Escriba's Conversion library starts this Saturday August 20 and continues through August 29.

This library provides a general system for type-to-type conversion. As such, it can be thought of either as a substitute for overloading static_cast, or as an alternative to lexical_cast when the intermediate conversion to text is not wanted.

Your review and/or discussion would be greatly appreciated. As always, please post to the main Boost list if possible, or the boost-users list, or you can send your review directly to the Review Managers. The primary concern I always have with libraries such as this one is

On 16/08/11 15:59, Gordon Woodhull wrote: that they promote ODR violations. In particular, consider:

library A defines type TA

library B defines type TB

library C defines a conversion from TA to TB

library D defines a conversion from TA to TB

Now libraries C and D are incompatible; they cannot both be used in the same program without ODR violation. You are right and I have no solution to this big issue. I must add this to the documentation.

I think that the same case appears when library T defines a trait type tt that must be specialized by the user. library A defines type TA library C needs to specialize the type trait tt for TA library D needs to specialize the type trait tt for TA Now libraries C and D are incompatible for the same reason. An organization could choose to organize his libraries in a way that make possible to avoid the odr in an easy way. But it is not simple to formalize some guidelines that can be shared between different organizations. I don't know, even if the following is not a solution to the issue, maybe the library could suggest to the library author defining the conversion to add a static variable that could be used to detect the odr violations at link time. I don't remember who, suggested that library C should define classes C::TA and C::TB inheriting from TA and TB so one knows the other and conversion can be defined intrinsicaly to these specific classes. I have not explored the usage, advantages and liabilities of this approach.

This means, if I am writing a library, then I cannot safely define a conversion between types both of which are not in my library, because it would make my library incompatible with any other library that also defines such a conversion.

But, on the other hand, if one of the types *is* in my library, then I can probably make do with a conversion constructor or conversion operator (except perhaps if I want an explicit conversion operator and want to support compilers without those).

So, as I see it, conversions can only be defined in two situations:

- When writing non-library code (i.e. code that will not be combined with other code over which the author has no control).

Yes, here there is no problem as the author/organization can manage with which code is included in the executable.

- As a stop-gap substitute for explicit conversion operators. This was one of the initial motivation of the library.

Is it indeed intended only to allow conversions to be defined in these limited circumstances? If so, then the documentation should state that clearly. If not, how do you intend to avoid ODR violations?

You are completly right and during a good period I was tempted to withdraw the library submission. However,as you pointed out, there are some context in which the library is useful so I have persisted in the development. Thanks for raising (again) this big issue, which could be one of the reasons to reject the library. Best Vicente

On 17/08/11 00:43, Vicente J. Botet Escriba wrote:

Le 16/08/11 23:29, John Bytheway a écrit :

...

...

Hi all,

The review of Vicente Botet Escriba's Conversion library starts this Saturday August 20 and continues through August 29.

This library provides a general system for type-to-type conversion. As such, it can be thought of either as a substitute for overloading static_cast, or as an alternative to lexical_cast when the intermediate conversion to text is not wanted.

Your review and/or discussion would be greatly appreciated. As always, please post to the main Boost list if possible, or the boost-users list, or you can send your review directly to the Review Managers. The primary concern I always have with libraries such as this one is

On 16/08/11 15:59, Gordon Woodhull wrote: that they promote ODR violations. In particular, consider:

library A defines type TA

library B defines type TB

library C defines a conversion from TA to TB

library D defines a conversion from TA to TB

Now libraries C and D are incompatible; they cannot both be used in the same program without ODR violation. You are right and I have no solution to this big issue. I must add this to the documentation.

I think that the same case appears when

library T defines a trait type tt that must be specialized by the user. library A defines type TA library C needs to specialize the type trait tt for TA library D needs to specialize the type trait tt for TA

Now libraries C and D are incompatible for the same reason.

An organization could choose to organize his libraries in a way that make possible to avoid the odr in an easy way. But it is not simple to formalize some guidelines that can be shared between different organizations.

I don't know, even if the following is not a solution to the issue, maybe the library could suggest to the library author defining the conversion to add a static variable that could be used to detect the odr violations at link time.

I don't remember who, suggested that library C should define classes C::TA and C::TB inheriting from TA and TB so one knows the other and conversion can be defined intrinsicaly to these specific classes. I have not explored the usage, advantages and liabilities of this approach.

I once suggested a tag-dispatch system to help with an issue like this. It would go something like: Boost.Conversion would define the base tag type boost::conversion::tag. Library C would define C::tag, inheriting (probably virtually) from boost::conversion::tag, and all the conversions it defines would be marked somehow with this tag. Similarly, library D would define D::tag, inheriting from boost::conversion::tag. If I write code that wishes to use conversions from both libraries (say in program E) then I define E::tag, which inherits from A::tag and B::tag. Then I do e.g. TA a; TB b = convert_to<TB>(a, E::tag()); The definition of convert_to is contrived such that if either C or D (or Boost.Conversion) defines the conversion from TA to TB then it will appear in the overload set as convert_to(A, C::tag) or convert_to(A, D::tag) respectively, and thus be used. However, if they *both* do then there will be an ambiguity in the function resolution because E::tag is convertible to both C::tag and D::tag. So, in summary, by sticking a library-specific tag type into all the specializations and overloads defined by a library, we reduce the risk of ODR violation (although it can still happen in a slightly more obscure and indirect way). Furthermore, it automatically detects some (depending on which headers were included, etc. etc.) situations where there is disagreement about the implementation of the conversion and turns it into a compiler error which (hopefully) makes the problem somewhat clear. One downside is that it's harder to write generic code using such conversions; it would probably be necessary to have your caller provide the appropriate tag type. On balance I suspect this is not worth the trouble, but I just wanted to throw the idea out. John

Hi Vicente, On Aug 21, 2011, at 6:31 AM, Vicente J. Botet Escriba wrote:

I have added a section to the documentation containing a possible approach that could address this issue.

I have added it below.

Best, Vicente

How can libraries define and use conversions between unrelated types?

Do I understand that there would be the same problem and possible solution for an overloadable static_cast? I think the rule you're proposing could be summarized as: the end user must choose which conversions are defined. Thus, 1. The end user can define and use conversions in any way as long as they heed the ODR. 2. Libraries can define conversions only in headers, and they can't cause those headers to be included. 3. Libraries can use conversions, but only in header-only code that is not used by cpp's. (Actually #2 already ensures this, because the cpp would not compile without including conversion definition headers.) Does this cover it? Cheers, Gordon

On Mon, Aug 22, 2011 at 10:38 AM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:

Le 22/08/11 09:19, Gordon Woodhull a écrit :

...

Hi Vicente,

On Aug 21, 2011, at 6:31 AM, Vicente J. Botet Escriba wrote:

...

I have added a section to the documentation containing a possible approach that could address this issue.

I have added it below.

Best, Vicente

How can libraries define and use conversions between unrelated types?

Do I understand that there would be the same problem and possible solution for an overloadable static_cast?

I think the rule you're proposing could be summarized as: the end user must choose which conversions are defined.  Thus, 1. The end user can define and use conversions in any way as long as they heed the ODR.

It might be possible to abuse unnamed namespaces to avoid violating ODR. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

On Mon, Aug 22, 2011 at 11:56 AM, Vicente J. Botet Escriba <vicente.botet@wanadoo.fr> wrote:

Le 22/08/11 20:04, Emil Dotchevski a écrit :

...

It might be possible to abuse unnamed namespaces to avoid violating ODR.

The needed overload is dependent on the namespace of the types, and the trait specialization must be on boost::conversion. Could you clarify how could unamed namespaces help on this?

I'm not familiar with your library but I guess you could have an unnamed namespace inside the boost::conversion namespace and put the specializations you need in it. I'm not saying it's a good idea, only that it might be possible. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode