Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use std::is_signed and std::enable_if #2395

Merged
merged 3 commits into from
Nov 1, 2022
Merged

Conversation

kevinbackhouse
Copy link
Collaborator

Now that we're using C++11, we can use std::is_signed and std::enable_if.

@kevinbackhouse kevinbackhouse marked this pull request as draft October 26, 2022 21:35
@kevinbackhouse kevinbackhouse added the refactoring Cleanup / Simplify code -> more readable / robust label Oct 26, 2022
@@ -73,7 +73,7 @@ const T AdditionTestValues<T, typename si::enable_if<si::is_signed<T>::VALUE>::t

template <typename T>
const bool
AdditionTestValues<T, typename si::enable_if<si::is_signed<T>::VALUE>::type>::overflow[case_count][case_count] = {
AdditionTestValues<T, typename std::enable_if<std::is_signed<T>::VALUE>::type>::overflow[case_count][case_count] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

is_signed_t

@neheb
Copy link
Collaborator

neheb commented Oct 27, 2022

--- a/unitTests/test_safe_op.cpp
+++ b/unitTests/test_safe_op.cpp
@@ -26,19 +26,19 @@ struct AdditionTestValues;
  * Overload for unsigned types.
  */
 template <typename T>
-struct AdditionTestValues<T, typename std::enable_if<!std::is_signed<T>::VALUE>::type> {
+struct AdditionTestValues<T, std::enable_if_t<!std::is_signed_v<T>>> {
   static const size_t case_count = 5;
   static const T summand[case_count];
   static const bool overflow[case_count][case_count];
 };

 template <typename T>
-const T AdditionTestValues<T, typename std::enable_if<!std::is_signed<T>::VALUE>::type>::summand[] = {
+const T AdditionTestValues<T, std::enable_if_t<!std::is_signed_v<T>>>::summand[] = {
     0, 1, 2, static_cast<T>(std::numeric_limits<T>::max() - 1), std::numeric_limits<T>::max()};

 template <typename T>
 const bool AdditionTestValues<
-    T, typename std::enable_if<!std::is_signed<T>::VALUE>::type>::overflow[case_count][case_count] = {
+    T, std::enable_if_t<!std::is_signed_v<T>>>::overflow[case_count][case_count] = {
     // 0
     {false, false, false, false, false},
     // 1
@@ -54,14 +54,14 @@ const bool AdditionTestValues<
  * Overload for signed integers
  */
 template <typename T>
-struct AdditionTestValues<T, typename std::enable_if<std::is_signed<T>::VALUE>::type> {
+struct AdditionTestValues<T, std::enable_if_t<std::is_signed_v<T>>> {
   static const size_t case_count = 8;
   static const T summand[case_count];
   static const bool overflow[case_count][case_count];
 };

 template <typename T>
-const T AdditionTestValues<T, typename std::enable_if<std::is_signed<T>::VALUE>::type>::summand[] = {
+const T AdditionTestValues<T, std::enable_if_t<std::is_signed_v<T>>>::summand[] = {
     std::numeric_limits<T>::min(),
     static_cast<T>(std::numeric_limits<T>::min() + 1),
     -1,
@@ -73,7 +73,7 @@ const T AdditionTestValues<T, typename std::enable_if<std::is_signed<T>::VALUE>:

 template <typename T>
 const bool
-    AdditionTestValues<T, typename std::enable_if<std::is_signed<T>::VALUE>::type>::overflow[case_count][case_count] = {
+    AdditionTestValues<T, std::enable_if_t<std::is_signed_v<T>>>::overflow[case_count][case_count] = {
         // min
         {true, true, true, false, false, false, false, false},
         // min + 1

@kevinbackhouse
Copy link
Collaborator Author

@neheb: thanks, I added your suggestion in commit 2.

@codecov
Copy link

codecov bot commented Oct 27, 2022

Codecov Report

Merging #2395 (d6df99e) into main (761acaf) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2395   +/-   ##
=======================================
  Coverage   64.28%   64.28%           
=======================================
  Files         119      119           
  Lines       21136    21136           
  Branches    10428    10428           
=======================================
  Hits        13587    13587           
  Misses       5400     5400           
  Partials     2149     2149           
Impacted Files Coverage Δ
src/safe_op.hpp 96.87% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@neheb
Copy link
Collaborator

neheb commented Oct 27, 2022

I’ll note this works because exiv2 is c++17. This shouldn’t be done for public facing headers where C++11 is requested.

@kevinbackhouse kevinbackhouse marked this pull request as ready for review October 27, 2022 23:05
@kevinbackhouse kevinbackhouse marked this pull request as draft October 27, 2022 23:06
@kevinbackhouse kevinbackhouse marked this pull request as ready for review October 30, 2022 21:50
@kevinbackhouse
Copy link
Collaborator Author

This PR now uses these features:

  • std::is_signed (C++11)
  • std::enable_if (C++11)
  • std::is_signed_v (C++17)
  • std::enable_if_t (C++14)

The C++14, C++17 features are only used in unitTests/test_safe_op.cpp so they won't affect our public facing headers.

@neheb
Copy link
Collaborator

neheb commented Oct 30, 2022

it looks like safe_op.hpp is a private header as well. It could use the same treatment.

@neheb
Copy link
Collaborator

neheb commented Oct 30, 2022

I also noticed test_slice.cpp has several comments indicating a bunch of code can go away. Can you take a look?

@neheb neheb merged commit dc5dc0d into Exiv2:main Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Cleanup / Simplify code -> more readable / robust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants