[detail] Compilation broken in develop and some questionable ideas
Hi, The commit https://github.com/boostorg/detail/commit/9fcf2ae390c7c21a9b42fd40dd5c0eec11... breaks the compilation of Boost.Filesystem: D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(126): error C2487: 'do_in': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(136): error C2487: 'do_out': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(152): error C2487: 'get_octet_count': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(156): error C2487: 'get_cont_octet_out_count': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(189): error C2487: 'do_length': member of dll interface class may not be declared with dll interface According to MSDN only the whole class or class separate members could be decorated with BOOST_UTF8_DECL, not both ( https://msdn.microsoft.com/en-us/library/t72ahzw1.aspx) Now the questionable part... We could theoreticaly avoid duplication of `utf8_codecvt_facet` code in libraries and even avoid code breakage if we: * move `struct utf8_codecvt_facet` to namespace `boost::detail::utf8_codecvt_facet_namespace` * mark all it's functions as inline, even the virtual ones => `inline` will implicitly mark the stuff in `struct utf8_codecvt_facet` as weak symbols, so that multiple occurences of it will be reduced to one by linker * add the using declaration `BOOST_UTF8_BEGIN_NAMESPACE using boost::detail::utf8_codecvt_facet_namespace::utf8_codecvt_facet; BOOST_UTF8_END_NAMESPACE` => using declaration will help the existing code to find class in `BOOST_UTF8_BEGIN_NAMESPACE` defined namespaces. Questionable part number two... Is it time to move Boost.Detail module into Boost.Core/Boost.Algorithm/Boost.MPL? Main points are: * Boost.Detail is really old and many people who maintained it are inactive. Moving it into rapidly developing modules will improve it's support (maybe even docs will appear! or tests!) * Nobody knows what does `Deatil` mean in that particular case. `detail::` namespace is a namespace that user must not use. However there's some cool stuff in Detail module that must be available to users, so the name Detail is obfuscating here. -- Best regards, Antony Polukhin
Antony Polukhin wrote:
Is it time to move Boost.Detail module into Boost.Core/Boost.Algorithm/Boost.MPL?
Some parts of Detail have already been moved into Core. It doesn't make much sense to speak of Detail as a module because it contains a bunch of separate components that don't have much to do with each other; every one of those needs to be considered on its own and then moved wherever would be appropriate.
On 10/23/15 10:35 AM, Antony Polukhin wrote:
Hi,
The commit https://github.com/boostorg/detail/commit/9fcf2ae390c7c21a9b42fd40dd5c0eec11... breaks the compilation of Boost.Filesystem:
OK - that's my doing
D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(126): error C2487: 'do_in': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(136): error C2487: 'do_out': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(152): error C2487: 'get_octet_count': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(156): error C2487: 'get_cont_octet_out_count': member of dll interface class may not be declared with dll interface D:\t08\run\boost_root\boost/detail/utf8_codecvt_facet.hpp(189): error C2487: 'do_length': member of dll interface class may not be declared with dll interface
According to MSDN only the whole class or class separate members could be decorated with BOOST_UTF8_DECL, not both ( https://msdn.microsoft.com/en-us/library/t72ahzw1.aspx)
Which it seems is somewhat different than the visibility attributes required by gcc and clang. So I've had difficulty getting all this reconciled.
Now the questionable part...
We could theoreticaly avoid duplication of `utf8_codecvt_facet` code in libraries and even avoid code breakage if we: * move `struct utf8_codecvt_facet` to namespace `boost::detail::utf8_codecvt_facet_namespace` * mark all it's functions as inline, even the virtual ones => `inline` will implicitly mark the stuff in `struct utf8_codecvt_facet` as weak symbols, so that multiple occurences of it will be reduced to one by linker * add the using declaration `BOOST_UTF8_BEGIN_NAMESPACE using boost::detail::utf8_codecvt_facet_namespace::utf8_codecvt_facet; BOOST_UTF8_END_NAMESPACE` => using declaration will help the existing code to find class in `BOOST_UTF8_BEGIN_NAMESPACE` defined namespaces.
The current setup is a hack imposed by certain unnamed persons who are no longer involved in boost to address the fact that the utf8 facet was never subjected to official review so it should be in a detail section. Which might have been OK except for the fact that it's shared by several components. This should have been a first class component of Boost for many, many years now. If we do this - this whole mess goes away.
Questionable part number two...
Is it time to move Boost.Detail module into Boost.Core/Boost.Algorithm/Boost.MPL? Main points are: * Boost.Detail is really old and many people who maintained it are inactive. Moving it into rapidly developing modules will improve it's support (maybe even docs will appear! or tests!) * Nobody knows what does `Deatil` mean in that particular case. `detail::` namespace is a namespace that user must not use. However there's some cool stuff in Detail module that must be available to users, so the name Detail is obfuscating here.
again all addressed by the above solution. And better yet, we now have the opportunity to resplace this component with a purportedly more correct solution which is header only. This would make things that much easier. We haven't moved on this because because we're trying to get all the bugs fixed for this release. I would hope we can address this when we have the current monkey of our backs. In short we need to: a) fix this. b) get the release done c) make utf8_codecvt facet a first class boost component d) consider the proposed replacement Robert Ramey
This should have been a first class component of Boost for many, many
years now. If we do this - this whole mess goes away.
[snip]
stuff in Detail module that must be available to users, so the name Detail
is obfuscating here.
again all addressed by the above solution.
And better yet, we now have the opportunity to resplace this component with a purportedly more correct solution which is header only. This would make things that much easier. We haven't moved on this because because we're trying to get all the bugs fixed for this release. I would hope we can address this when we have the current monkey of our backs.
In short we need to:
a) fix this. b) get the release done c) make utf8_codecvt facet a first class boost component d) consider the proposed replacement
Robert Ramey
There already is header only utf8 codecvt facet there. So (c) and (d) are done. http://lists.boost.org/Archives/boost/2015/10/226245.php It would be released in boost 1.60 but I suggest that all develop branches that use old and broken utf8 facet to switch to the new one right now. Rationale of having it in Boost.Locale (a) It was already there (also wasn't header only) (b) Facets are part of C++ standard localization library (c) It is partial case of generic codecvt of any-encoding to wchar_t/char16_t/char32_t I don't really understand why this message regarding utf8 codecvt in Boost.Locale was ignored by Boost community. I hope it didn't went directly to spam. Artyom Beilis
On 23/10/2015 19:24, Robert Ramey wrote:
On 10/23/15 10:35 AM, Antony Polukhin wrote:
Hi,
The commit https://github.com/boostorg/detail/commit/9fcf2ae390c7c21a9b42fd40dd5c0eec11...
breaks the compilation of Boost.Filesystem:
OK - that's my doing
Same issue effects the serialization lib build (log attached). John.
participants (5)
-
Antony Polukhin
-
Artyom Beilis
-
John Maddock
-
Peter Dimov
-
Robert Ramey