Community
cancel
Showing results for 
Search instead for 
Did you mean: 
Fan_Xp_
Beginner
49 Views

Proposed patch 4.3 for simplifying compilation errors for tbb::atomic

Dear TBB Developers,

Please consider the attached patch for tbb43_20141009oss_src which simplifies the compilation errors for misusing tbb::atomic.

Basically, tbb users sometimes misuse tbb::atomic with unsupported types, for example.
And the compiler-error message about the misuse is not easy to understand since it exposes internal details about tbb.

Here is the compiler-error message of misusing tbb::atomic with a non-supported type with Clang 3.5.

tbb43_20141009oss/include/tbb/atomic.h:220:34: error: implicit instantiation of undefined template 'tbb::internal::aligned_storage<MyPair, 16>'
    aligned_storage<T,sizeof(T)> my_storage;
                                 ^
tbb43_20141009oss/include/tbb/atomic.h:405:16: note: in instantiation of template class 'tbb::internal::atomic_impl<MyPair>' requested here
struct atomic: internal::atomic_impl<T> {
               ^
main.cpp:11:23: note: in instantiation of template class 'tbb::atomic<MyPair>' requested here
  tbb::atomic<MyPair> b;
                      ^
tbb43_20141009oss/include/tbb/atomic.h:93:8: note: template is declared here
struct aligned_storage;

With the attached patch, the compiler-error message shall be like this, with helpful error message.

mytbb43/include/tbb/atomic.h:448:3: error: static_assert failed "tbb::atomic only supports integral type, enumeration type and pointer type"
  __TBB_STATIC_ASSERT(internal::atomic_not_support_type<T>::value, "tbb::atomic only supports integral type, enumeration type and pointer type");
  ^                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mytbb43/include/tbb/tbb_stddef.h:493:44: note: expanded from macro '__TBB_STATIC_ASSERT'
#define __TBB_STATIC_ASSERT(condition,msg) static_assert(condition,msg)
                                           ^
main.cpp:11:23: note: in instantiation of template class 'tbb::atomic<MyPair, tbb::internal::bool_constant<false> >' requested here
  tbb::atomic<MyPair> b;

I think this kind of compiler-error message will be more helpful to tbb users.

Thanks

-xuepeng

0 Kudos
5 Replies
RafSchietekat
Black Belt
49 Views

Sure, why not (to the idea of a helpful diagnostic, as I haven't inspected the patch), but that list of supported types is incomplete: the current implementation also supports floating-point types (as long as you're very careful with plus/minus zero when evaluating CAS!), and probably also non-primitive types like structs if they are not too large (right? or did I overlook something?).

(2015-03-04 Added) Well, "not too large", but also a power of 2, of course...

Fan_Xp_
Beginner
49 Views

Raf Schietekat wrote:

Sure, why not (to the idea of a helpful diagnostic, as I haven't inspected the patch), but that list of supported types is incomplete: the current implementation also supports floating-point types (as long as you're very careful with plus/minus zero when evaluating CAS!), and probably also non-primitive types like structs if they are not too large (right? or did I overlook something?).

Thanks for your comments, Raf. 

Actually, tbb::atomic does support float and primitive types like structs when they are small. However, the online document here (Type T may be an integral type, enumeration type, or a pointer type) of TBB does not show any clue about supporting float and small structs. Maybe the TBB developers wanna keep this as some "advanced tech", so they didn't document it.

For the proposed patch, it works ok with float and small structs. But I feel it's better to keep the compiler-error message consistency with the official document. And of course, I'm glad to revise the compiler-error message if the developers revise the document.

Thanks

Xuepeng

Fan_Xp_
Beginner
49 Views

Raf Schietekat wrote:

Sure, why not (to the idea of a helpful diagnostic, as I haven't inspected the patch), but that list of supported types is incomplete: the current implementation also supports floating-point types (as long as you're very careful with plus/minus zero when evaluating CAS!), and probably also non-primitive types like structs if they are not too large (right? or did I overlook something?).

Thanks for your comments, Raf. 

Actually, tbb::atomic does support float and primitive types like structs when they are small. However, the online document here (Type T may be an integral type, enumeration type, or a pointer type) of TBB does not show any clue about supporting float and small structs. Maybe the TBB developers wanna keep this as some "advanced tech", so they didn't document it.

For the proposed patch, it works ok with float and small structs. But I feel it's better to keep the compiler-error message consistency with the official document. And of course, I'm glad to revise the compiler-error message if the developers revise the document.

Thanks

Xuepeng

RafSchietekat
Black Belt
49 Views

The documentation should be updated.

Fan_Xp_
Beginner
49 Views

Raf Schietekat wrote:

The documentation should be updated.

I agree with you. Let's see how the TBB developers say.

 

Reply