On 21/06/19 18:08 +0100, Jonathan Wakely wrote:
On 21/06/19 13:01 -0400, Nathan Sidwell wrote:
On 6/21/19 12:01 PM, Jonathan Wakely wrote:
Nathan noticed that the 'static inline' functions in <ext/atomicity.h>
cause ODR violations when used from inline functions or templates (see
[basic.def.odr] p12 bullet (12.2)). His modules branch now diagnoses
those violations, so we need a fix.
Looking at the history (r114044 by Paolo) I believe the idea was indeed
to allow different definitions to be used in different TUs. I think
__attribute__((__always_inline__)) is a better match for that than
'static inline', and doesn't violate the ODR (at least, not if all TUs
have the same values for __GTHREADS and _GLIBCXX_ATOMIC_BUILTINS).
These functions still violate this rule in [dcl.inline]:
C++17: "If a function with external linkage is declared inline in one
translation unit, it shall be declared inline in all translation units
in which it appears; no diagnostic is required."
C++2a WP: "If a function or variable with external or module linkage
is declared inline in one translation unit, there shall be a reachable
inline declaration in all translation units in which it is declared;
no diagnostic is required."
But that doesn't seem to be diagnosable by today's implementations.
Does this change seem reasonable?
yes, thanks!
Actually, I think I prefer this version. This produces identical code
to the always_inline version, but without the indirection to
additional inline functions, i.e. just inline the relevant code into
the _dispatch functions.
Tests are still running but no failures so far, as expected.
Oops, I spoke too soon:
FAIL: tr1/2_general_utilities/shared_ptr/thread/default_weaktoshared.cc (test
for excess errors)
UNRESOLVED: tr1/2_general_utilities/shared_ptr/thread/default_weaktoshared.cc
compilation failed to produce executable
FAIL: tr1/2_general_utilities/shared_ptr/thread/mutex_weaktoshared.cc (test for
excess errors)
UNRESOLVED: tr1/2_general_utilities/shared_ptr/thread/mutex_weaktoshared.cc
compilation failed to produce executable
Those tests explicitly use the __atomic_add function that the patch
removes. But those would be easy to adjust.
