Add declarations of explicit specializations and instantiations. #428
Conversation
Currently, code that uses the specializations in question will accidentally and erroneously specialize the primary template. The reason this appears to "work" is a pure accident due to the way the linker currently works. The code violates the one-definition-rule, and all programs depending on it are ill-formed, no diagnostic required (i.e. running all such programs has undefined behaviour). Additionally, declaring explicit instantiations is also good practice, since it declares intent and prevents accidental later explicit specializations (it would be an error to declare those after the extern declaration of the explicit instantiation had been seen). Explicit specializations and instantiations are declared in .hpp files near the respective template. (Found by @tkoeppe by compiling with clang -Wundefined-func-template.)
|
Ack. Thanks for the PR. I'll merge it in as soon as possible. I will port this to 0.19 (mnt-v0 branch) as well. |
|
Thanks!
…On Wed, Mar 27, 2019, 10:05 Roma Dubtsov ***@***.***> wrote:
Ack. Thanks for the PR. I'll merge it in as soon as possible. I will port
this to 0.19 (mnt-v0 branch) as well.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#428 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AHJUTeQ2xqAkKA4LyrN2ztx0g1pR7Cv_ks5va6TggaJpZM4cAYXq>
.
|
Made named namespace for uni_bnorm_driver_t
|
@jktomer: I've reverted the patches because they, among other things, cause gcc to fail with internal compiler error. How important for you to have these changes? |
|
@rsdubtso: (original author here) They're not essential, just nice-to-have and a demonstration of best practices for using templates. A large part of the value is probably people copy-pasting good examples. But it's OK to not have them. Are the crashes only on very old compilers (e.g. GCC 4.8)? Hopefully there are bug reports for the vendor? |
|
@tkoeppe: Thanks! The crashes are on 8.x line. Here's the bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89919 |
|
According to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89919 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87770, this has been fixed, but I'm not quite sure for which version (8.3? 8.4?). @rsdubtso: Could you perhaps check if this change can now be applied? |
|
@rsdubtso: Hello :-) Could we try to reapply these patches perhaps? |
|
Hi @tkoeppe. Unfortunately, Roman is not with oneDNN team any more. If you have any issues with oneDNN, please file a new one. It's easier for us to help on open trackers over merged. Thank you. |
Currently, code that uses the specializations in question will
accidentally and erroneously specialize the primary template. The
reason this appears to "work" is a pure accident due to the way the
linker currently works. The code violates the one-definition-rule,
and all programs depending on it are ill-formed, no diagnostic
required (i.e. running all such programs has undefined behaviour).
Additionally, declaring explicit instantiations is also good
practice, since it declares intent and prevents accidental later
explicit specializations (it would be an error to declare those
after the extern declaration of the explicit instantiation had been
seen).
Explicit specializations and instantiations are declared in .hpp
files near the respective template.
(Found by @tkoeppe by compiling with clang -Wundefined-func-template.)