From 47da1121482fa86526a107165f59616f5c6b68f0 Mon Sep 17 00:00:00 2001 From: Philipp Geier Date: Mon, 24 Jul 2023 16:05:36 +0200 Subject: [PATCH 1/7] Add extended SFINAE capabilities for eckit::Translator --- src/eckit/utils/Translator.h | 77 ++++++++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/src/eckit/utils/Translator.h b/src/eckit/utils/Translator.h index 1b2e1f4bc..bcbf05b24 100644 --- a/src/eckit/utils/Translator.h +++ b/src/eckit/utils/Translator.h @@ -18,6 +18,7 @@ #include #include +#include #include namespace eckit { @@ -29,9 +30,44 @@ namespace eckit { template struct Translator { - - // Default template calls cast - To operator()(const From& from) { return To(from); } + // To allow using IsTranslatable with SFINAE it is important the the operator is templated and has a `auto` return type + // C++14 version would be with enable_if_t and decay_t. FDB5 seems not to have migrated to C++17 compilation yet. + +#if __cplusplus >= 201402L + // Test for explicit conversion through constructor (also involves implicit conversion or or user-defined conversion operator). + // Note: To(from) is called with () brackets and not with {} because it includes conversion of plain datatypes - MIR is using this + template , To>::value && (std::is_same()))>, To>::value)), + bool> = true> + auto operator()(F&& from) { + return To(std::forward(from)); + } + + // If from and to type are same - simply forward - i.e. allow moving or passing references instead of performing copies + template , To>::value && !(std::is_lvalue_reference::value && !std::is_const::value)), + bool> = true> + decltype(auto) operator()(F&& from) { + return std::forward(from); + } + + // If mutable references are passed, a const ref is returened to avoid modifications downstream... + template , To>::value && (std::is_lvalue_reference::value && !std::is_const::value)), + bool> = true> + const To& operator()(F&& from) { + return const_cast(from); + } +#else + // If conversion is possible + template ()))>::type, To>::value), + bool>::type + = true> + To operator()(F&& from) { + return To(std::forward(from)); + } +#endif }; // Those are predefined @@ -159,12 +195,12 @@ struct Translator { }; template <> -struct Translator > { +struct Translator> { std::vector operator()(const std::string&); }; template <> -struct Translator > { +struct Translator> { std::vector operator()(const std::string&); }; @@ -179,7 +215,7 @@ struct Translator, std::string> { }; template <> -struct Translator > { +struct Translator> { std::set operator()(const std::string&); }; @@ -188,15 +224,42 @@ struct Translator, std::string> { std::string operator()(const std::set&); }; +template <> +struct Translator { + std::string operator()(signed char v) { return Translator{}(static_cast(v)); }; +}; + //---------------------------------------------------------------------------------------------------------------------- // This allows using the Translator without having to explicitly name the type of an argument. For example in case of // generic string conversion: translate(someVariable) +#if __cplusplus >= 201402L +template +decltype(auto) translate(From&& from) { + return Translator::type, To>{}(std::forward(from)); +} +#else template To translate(From&& from) { - return Translator::type, To>()(std::forward(from)); + return Translator::type, To>{}(std::forward(from)); } +#endif + +//---------------------------------------------------------------------------------------------------------------------- + +#if __cplusplus >= 201703L + +// primary template handles types that do not support pre-increment: +template +struct IsTranslatable : std::false_type {}; + +// specialization recognizes types that do support pre-increment: +template +struct IsTranslatable{}(std::declval()))>> : std::true_type {}; + +#endif //---------------------------------------------------------------------------------------------------------------------- From 66f2706537fb0eda0526b9c158a5641a7e517d60 Mon Sep 17 00:00:00 2001 From: Philipp Geier Date: Fri, 15 Sep 2023 11:54:20 +0200 Subject: [PATCH 2/7] Further additions --- src/eckit/utils/VariantHelpers.h | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/eckit/utils/VariantHelpers.h diff --git a/src/eckit/utils/VariantHelpers.h b/src/eckit/utils/VariantHelpers.h new file mode 100644 index 000000000..ce24c48ee --- /dev/null +++ b/src/eckit/utils/VariantHelpers.h @@ -0,0 +1,28 @@ +#pragma once + +//----------------------------------------------------------------------------- + +namespace eckit { + +// Overload pattern for visiting std::variant using std::visit, see +// https://en.cppreference.com/w/cpp/utility/variant/visit + +// The struct Overloaded can have arbitrary many base classes (Ts ...). It publicly inherits from each class and brings +// the call operator (Ts::operator...) of each base class into its scope. The base classes need an overloaded call +// operator (Ts::operator()). + + +// Overload pattern +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +// Explicit deduction guide for the overload pattern above (not needed as of C++20) +template +Overloaded(Ts...) -> Overloaded; + +} // namespace eckit + + +//----------------------------------------------------------------------------- From e1cdc13f7b9935dd6e5a683645e1b30e7e31e888 Mon Sep 17 00:00:00 2001 From: Philipp Geier Date: Wed, 20 Sep 2023 18:08:38 +0200 Subject: [PATCH 3/7] Rename VariantHelpers.h to Overloaded.h --- src/eckit/CMakeLists.txt | 1 + src/eckit/utils/{VariantHelpers.h => Overloaded.h} | 0 2 files changed, 1 insertion(+) rename src/eckit/utils/{VariantHelpers.h => Overloaded.h} (100%) diff --git a/src/eckit/CMakeLists.txt b/src/eckit/CMakeLists.txt index 099080958..c87db4d3d 100644 --- a/src/eckit/CMakeLists.txt +++ b/src/eckit/CMakeLists.txt @@ -684,6 +684,7 @@ list( APPEND eckit_utils_srcs utils/MD5.h utils/EnumBitmask.h utils/Optional.h + utils/Overloaded.h utils/RLE.cc utils/RLE.h utils/Regex.cc diff --git a/src/eckit/utils/VariantHelpers.h b/src/eckit/utils/Overloaded.h similarity index 100% rename from src/eckit/utils/VariantHelpers.h rename to src/eckit/utils/Overloaded.h From 27aa40cb3f8e85b630e4c3b70c619929aefdef2c Mon Sep 17 00:00:00 2001 From: Philipp Geier Date: Wed, 25 Oct 2023 10:34:29 +0200 Subject: [PATCH 4/7] Full C++17 migration for Translator --- src/eckit/utils/Translator.cc | 7 +++++ src/eckit/utils/Translator.h | 49 +++++++++------------------------- tests/utils/test_translator.cc | 7 +++++ 3 files changed, 26 insertions(+), 37 deletions(-) diff --git a/src/eckit/utils/Translator.cc b/src/eckit/utils/Translator.cc index f8883ead3..62aacd3ae 100644 --- a/src/eckit/utils/Translator.cc +++ b/src/eckit/utils/Translator.cc @@ -316,12 +316,19 @@ char Translator::operator()(const std::string& s) { return s[0]; } + std::string Translator::operator()(char c) { std::string s; s = c; return s; } + +std::string Translator::operator()(signed char v) { + return Translator{}(static_cast(v)); +}; + + //---------------------------------------------------------------------------------------------------------------------- } // namespace eckit diff --git a/src/eckit/utils/Translator.h b/src/eckit/utils/Translator.h index bcbf05b24..d865c1854 100644 --- a/src/eckit/utils/Translator.h +++ b/src/eckit/utils/Translator.h @@ -30,44 +30,28 @@ namespace eckit { template struct Translator { - // To allow using IsTranslatable with SFINAE it is important the the operator is templated and has a `auto` return type - // C++14 version would be with enable_if_t and decay_t. FDB5 seems not to have migrated to C++17 compilation yet. -#if __cplusplus >= 201402L // Test for explicit conversion through constructor (also involves implicit conversion or or user-defined conversion operator). // Note: To(from) is called with () brackets and not with {} because it includes conversion of plain datatypes - MIR is using this template , To>::value && (std::is_same()))>, To>::value)), + (!std::is_same_v, To> && std::is_constructible_v), bool> = true> auto operator()(F&& from) { return To(std::forward(from)); } + // If from and to type are same - simply forward - i.e. allow moving or passing references instead of performing copies - template , To>::value && !(std::is_lvalue_reference::value && !std::is_const::value)), - bool> = true> + template , To>, + bool> = true> decltype(auto) operator()(F&& from) { - return std::forward(from); + if constexpr (std::is_lvalue_reference_v && !std::is_const_v) { + return const_cast(from); + } + else { + return std::forward(from); + } } - - // If mutable references are passed, a const ref is returened to avoid modifications downstream... - template , To>::value && (std::is_lvalue_reference::value && !std::is_const::value)), - bool> = true> - const To& operator()(F&& from) { - return const_cast(from); - } -#else - // If conversion is possible - template ()))>::type, To>::value), - bool>::type - = true> - To operator()(F&& from) { - return To(std::forward(from)); - } -#endif }; // Those are predefined @@ -226,7 +210,7 @@ struct Translator, std::string> { template <> struct Translator { - std::string operator()(signed char v) { return Translator{}(static_cast(v)); }; + std::string operator()(signed char v); }; @@ -234,21 +218,13 @@ struct Translator { // This allows using the Translator without having to explicitly name the type of an argument. For example in case of // generic string conversion: translate(someVariable) -#if __cplusplus >= 201402L template decltype(auto) translate(From&& from) { - return Translator::type, To>{}(std::forward(from)); -} -#else -template -To translate(From&& from) { - return Translator::type, To>{}(std::forward(from)); + return Translator, To>{}(std::forward(from)); } -#endif //---------------------------------------------------------------------------------------------------------------------- -#if __cplusplus >= 201703L // primary template handles types that do not support pre-increment: template @@ -259,7 +235,6 @@ template struct IsTranslatable{}(std::declval()))>> : std::true_type {}; -#endif //---------------------------------------------------------------------------------------------------------------------- diff --git a/tests/utils/test_translator.cc b/tests/utils/test_translator.cc index a51d7350e..93ba1fcf5 100644 --- a/tests/utils/test_translator.cc +++ b/tests/utils/test_translator.cc @@ -131,6 +131,13 @@ CASE("Translate strings to double") { EXPECT_THROWS_AS(t("foo555bar"), BadParameter); } + +CASE("Translate signed char as a number") { + Translator t; + + EXPECT(t((signed char)127) == std::string("127")); +} + //---------------------------------------------------------------------------------------------------------------------- } // namespace eckit::test From 952b158f362141090a0708927761149a7eae35ff Mon Sep 17 00:00:00 2001 From: Philipp Geier Date: Wed, 25 Oct 2023 17:03:44 +0200 Subject: [PATCH 5/7] Fix comments & add IsTranslatable_v --- src/eckit/utils/Translator.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/eckit/utils/Translator.h b/src/eckit/utils/Translator.h index d865c1854..851267fa8 100644 --- a/src/eckit/utils/Translator.h +++ b/src/eckit/utils/Translator.h @@ -217,7 +217,7 @@ struct Translator { //---------------------------------------------------------------------------------------------------------------------- // This allows using the Translator without having to explicitly name the type of an argument. For example in case of -// generic string conversion: translate(someVariable) +// generic string conversion: translate(someVariable) template decltype(auto) translate(From&& from) { return Translator, To>{}(std::forward(from)); @@ -226,16 +226,20 @@ decltype(auto) translate(From&& from) { //---------------------------------------------------------------------------------------------------------------------- -// primary template handles types that do not support pre-increment: +// primary template handles types that do not support translation template struct IsTranslatable : std::false_type {}; -// specialization recognizes types that do support pre-increment: +// specialization recognizes types that do support translation template struct IsTranslatable{}(std::declval()))>> : std::true_type {}; +template +inline constexpr bool IsTranslatable_v = IsTranslatable::value; + + //---------------------------------------------------------------------------------------------------------------------- } // namespace eckit From 29a19c9b483dc3168073ddf2c1c973db9f91714b Mon Sep 17 00:00:00 2001 From: Philipp Geier Date: Wed, 15 Nov 2023 13:23:25 +0100 Subject: [PATCH 6/7] Add eckit::Translator test for negative number --- tests/utils/test_translator.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/utils/test_translator.cc b/tests/utils/test_translator.cc index 93ba1fcf5..1456d49d7 100644 --- a/tests/utils/test_translator.cc +++ b/tests/utils/test_translator.cc @@ -136,6 +136,8 @@ CASE("Translate signed char as a number") { Translator t; EXPECT(t((signed char)127) == std::string("127")); + EXPECT(t((signed char)-127) == std::string("-127")); + EXPECT(t((signed char)128) == std::string("-128")); } //---------------------------------------------------------------------------------------------------------------------- From a997a9e169bda7d962edb58a399420bb81b0abe9 Mon Sep 17 00:00:00 2001 From: Philipp Geier Date: Fri, 24 Nov 2023 09:20:22 +0100 Subject: [PATCH 7/7] Test eckit::Translator through construction & implicit conversion --- tests/utils/test_translator.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/utils/test_translator.cc b/tests/utils/test_translator.cc index 1456d49d7..f46ac7207 100644 --- a/tests/utils/test_translator.cc +++ b/tests/utils/test_translator.cc @@ -16,6 +16,8 @@ #include "eckit/utils/StringTools.h" #include "eckit/utils/Translator.h" +#include "eckit/filesystem/PathName.h" + #include "eckit/testing/Test.h" using namespace std; @@ -140,6 +142,14 @@ CASE("Translate signed char as a number") { EXPECT(t((signed char)128) == std::string("-128")); } + +CASE("Translate through construction (including implicit conversion)") { + Translator t; + + EXPECT(t("123") == 123); + EXPECT(t(eckit::PathName("123")) == 123); +} + //---------------------------------------------------------------------------------------------------------------------- } // namespace eckit::test