[modularization] Recommendations

older
[testing] Dissapearing regression...

Stephen Kelly

8 Jun 2014 8 Jun '14

10:21 a.m.

Hello, My last set of recommendations is here: http://thread.gmane.org/gmane.comp.lib.boost.devel/245078/focus=245806 The current graph of strongly connected modules is: http://www.steveire.com/boost/2014_june8_all.dot http://www.steveire.com/boost/2014_june8_all.png The type_traits mpl typeof core form a closed cylce. I consider this a small problem and I recommend deferring efforts to resolve it for now and focusing on the other edges which impact the rest of the graph. It may not be realistic to resolve every cycle or doing so may involve merging modules etc. I recommend deferring that discussion/work until after the major cycle-forming-edges are resolved. Group the above in the graph as a single node for now, removing many cycles: http://www.steveire.com/boost/2014_june8_with_kernel.png http://www.steveire.com/boost/2014_june8_with_kernel.dot Recommendation 1) Move parenthesized_type.hpp from parameter to core This removes the dependency from concept_check on parameter. Recommendation 2) Move implicit_cast.hpp and cast.hpp from conversion to core This also requires moving select_type.hpp from detail to core. This allows removal of the dependencies from iterator and range to conversion. As a side effect, it leaves lexical_cast as the only remaining public header in the conversion library. You can decide what impact that has for the recent 'Extracting lexical_cast from conversion' discussion. This requires removing the include of numeric/conversion/cast.hpp from cast.hpp. The content of that was moved to the numeric library in 2005 and the include was left behind for source compatibility. The result of the above recommendations is: http://www.steveire.com/boost/2014_june8_conversion_rdep.png http://www.steveire.com/boost/2014_june8_conversion_rdep.dot Recommendation 3) Remove use of algorithm from range Range includes boost/algorithm/string/config.hpp for no known purpose. According to git log -S BOOST_STRING_TYPENAME that define was never used in the range library. According to the history of boost/algorithm/string/config.hpp it never really had anything else useful. It might have made sense to include it for the Borland portability with the BOOST_STRING_TYPENAME, but that define was never used in range. Simply remove the use of the boost/algorithm/string/config.hpp header. Use boost/type_traits/detail/yes_no_type.hpp instead of boost/algorithm/string/yes_no_type.hpp in include/boost/range/detail/collection_traits_detail.hpp The resulting strongly connected graph is http://www.steveire.com/boost/2014_june8_range_deps.png http://www.steveire.com/boost/2014_june8_range_deps.dot There are more recommendations which can be made to reduce the strongly connected graph further, but I think the above are enough for now, so let's see if we can get through those first. Thanks, Steve.

Show replies by date

Stephen Kelly

8 Jun 8 Jun

10:54 a.m.

Stephen Kelly wrote:

...

Range includes

boost/algorithm/string/config.hpp

for no known purpose. According to

git log -S BOOST_STRING_TYPENAME

that define was never used in the range library.

I checked again and I was wrong about this. The define is used in Boost.Range, but now, given updated compiler requirements, it can be replaced with BOOST_DEDUCED_TYPENAME. I recommend doing that. Thanks, Steve.

Peter Dimov

11:44 a.m.

Thanks, Stephen, for the updated recommendations. Stephen Kelly wrote:

...

Recommendation 2) Move implicit_cast.hpp and cast.hpp from conversion to core

This also requires moving select_type.hpp from detail to core.

Actually, select_type.hpp is not being used in cast.hpp at all, as far as I can see. The include can just be removed. cast.hpp probably also needs to die, the polymorphic casts going into their own header. This will be post-release because it will break code, though. Inclusion report for (in module conversion): from gil: from range: from statechart:

Stephen Kelly

12:07 p.m.

Peter Dimov wrote:

...

cast.hpp probably also needs to die, the polymorphic casts going into their own header. This will be post-release because it will break code, though.

Actually it doesn't need to be a breaking change. You can create a new polymorphic_cast.hpp in core and #include that in cast.hpp, and move that cast.hpp (which now only contains two includes) to numeric. Then you can deprecate cast.hpp with a pragma warning and port any uses of it to include polymorphic_cast.hpp instead. Non breaking change. Thanks, Steve.

Peter Dimov

2:15 p.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

Stephen Kelly wrote:

...

Peter Dimov wrote:

...
cast.hpp probably also needs to die, the polymorphic casts going into their own header. This will be post-release because it will break code, though.

Actually it doesn't need to be a breaking change.

You can create a new polymorphic_cast.hpp in core and #include that in cast.hpp, and move that cast.hpp (which now only contains two includes) to numeric. Then you can deprecate cast.hpp with a pragma warning and port any uses of it to include polymorphic_cast.hpp instead. Non breaking change.

Yes, you are right. Moving cast.hpp to numeric/conversion is a better way to deal with it. There'll be no need to remove the numeric/ include then, and it will truly be a non-breaking change. Re moving *_cast.hpp to Core, I actually prefer the other way, moving lexical_cast to its own module. Conversion will then drop to level 2, same as Core.

Antony Polukhin

2:32 p.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

2014-06-08 18:15 GMT+04:00 Peter Dimov :

...

Stephen Kelly wrote:

...
Peter Dimov wrote:

...
cast.hpp probably also needs to die, the polymorphic casts going into > their own header. This will be post-release because it will break code, > though.

Actually it doesn't need to be a breaking change.

You can create a new polymorphic_cast.hpp in core and #include that in cast.hpp, and move that cast.hpp (which now only contains two includes) to numeric. Then you can deprecate cast.hpp with a pragma warning and port any uses of it to include polymorphic_cast.hpp instead. Non breaking change.

Yes, you are right. Moving cast.hpp to numeric/conversion is a better way to deal with it. There'll be no need to remove the numeric/ include then, and it will truly be a non-breaking change.

This does not look right. polymorpic_cast and polymotphic_downcast do not look like they belong to Numeric. They do not deal with numbers at all.

...

Re moving *_cast.hpp to Core, I actually prefer the other way, moving lexical_cast to its own module. Conversion will then drop to level 2, same as Core.

Most part of the Conversion dependencies are related to the LexicalCast. Moving it into a separate module looks reasonable. It is already represented as a separate library on the website. -- Best regards, Antony Polukhin

Peter Dimov

2:51 p.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

Antony Polukhin wrote:

...

This does not look right. polymorpic_cast and polymotphic_downcast do not look like they belong to Numeric. They do not deal with numbers at all.

The proposal is: - move the polymorphic casts into their own header, in conversion, boost/polymorphic_cast.hpp; - move boost/cast.hpp, which is now #include #include #include into numeric/conversion, pending deprecation.

...

Most part of the Conversion dependencies are related to the LexicalCast. Moving it into a separate module looks reasonable. It is already represented as a separate library on the website.

Everyone seems to agree on that, so what do you need to move forward with it? Will just an empty 'lexical_cast' repository be enough, or do you need me to do something else? Perhaps cloning the current contents of 'conversion' into 'lexical_cast' may be a better starting point?

Antony Polukhin

5:58 p.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

2014-06-08 18:51 GMT+04:00 Peter Dimov :

...

Antony Polukhin wrote:

This does not look right. polymorpic_cast and polymotphic_downcast do not

...
look like they belong to Numeric. They do not deal with numbers at all.

The proposal is:

- move the polymorphic casts into their own header, in conversion, boost/polymorphic_cast.hpp; - move boost/cast.hpp, which is now

#include #include #include

into numeric/conversion, pending deprecation.

This looks reasonable. BTW what about the new Conversion library (was it accepted?)? If it was accepted, then it will add a lot of dependencies and it would be better to separate it from the old conversion library.

...

Most part of the Conversion dependencies are related to the LexicalCast.

...
Moving it into a separate module looks reasonable. It is already represented as a separate library on the website.

Everyone seems to agree on that, so what do you need to move forward with it? Will just an empty 'lexical_cast' repository be enough, or do you need me to do something else? Perhaps cloning the current contents of 'conversion' into 'lexical_cast' may be a better starting point?

Repo with cloned contest of conversion would be great. I'll do the remaining part of the work tomorrow (remove the files of other libraries, update the docs... ). Please make a repo. -- Best regards, Antony Polukhin

Peter Dimov

8:47 p.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

Antony Polukhin wrote:

...

Repo with cloned contest of conversion would be great. I'll do the remaining part of the work tomorrow (remove the files of other libraries, update the docs... ).

Please make a repo.

Done. You have a repo lexical_cast that should be a carbon copy of conversion, and a submodule lexical_cast in develop. There's no submodule in master yet, let me know when you think I should create it there as well.

Edward Diener

9:36 p.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

On 6/8/2014 1:58 PM, Antony Polukhin wrote:

...

2014-06-08 18:51 GMT+04:00 Peter Dimov :

...
Antony Polukhin wrote:

This does not look right. polymorpic_cast and polymotphic_downcast do not

...
look like they belong to Numeric. They do not deal with numbers at all.

The proposal is:

- move the polymorphic casts into their own header, in conversion, boost/polymorphic_cast.hpp; - move boost/cast.hpp, which is now

#include #include #include

into numeric/conversion, pending deprecation.

This looks reasonable.

BTW what about the new Conversion library (was it accepted?)? If it was accepted, then it will add a lot of dependencies and it would be better to separate it from the old conversion library.

The new library, called Convert ( not Conversion ), was accepted.

Peter Dimov

9:59 p.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

Edward Diener wrote:

...

The new library, called Convert ( not Conversion ), was accepted.

That's probably going to create a lot of confusion. :-)

Edward Diener

9 Jun 9 Jun

12:05 a.m.

New subject: conversion, cast.hpp (Was: [modularization] Recommendations)

On 6/8/2014 5:59 PM, Peter Dimov wrote:

...

Edward Diener wrote:

...
The new library, called Convert ( not Conversion ), was accepted.

That's probably going to create a lot of confusion. :-)

Perhaps but Conversion is really 'cast' and 'lexical_cast' so maybe it is badly named and should be called the 'Cast' or 'Casting' library instead purely based on its internal classes.

Antony Polukhin

9:31 a.m.

2014-06-08 15:44 GMT+04:00 Peter Dimov :

...

Thanks, Stephen, for the updated recommendations.

Stephen Kelly wrote:

Recommendation 2) Move implicit_cast.hpp and cast.hpp from conversion to

...
core

This also requires moving select_type.hpp from detail to core.

Actually, select_type.hpp is not being used in cast.hpp at all, as far as I can see. The include can just be removed.

cast.hpp probably also needs to die, the polymorphic casts going into their own header. This will be post-release because it will break code, though.

Inclusion report for (in module conversion):

from gil:

from range:

from statechart:

Made pull requests for those libraries to use polymorphic_cast.hpp instead of cast.hpp: https://github.com/boostorg/gil/pull/9 https://github.com/boostorg/statechart/pull/1 https://github.com/boostorg/range/pull/10 Moved cast.hpp to the NumericCast library: https://github.com/boostorg/numeric_conversion/pull/2 -- Best regards, Antony Polukhin

Peter Dimov

11:21 a.m.

Antony Polukhin wrote:

...

Made pull requests for those libraries to use polymorphic_cast.hpp instead of cast.hpp: https://github.com/boostorg/gil/pull/9 https://github.com/boostorg/statechart/pull/1 https://github.com/boostorg/range/pull/10

Moved cast.hpp to the NumericCast library: https://github.com/boostorg/numeric_conversion/pull/2

I can merge these if there are no objections.

Antony Polukhin

10 Jun 10 Jun

9:46 a.m.

2014-06-09 15:21 GMT+04:00 Peter Dimov :

...

Antony Polukhin wrote:

...
Made pull requests for those libraries to use polymorphic_cast.hpp instead of cast.hpp: https://github.com/boostorg/gil/pull/9 https://github.com/boostorg/statechart/pull/1 https://github.com/boostorg/range/pull/10

Moved cast.hpp to the NumericCast library: https://github.com/boostorg/numeric_conversion/pull/2

I can merge these if there are no objections.

I have no objections, but please add the new LexicalCast module to the master branch of boostorg/boost project first. -- Best regards, Antony Polukhin

Peter Dimov

10:03 a.m.

Antony Polukhin wrote:

...

I have no objections, but please add the new LexicalCast module to the master branch of boostorg/boost project first.

Should be already there.

Vicente J. Botet Escriba

9 Jun 9 Jun

8:09 a.m.

Le 08/06/14 12:21, Stephen Kelly a écrit :

...

Recommendation 3) Remove use of algorithm from range

Range includes

boost/algorithm/string/config.hpp

for no known purpose. According to

git log -S BOOST_STRING_TYPENAME

that define was never used in the range library. According to the history of

boost/algorithm/string/config.hpp

it never really had anything else useful. It might have made sense to include it for the Borland portability with the BOOST_STRING_TYPENAME, but that define was never used in range.

Simply remove the use of the

boost/algorithm/string/config.hpp

header.

Use

boost/type_traits/detail/yes_no_type.hpp

instead of

boost/algorithm/string/yes_no_type.hpp

in

include/boost/range/detail/collection_traits_detail.hpp

Hi, I agree with this modifications. Could we decide if this is the way to resolve this dependency? Best, Vicente

Peter Dimov

10:49 a.m.

New subject: range -> algorithm, Was: [modularization] Recommendations

Vicente J. Botet Escriba wrote:

...

Le 08/06/14 12:21, Stephen Kelly a écrit :

...
Recommendation 3) Remove use of algorithm from range

Range includes

boost/algorithm/string/config.hpp

... Hi,

I agree with this modifications.

Could we decide if this is the way to resolve this dependency?

Our general procedure should be that the decision if or when to resolve a dependency is up to the library maintainers.

Neil Groves

11:25 a.m.

New subject: range -> algorithm, Was: [modularization] Recommendations

On Mon, Jun 9, 2014 at 11:49 AM, Peter Dimov wrote:

...

Vicente J. Botet Escriba wrote:

...
Le 08/06/14 12:21, Stephen Kelly a écrit :

...
Recommendation 3) Remove use of algorithm from range

Range includes

boost/algorithm/string/config.hpp

...

...
Hi,

I agree with this modifications.

Could we decide if this is the way to resolve this dependency?

Our general procedure should be that the decision if or when to resolve a dependency is up to the library maintainers.

I am a Boost.Range maintainer and I don't mind having the dependency moved or removed. My delayed response is due to considering the way in which I might assist best. I would like to improve the component breakdown and hence modularisation effort. In addition to the dependency on algorithm, I think Boost.Range would be improved by moving the algorithms into Boost.Algorithm, and the tokenized adaptor should be moved into Boost.Regex. Currently some C++11 algorithms are missing from Boost.Range but present in Boost.Algorithm whereas C++03 algorithms all live inside Boost.Range. I'd like to see Boost.Range become smaller and encourage other components to provide range algorithms and adaptors. This is why the richness of the Boost.Range adaptors is a little disappointing, I am not keen on providing rich adaptors that couple other components. The adaptor idiom should be adopted elsewhere. I don't want to do less to help Boost and the lack of write access to other components makes this reorganisation difficult for me to accomplish. Obviously I would wish to discuss and gain support from the appropriate library maintainers, but the write access issue is a barrier. I believe that the strong rights to access to the component I am responsible for combined with low rights to other components drives me toward making design decisions that are suboptimal. I wonder if this has the same effect with other developers. If so should we reconsider re-enabling broader write access and encourage a less formal peer collaboration? I would like to join the community maintenance team and have broader write access? I'd be delighted to help modularise Boost.Range and help out where I can with other maintenance issues. Over the last few months I've been able to dedicate significantly more time to Boost. In summary, I'm happy to move aside to so that those working on the modularisation effort can make progress. I am equally happy to directly help. Regards, Neil Groves

Vicente J. Botet Escriba

11:59 a.m.

New subject: range -> algorithm, Was: [modularization] Recommendations

Le 09/06/14 13:25, Neil Groves a écrit :

...

On Mon, Jun 9, 2014 at 11:49 AM, Peter Dimov wrote:

...
Vicente J. Botet Escriba wrote:

...
Le 08/06/14 12:21, Stephen Kelly a écrit :

...
Recommendation 3) Remove use of algorithm from range

Range includes

boost/algorithm/string/config.hpp

...

...
Hi,

I agree with this modifications.

Could we decide if this is the way to resolve this dependency?

Our general procedure should be that the decision if or when to resolve a dependency is up to the library maintainers.

I am a Boost.Range maintainer and I don't mind having the dependency moved or removed. My delayed response is due to considering the way in which I might assist best. I would like to improve the component breakdown and hence modularisation effort. In addition to the dependency on algorithm, I think Boost.Range would be improved by moving the algorithms into Boost.Algorithm, and the tokenized adaptor should be moved into Boost.Regex. Currently some C++11 algorithms are missing from Boost.Range but present in Boost.Algorithm whereas C++03 algorithms all live inside Boost.Range. I'd like to see Boost.Range become smaller and encourage other components to provide range algorithms and adaptors. This is why the richness of the Boost.Range adaptors is a little disappointing, I am not keen on providing rich adaptors that couple other components. The adaptor idiom should be adopted elsewhere. I don't want to do less to help Boost and the lack of write access to other components makes this reorganisation difficult for me to accomplish. Obviously I would wish to discuss and gain support from the appropriate library maintainers, but the write access issue is a barrier. Neil I think that the most urgent is in removing the dependencies that make cycles. Once this is done, xe could refactor more if this makes Boost more manageable. I believe that the strong rights to access to the component I am responsible for combined with low rights to other components drives me toward making design decisions that are suboptimal. I wonder if this has the same effect with other developers. If so should we reconsider re-enabling broader write access and encourage a less formal peer collaboration? You could always fork and do pull requests, even if this is not optimal. I would like to join the community maintenance team and have broader write access? I'd be delighted to help modularise Boost.Range and help out where I can with other maintenance issues. Over the last few months I've been able to dedicate significantly more time to Boost.

In summary, I'm happy to move aside to so that those working on the modularisation effort can make progress. I am equally happy to directly help.

I think it is enough you request to be a member of the community maintenece team explicitly. Thanks, Vicente

3822

Age (days ago)

3824

Last active (days ago)

List overview

Download

19 comments

6 participants

participants (6)

Antony Polukhin
Edward Diener
Neil Groves
Peter Dimov
Stephen Kelly
Vicente J. Botet Escriba