[type_traits][core] modularisation and moving stuff about (again)
So... I have a couple of PR's to move some of type_traits into config so it can be used by core. It's a modest proposal so I can go with that I guess... but this moving stuff about all over the place just to satisfy dependencies does wind me up a bit - I know that I sometime struggle to figure out where stuff is on Github - half of Integer is in Config for example. How on earth would an end user figure that out in order to file a PR or a bug report? I know... "calm down dear" and all that ;) So radical suggestion - does type_traits just belong in core and lets be done with it? What would happen to the file history if we moved it (can I still "git blame")? DARFC, John. --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
On Wed, Aug 22, 2018 at 1:55 PM John Maddock wrote:
So... I have a couple of PR's to move some of type_traits into config so it can be used by core.
To be clear, these are intrinsics that we are discussing moving. I believe these in general are better suited for Config, even for other libraries that may not depend on Core but will still depend on Config. i.e. We're talking about things like BOOST_IS_FINAL(T) which may or may not be defined on a given implementation (e.g. to __is_final(T) or some other implementation specific intrinsic). A library author might have use for the intrinsic, but not the trait. For example, boost::is_final<T>::value might be false because the implementation has no intrinsic, but that doesn't mean you can derive from T if it really is final. On the other hand, if defined(BOOST_IS_FINAL) then BOOST_IS_FINAL(T) enables that. The intrinsic macros aside, whether the traits like boost::is_empty<T> should be in Core is a slightly different question. Glen
On Wed, Aug 22, 2018 at 9:18 PM, Glen Fernandes via Boost <boost@lists.boost.org> wrote:
On Wed, Aug 22, 2018 at 1:55 PM John Maddock wrote:
So... I have a couple of PR's to move some of type_traits into config so it can be used by core.
To be clear, these are intrinsics that we are discussing moving. I believe these in general are better suited for Config, even for other libraries that may not depend on Core but will still depend on Config.
i.e. We're talking about things like BOOST_IS_FINAL(T) which may or may not be defined on a given implementation (e.g. to __is_final(T) or some other implementation specific intrinsic).
A library author might have use for the intrinsic, but not the trait. For example, boost::is_final<T>::value might be false because the implementation has no intrinsic, but that doesn't mean you can derive from T if it really is final. On the other hand, if defined(BOOST_IS_FINAL) then BOOST_IS_FINAL(T) enables that.
I would rather encourage people use type traits, the official interface. Boost.Config or Boost.TypeTraits could provide macros that allow users to detect whether is_final is emulated or is the real deal.
The intrinsic macros aside, whether the traits like boost::is_empty<T> should be in Core is a slightly different question.
I think we should keep stuff where it belongs, if possible. Moving stuff around just because dependencies leaves a real mess where everything is everywhere. Note that dependencies may change as components evolve, so while something seems light right now, it doesn't mean it will remain light in the future.
On Wednesday, August 22, 2018, Andrey Semashev wrote:
I think we should keep stuff where it belongs, if possible.
I agree, in general. The reason we are moving the two macros BOOST_IS_FINAL and BOOST_IS_EMPTY to Boost.Config is because it isn't possible. We want to use them in Boost.Core and Boost.Core cannot depend on Boost.TypeTraits. They are not out of place in Config, as they're very much like BOOST_ALIGNMENT or BOOST_FALLTHROUGH or BOOST_NOEXCEPT_EXPR which currently live in Config, and are all about providing a macro based on compiler detection. Glen
On Thu, Aug 23, 2018 at 12:38 AM, Glen Fernandes <glen.fernandes@gmail.com> wrote:
On Wednesday, August 22, 2018, Andrey Semashev wrote:
I think we should keep stuff where it belongs, if possible.
I agree, in general. The reason we are moving the two macros BOOST_IS_FINAL and BOOST_IS_EMPTY to Boost.Config is because it isn't possible. We want to use them in Boost.Core and Boost.Core cannot depend on Boost.TypeTraits.
Maybe we should make it possible to use Boost.TypeTraits in Boost.Core? I wasn't happy when we duplicated is_same, now this. I'm sure, we'll want more traits in time. Is the restriction of Boost.TypeTraits because it depends on Boost.MPL?
On Wednesday, August 22, 2018, Andrey Semashev wrote:
On Thu, Aug 23, 2018 at 12:38 AM, Glen Fernandes wrote:
On Wednesday, August 22, 2018, Andrey Semashev wrote:
I think we should keep stuff where it belongs, if possible.
I agree, in general. The reason we are moving the two macros BOOST_IS_FINAL and BOOST_IS_EMPTY to Boost.Config is because it isn't possible. We want to use them in Boost.Core and Boost.Core cannot depend on Boost.TypeTraits.
Maybe we should make it possible to use Boost.TypeTraits in Boost.Core? I wasn't happy when we duplicated is_same, now this. I'm sure, we'll want more traits in time.
Is the restriction of Boost.TypeTraits because it depends on Boost.MPL?
At one point in time. Since Core was established the criteria for for inclusion in it has been "not dependent on any other Boost modules except Core itself, Config, Assert, Static Assert, or Predef." I suppose TypeTraits has changed sufficiently enough now to consider relaxing that... Glen
John Maddock wrote:
So radical suggestion - does type_traits just belong in core and lets be done with it? What would happen to the file history if we moved it (can I still "git blame")?
That would be overkill. We could just allow use of type_traits in core. This will create a dependency cycle though, we need to get rid of the enable_if/noncopyable dependencies first.
On Thu, Aug 23, 2018 at 12:57 AM, Peter Dimov via Boost <boost@lists.boost.org> wrote:
John Maddock wrote:
So radical suggestion - does type_traits just belong in core and lets be done with it? What would happen to the file history if we moved it (can I still "git blame")?
That would be overkill. We could just allow use of type_traits in core. This will create a dependency cycle though, we need to get rid of the enable_if/noncopyable dependencies first.
Move enable_if to type traits? For noncopyable, we could convert its detection from is_base_and_derived to a dispatch based on SFINAE. // Boost.Core class noncopyable { public: typedef void _is_boost_noncopyable; }; // Boost.TypeTraits template< typename T, typename = void > struct is_boost_noncopyable : false_type {}; template< typename T > struct is_boost_noncopyable< T, typename T::_is_boost_noncopyable > : true_type {};
Andrey Semashev wrote:
Move enable_if to type traits?
We want type_traits to have enable_if anyway, to match std::enable_if, but the problem is that our existing boost::enable_if is not compatible with the standard one. So maybe call the TT one enable_if_ (+ enable_if_t which is fine).
Andrey Semashev wrote:
For noncopyable, we could convert its detection from is_base_and_derived to a dispatch based on SFINAE.
// Boost.Core class noncopyable { public: typedef void _is_boost_noncopyable; };
// Boost.TypeTraits template< typename T, typename = void > struct is_boost_noncopyable : false_type {};
template< typename T > struct is_boost_noncopyable< T, typename T::_is_boost_noncopyable > : true_type {};
This doesn't work because types derive privately from boost::noncopyable, and the trait above returns false in this case.
On 08/23/18 14:12, Peter Dimov via Boost wrote:
Andrey Semashev wrote:
For noncopyable, we could convert its detection from is_base_and_derived to a dispatch based on SFINAE.
// Boost.Core class noncopyable { public: typedef void _is_boost_noncopyable; };
// Boost.TypeTraits template< typename T, typename = void > struct is_boost_noncopyable : false_type {};
template< typename T > struct is_boost_noncopyable< T, typename T::_is_boost_noncopyable > : true_type {};
This doesn't work because types derive privately from boost::noncopyable, and the trait above returns false in this case.
Does is_base_and_derived (non-intrinsic-based) work with private inheritance?
Andrey Semashev wrote:
Does is_base_and_derived (non-intrinsic-based) work with private inheritance?
Yes, it does.
We'll have to do something like #ifndef BOOST_NONCOPYABLE_BASE_TOKEN_DEFINED #define BOOST_NONCOPYABLE_BASE_TOKEN_DEFINED namespace noncopyable_ { struct base_token {}; } #endif then derive noncopyable from that and repeat the above definition where it's used in Type Traits.
On 08/23/18 17:43, Peter Dimov via Boost wrote:
Andrey Semashev wrote:
Does is_base_and_derived (non-intrinsic-based) work with private > inheritance?
Yes, it does.
We'll have to do something like
#ifndef BOOST_NONCOPYABLE_BASE_TOKEN_DEFINED #define BOOST_NONCOPYABLE_BASE_TOKEN_DEFINED
namespace noncopyable_ { struct base_token {}; }
#endif
then derive noncopyable from that and repeat the above definition where it's used in Type Traits.
Or maybe have it just in TypeTraits and use it in Core. It could be named appropriately, e.g. noncopyable_base.
On 22 Aug 2018, at 19:54, John Maddock via Boost <boost@lists.boost.org> wrote:
So radical suggestion - does type_traits just belong in core and lets be done with it? What would happen to the file history if we moved it (can I still "git blame")?
For the record if you ever are going ahead with something like this. I think there are several options for keeping the history in Git so when moving stuff from one repository to another so git blame and friends still work. But some of them break historic references. You will want to keep historic commits unchanged including their complete history with hashes intact. Keeping commit hashes is essential for historic commits from the core repository in this case as they are referenced by the main boost repository and possibly elsewhere. Hence, any history-changing commands or scripted rebuilds should in general be avoided. I just tried out one method which you can try to see its it does what you want to do. First call git remote add -f type_traits ../type_traits to make the type_traits repository tracked as a remote in the core repository, then if you may need to call git fetch —all before the merge below if you get the «this is something we cannot merge» error. You then merge telling Git to ignore the fact that histories are not related. git merge —allow-unrelated-histories type_traits/master mac-bjorn:core bjorn$ git merge --allow-unrelated-histories type_traits/master Auto-merging test/is_same_test.cpp CONFLICT (add/add): Merge conflict in test/is_same_test.cpp Auto-merging test/Jamfile.v2 CONFLICT (add/add): Merge conflict in test/Jamfile.v2 Auto-merging meta/libraries.json CONFLICT (add/add): Merge conflict in meta/libraries.json Auto-merging index.html CONFLICT (add/add): Merge conflict in index.html Auto-merging doc/is_same.qbk CONFLICT (add/add): Merge conflict in doc/is_same.qbk Auto-merging doc/Jamfile.v2 CONFLICT (add/add): Merge conflict in doc/Jamfile.v2 Auto-merging appveyor.yml CONFLICT (add/add): Merge conflict in appveyor.yml Auto-merging .travis.yml CONFLICT (add/add): Merge conflict in .travis.yml Automatic merge failed; fix conflicts and then commit the result. The result, after conflict resolution, should be histories from both repositories in one repository, with a merge commit at the top. The only odd thing is that the histories do not have a common root. There is no common ancestor for these histories, but that will likely not cause any problems as the merge commit create a common ancestor for later work. I would have resolved the conflicts and committed, then based on the new commit started moving files without changing them into include/core (if that is what you want). In this case I think you will be fine moving files before you commit the merge if you just want a single commit. Note that in general, if you are moving files and making file content changes on the same file in one commit, then Git may have problems tracking that file move. I try to use a separate commit when I am moving files in Git to avoid this potential problem. However in 99 of 100 cases it will work regardless as it look for similar files that look like they are moved as well. There may also be a good idea to make a single commit removing all files in the historic deprecated repository. A notes file may be added to that commit to notify where the content has been moved and maybe why. The repository cannot be removed from GitHub as it would break references, and thus this last commit in master may prevent some people getting lost. Best regards, — Bjørn
participants (5)
-
Andrey Semashev -
Bjørn Roald -
Glen Fernandes -
John Maddock -
Peter Dimov