On Tue, Apr 7, 2009 at 1:55 PM, Anthony Williams <anthony.ajw@gmail.com> wrote:
Emil Dotchevski <emildotchevski@gmail.com> writes:
On Tue, Apr 7, 2009 at 8:57 AM, Anthony Williams <anthony.ajw@gmail.com> wrote:
joaquin@tid.es writes:
Looks like a sensible part of Boost.Threads, namely that dealing with mutexes and locks, would be header-only but for its relying on boost/thread/exceptions.hpp, whose implementation is located at /libs/thread/*/exceptions.cpp.
Given that this .cpp mostly consists of extremely simple definitions (as simple as do-nothing functions in many cases), would it make sense to move this to inline definitions in boost/thread/exceptions.hpp thus making mutexes and locks header-only? This would greatly enhance the appeal of this part of Boost.Threads, as having to link a separate module is a considerable burden.
That seems sensible.
I am against such a move. Boost Threads requires linking for other features which makes it one of the few libraries in Boost that can be properly designed to avoid unnecessary physical coupling. Unless something is proven to cause performance problems it should not be inlined, regardless of how simple it is.
Wow, that's a hard line you've drawn there. I'm not sure I agree. The only reason this matters is if you're going to be changing the implementation and don't want to recompile the code that uses the header. For boost users, the implementation only changes if they change boost versions, and in that case I would expect people to recompile anyway --- I wouldn't trust something compiled against boost 1.37 to link against a 1.38 lib, for example.
You're arguing that physical coupling to something like Boost isn't a problem. I'm arguing that introducing any physical coupling must be justified. From certain point of view both arguments are valid, which is why we disagree. :)
In any case, after further thought I remembered the reason I didn't do this before. If the exceptions are header-only then they cannot be thrown from a DLL and caught outside the DLL, since the type-ids won't match. This would mean that any exceptions thrown by the DLL version of boost.thread wouldn't be able to be caught in user code other than by catching std::exception or with catch(...). This is the case for MSVC/Windows anyway --- I'm not sure about other compilers/platforms.
I'm not sure I understand why the type IDs wont match, but I do believe you.
I could change the code so that if you're linking against the static library then the exception functions are inline, since they can't be thrown across DLL boundaries anyway in this case. That would mean that if you were static linking against boost.thread then you wouldn't actually need the lib in some cases (e.g. if all you used was boost::mutex).
In my book this is another example of unnecessary physical coupling. If I'm calling Boost Thread from an exception-neutral context, I'd rather not see definitions of exception types (not that exceptions are in any way special, this is just an example.) Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode