[Test] [All Library Developers] multiple definitions
I just tried to compile something with the test library and two translation units. Boom! It was this definition, which appears in boost/test/impl/execution_monitor.ipp. signal_handler* signal_handler::s_active_handler = NULL; //!! need to be placed in thread specific storage I can hardly blame Gennadiy, though, having made this mistake myself many times. The reason I noticed this was that I have just started a simple practice that should root out these problems in my own code: I build a program that #includes all my headers in two translation units. For example: // ---- main.cpp ---- #include <boost/python.hpp> extern int f(); int main() { return f(); } // ---- main.cpp ---- #include <boost/python.hpp> int f() { return 0; } Presto! Instant linker errors pointing me at offending definitions. Every library should include a test like the one above. You may be wondering how to work around these issues when you really need header-only capability and can't stick the definition in a source file. In this case, the fix was easy: since the variable in question is a static member, just refactor it into a templated base class. class signal_handler; template <int = 0> struct signal_handler_ { static signal_handler* s_active_handler; }; class signal_handler : signal_handler_<> { ... }; //!! need to be placed in thread specific storage template <int N> signal_handler* signal_handler_<N>::s_active_handler = NULL; When you need a namespace-scope variable, things get a bit more interesting. You might consider sticking one in an unnamed namespace, but then you run a great risk of violating the One Definition Rule (see 3.2 in the standard) when identical template specializations and inline functions that use the variable are instantiated and use distinct objects in two translation units. Boost.Bind's placeholders have this problem. The answer is to initialize a *reference* in the unnamed namespace to a single, common object: namespace whatever { class foo {}; template <class T, int = 0> struct unique_instance { T& get() { static T x; return x; }; }; namespace { foo& bar = unique_instance<foo>::get(); } } -- Dave Abrahams Boost Consulting www.boost-consulting.com
"David Abrahams" <dave@boost-consulting.com> wrote in message news:u1x6xgong.fsf@boost-consulting.com...
I just tried to compile something with the test library and two translation units. Boom! It was this definition, which appears in boost/test/impl/execution_monitor.ipp.
I wonder which Boost.Test componenent were you using?
signal_handler* signal_handler::s_active_handler = NULL; //!! need to be placed in thread specific storage
I would prefer to fix this post release.
have this problem. The answer is to initialize a *reference* in the unnamed namespace to a single, common object:
namespace whatever { class foo {};
template <class T, int = 0> struct unique_instance { T& get() { static T x; return x; }; };
namespace { foo& bar = unique_instance<foo>::get(); } }
This is an idiom I am using in Boost.Test's trivial_singleton.hpp. Here is a solution for the issue above: ===================================== struct active_signal_handler_t : public singleton<active_signal_handler_t> { signal_handler* _; private: BOOST_TEST_SINGLETON_CONS( active_signal_handler_t ); }; BOOST_TEST_SINGLETON_INST( active_signal_handler ) ===================================== active_signal_handler._ is s_active_handler above. Gennadiy
"Gennadiy Rozental" <gennadiy.rozental@thomson.com> writes:
"David Abrahams" <dave@boost-consulting.com> wrote in message news:u1x6xgong.fsf@boost-consulting.com...
I just tried to compile something with the test library and two translation units. Boom! It was this definition, which appears in boost/test/impl/execution_monitor.ipp.
I wonder which Boost.Test componenent were you using?
Well, minimal test, and I now know it applies only to single TUs, but the bug would apply to multiple TUs.
signal_handler* signal_handler::s_active_handler = NULL; //!! need to be placed in thread specific storage
I would prefer to fix this post release.
I think this is a serious enought bug that it should be addressed pre-release, FWIW.
have this problem. The answer is to initialize a *reference* in the unnamed namespace to a single, common object:
namespace whatever { class foo {};
template <class T, int = 0> struct unique_instance { T& get() { static T x; return x; }; };
namespace { foo& bar = unique_instance<foo>::get(); } }
This is an idiom I am using in Boost.Test's trivial_singleton.hpp.
Here is a solution for the issue above:
===================================== struct active_signal_handler_t : public singleton<active_signal_handler_t> { signal_handler* _; private: BOOST_TEST_SINGLETON_CONS( active_signal_handler_t ); };
BOOST_TEST_SINGLETON_INST( active_signal_handler ) =====================================
active_signal_handler._ is s_active_handler above.
That's fine as long as you don't mind using member access syntax. When you need a non-member, you need something more like my solution. -- Dave Abrahams Boost Consulting www.boost-consulting.com
I just tried to compile something with the test library and two translation units. Boom! It was this definition, which appears in boost/test/impl/execution_monitor.ipp.
I wonder which Boost.Test componenent were you using?
Well, minimal test, and I now know it applies only to single TUs, but the bug would apply to multiple TUs.
Actually none of the Boost.Test inline components are supposed to be used for multiple testing modules (all supply main function()). So I do not think any fix is required. I recommend you to try auto unit test facilities. Not it's much more convinient and supports MTU. Gennadiy
"Gennadiy Rozental" <gennadiy.rozental@thomson.com> writes:
I just tried to compile something with the test library and two translation units. Boom! It was this definition, which appears in boost/test/impl/execution_monitor.ipp.
I wonder which Boost.Test componenent were you using?
Well, minimal test, and I now know it applies only to single TUs, but the bug would apply to multiple TUs.
Actually none of the Boost.Test inline components are supposed to be used for multiple testing modules (all supply main function()). So I do not think any fix is required.
I'm sorry, but when I look at the documentation for execution_monitor, I don't see "don't use this in multiple TUs." What am I missing? -- Dave Abrahams Boost Consulting www.boost-consulting.com
"David Abrahams" <dave@boost-consulting.com> wrote in message news:uekaropha.fsf@boost-consulting.com...
"Gennadiy Rozental" <gennadiy.rozental@thomson.com> writes:
I just tried to compile something with the test library and two translation units. Boom! It was this definition, which appears in boost/test/impl/execution_monitor.ipp.
I wonder which Boost.Test componenent were you using?
Well, minimal test, and I now know it applies only to single TUs, but the bug would apply to multiple TUs.
Actually none of the Boost.Test inline components are supposed to be used for multiple testing modules (all supply main function()). So I do not think any fix is required.
I'm sorry, but when I look at the documentation for execution_monitor, I don't see "don't use this in multiple TUs." What am I missing?
Nothing prevent you from using execution_monitor as a library with multiple TU. Inlined versions of Boost.Test components are intended just as a shortcuts to avoid linking in simple cases where header only solution is preferable. If you are using multiple TU based testing I don't think it should be considered as a "simple case" and linking with the library could be afforded. If you believe it's important for inlined version of execution_monitor component (by itself) to be usable in multiple TU situation (which I don't really recommend - it brings a lot of dependencies with it especially on windows) I am open for discussion, but 1. It wouldn't help you with any of existing inlined Boost.Test components 2. It's more like a new feature than bug fix and could wait till after release. Gennadiy
"Gennadiy Rozental" <gennadiy.rozental@thomson.com> writes:
"David Abrahams" <dave@boost-consulting.com> wrote in message news:uekaropha.fsf@boost-consulting.com...
"Gennadiy Rozental" <gennadiy.rozental@thomson.com> writes:
I just tried to compile something with the test library and two translation units. Boom! It was this definition, which appears in boost/test/impl/execution_monitor.ipp.
I wonder which Boost.Test componenent were you using?
Well, minimal test, and I now know it applies only to single TUs, but the bug would apply to multiple TUs.
Actually none of the Boost.Test inline components are supposed to be used for multiple testing modules (all supply main function()). So I do not think any fix is required.
I'm sorry, but when I look at the documentation for execution_monitor, I don't see "don't use this in multiple TUs." What am I missing?
Nothing prevent you from using execution_monitor as a library with multiple TU. Inlined versions of Boost.Test components are intended just as a shortcuts to avoid linking in simple cases where header only solution is preferable.
Oh, I see. You are calling these ".ipp" files "inlined," but they really have only non-inline functions and are not #included in any header files the way, IME, .ipp files normally are (to separate template implementations from their declarations).
If you are using multiple TU based testing I don't think it should be considered as a "simple case" and linking with the library could be afforded.
Sure.
If you believe it's important for inlined version of execution_monitor component (by itself) to be usable in multiple TU situation (which I don't really recommend - it brings a lot of dependencies with it especially on windows) I am open for discussion, but
1. It wouldn't help you with any of existing inlined Boost.Test components 2. It's more like a new feature than bug fix and could wait till after release.
No, I think I was just confused by the extensions on these files. Sorry about that. You might want to consider whether there's a better way to name them. -- Dave Abrahams Boost Consulting www.boost-consulting.com
Oh, I see. You are calling these ".ipp" files "inlined," but they really have only non-inline functions and are not #included in any header files the way, IME, .ipp files normally are (to separate template implementations from their declarations).
Theses are the files that contain an offline implementation, but reside amoung header files. *.cpp files and inlined component (like minimal.hpp) include these. [...]
No, I think I was just confused by the extensions on these files. Sorry about that. You might want to consider whether there's a better way to name them.
I thought you were promoting to limit number of used extensions? Gennadiy
"Gennadiy Rozental" <gennadiy.rozental@thomson.com> writes:
Oh, I see. You are calling these ".ipp" files "inlined," but they really have only non-inline functions and are not #included in any header files the way, IME, .ipp files normally are (to separate template implementations from their declarations).
Theses are the files that contain an offline implementation, but reside amoung header files. *.cpp files and inlined component (like minimal.hpp) include these.
Oh, now I feel stupid. Of course they are used in the traditional way in minimal.hpp.
No, I think I was just confused by the extensions on these files. Sorry about that. You might want to consider whether there's a better way to name them.
I thought you were promoting to limit number of used extensions?
I am. There are only 2 other choices that don't expand it ;-) And I was only asking you to _consider_ whether a change is appropriate. I probably should just quit while I'm behind, though :) -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
namespace whatever { class foo {};
template <class T, int = 0>
Wonderful solution! However, I wonder what the default template argument here (int = 0) is for?
struct unique_instance { T& get() { static T x; return x; }; };
namespace { foo& bar = unique_instance<foo>::get(); } }
Allen <yaozhen@ustc.edu> writes:
David Abrahams wrote:
namespace whatever { class foo {};
template <class T, int = 0>
Wonderful solution! However, I wonder what the default template argument here (int = 0) is for?
You might need to do this more than once for a given T. -- Dave Abrahams Boost Consulting www.boost-consulting.com
participants (3)
-
Allen -
David Abrahams -
Gennadiy Rozental