Skip to content

Conversation

@jglaser
Copy link
Contributor

@jglaser jglaser commented May 12, 2020

To enable compilation of code which includes the Random123 RNG library, we need the std::make_signed<> templates, which I am adding in this PR.

Copy link
Contributor

@benbarsdell benbarsdell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

Just a few small additions needed.

template<class Ret, class... Args> struct is_function<Ret(Args......)> : true_type {}; // variadic
template<typename T> struct make_signed { typedef T type; };
template<> struct make_signed<unsigned char> { typedef signed char type; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a separate specialization for char (which is a distinct type and not necessarily signed or unsigned).

template<> struct make_signed<unsigned long long> { typedef signed long long type; };
template<typename T> struct make_unsigned { typedef T type; };
template<> struct make_unsigned<char> { typedef unsigned char type; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a separate specialization for signed char.

template<> struct make_signed<unsigned int> { typedef signed int type; };
template<> struct make_signed<unsigned long> { typedef signed long type; };
template<> struct make_signed<unsigned long long> { typedef signed long long type; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need specializations to preserve CV-qualifiers.

Suggested change
template<typename T> struct make_signed<const T> { typedef const typename make_signed<T>::type type; };
template<typename T> struct make_signed<volatile T> { typedef volatile typename make_signed<T>::type type; };
template<typename T> struct make_signed<const volatile T> { typedef const volatile typename make_signed<T>::type type; };

template<> struct make_unsigned<int> { typedef unsigned int type; };
template<> struct make_unsigned<long> { typedef unsigned long type; };
template<> struct make_unsigned<long long> { typedef unsigned long long type; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need specializations to preserve CV-qualifiers.

Suggested change
template<typename T> struct make_unsigned<const T> { typedef const typename make_unsigned<T>::type type; };
template<typename T> struct make_unsigned<volatile T> { typedef volatile typename make_unsigned<T>::type type; };
template<typename T> struct make_unsigned<const volatile T> { typedef const volatile typename make_unsigned<T>::type type; };

template<class Ret, class... Args> struct is_function<Ret(Args...)> : true_type {}; //regular
template<class Ret, class... Args> struct is_function<Ret(Args......)> : true_type {}; // variadic
template<typename T> struct make_signed { typedef T type; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a specialization for bool with no implementation.

template<> struct make_signed<unsigned long> { typedef signed long type; };
template<> struct make_signed<unsigned long long> { typedef signed long long type; };
template<typename T> struct make_unsigned { typedef T type; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a specialization for bool with no implementation.

template<class Ret, class... Args> struct is_function<Ret(Args...)> : true_type {}; //regular
template<class Ret, class... Args> struct is_function<Ret(Args......)> : true_type {}; // variadic
template<typename T> struct make_signed { typedef T type; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also can you add a TODO noting that there are more complicated rules for enums, char, wchar_t etc. that aren't implemented here.

@maddyscientist
Copy link
Collaborator

@jglaser just wondering if you'll get round to making the requested changes for this nice PR? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants