review updates:

- initialize pointer - dereference pointer instead of array access - use `bool&` as return type instead of `bool*` Co-authored-by: Tobias Ribizel <[email protected]>
ginkgo-project · Aug 16, 2023 · 5285e41 · 5285e41
1 parent 8cdb658
commit 5285e41
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 20 deletions.
diff --git a/common/unified/distributed/partition_helpers_kernels.cpp b/common/unified/distributed/partition_helpers_kernels.cpp
@@ -46,7 +46,7 @@ namespace partition_helpers {
 template <typename GlobalIndexType>
 void check_consecutive_ranges(std::shared_ptr<const DefaultExecutor> exec,
                               const array<GlobalIndexType>& range_start_ends,
-                              bool* result)
+                              bool& result)
 {
     array<uint32> result_uint32{exec, 1};
     auto num_ranges = range_start_ends.get_num_elems() / 2;
@@ -64,10 +64,10 @@ void check_consecutive_ranges(std::shared_ptr<const DefaultExecutor> exec,
             [] GKO_KERNEL(auto x) { return x; }, static_cast<uint32>(true),
             result_uint32.get_data(), num_ranges - 1,
             range_start_ends.get_const_data() + 1);
-        *result =
+        result =
             static_cast<bool>(exec->copy_val_to_host(result_uint32.get_data()));
     } else {
-        *result = true;
+        result = true;
     }
 }
 

diff --git a/core/base/copy_assignable.hpp b/core/base/copy_assignable.hpp
@@ -74,9 +74,9 @@ class copy_assignable<
         }
     }
 
-    copy_assignable(const T& obj) : obj_{new(buf)(T)(obj)} {}
+    copy_assignable(const T& obj) : obj_{new (buf)(T)(obj)} {}
 
-    copy_assignable(T&& obj) : obj_{new(buf)(T)(std::move(obj))} {}
+    copy_assignable(T&& obj) : obj_{new (buf)(T)(std::move(obj))} {}
 
     copy_assignable& operator=(const copy_assignable& other)
     {
@@ -110,16 +110,16 @@ class copy_assignable<
     template <typename... Args>
     decltype(auto) operator()(Args&&... args) const
     {
-        return obj_[0](std::forward<Args>(args)...);
+        return (*obj_)(std::forward<Args>(args)...);
     }
 
-    T const& get() const { return obj_[0]; }
+    T const& get() const { return *obj_; }
 
-    T& get() { return obj_[0]; }
+    T& get() { return *obj_; }
 
 private:
     //!< Store wrapped object on the stack, should use std::optional in c++17
-    T* obj_;
+    T* obj_{};
     alignas(T) unsigned char buf[sizeof(T)];
 };
 

diff --git a/core/distributed/partition_helpers.cpp b/core/distributed/partition_helpers.cpp
@@ -100,7 +100,7 @@ build_partition_from_local_range(std::shared_ptr<const Executor> exec,
     // check for consistency
     bool consecutive_ranges = false;
     exec->run(partition_helpers::make_check_consecutive_ranges(
-        ranges_start_end, &consecutive_ranges));
+        ranges_start_end, consecutive_ranges));
     if (!consecutive_ranges) {
         GKO_INVALID_STATE("The partition contains gaps.");
     }

diff --git a/core/distributed/partition_helpers_kernels.hpp b/core/distributed/partition_helpers_kernels.hpp
@@ -54,7 +54,7 @@ namespace kernels {
 #define GKO_DECLARE_PARTITION_HELPERS_CHECK_CONSECUTIVE_RANGES(_type)          \
     void check_consecutive_ranges(std::shared_ptr<const DefaultExecutor> exec, \
                                   const array<_type>& range_start_ends,        \
-                                  bool* result)
+                                  bool& result)
 
 
 #define GKO_DECLARE_PARTITION_HELPERS_COMPRESS_RANGES(_type)          \

diff --git a/reference/distributed/partition_helpers_kernels.cpp b/reference/distributed/partition_helpers_kernels.cpp
@@ -68,7 +68,7 @@ GKO_INSTANTIATE_FOR_EACH_INDEX_TYPE(
 template <typename GlobalIndexType>
 void check_consecutive_ranges(std::shared_ptr<const DefaultExecutor> exec,
                               const array<GlobalIndexType>& range_start_ends,
-                              bool* result)
+                              bool& result)
 {
     auto num_parts = range_start_ends.get_num_elems() / 2;
     auto start_it =
@@ -80,11 +80,11 @@ void check_consecutive_ranges(std::shared_ptr<const DefaultExecutor> exec,
     auto range_it = detail::make_zip_iterator(start_it, end_it);
 
     if (num_parts) {
-        *result = std::all_of(
+        result = std::all_of(
             range_it, range_it + num_parts - 1,
             [](const auto& r) { return std::get<0>(r) == std::get<1>(r); });
     } else {
-        *result = true;
+        result = true;
     }
 }
 

diff --git a/reference/test/distributed/partition_helpers_kernels.cpp b/reference/test/distributed/partition_helpers_kernels.cpp
@@ -107,7 +107,7 @@ TYPED_TEST(PartitionHelpers, CanCheckConsecutiveRanges)
     bool result = false;
 
     gko::kernels::reference::partition_helpers::check_consecutive_ranges(
-        this->ref, range_start_ends, &result);
+        this->ref, range_start_ends, result);
 
     ASSERT_TRUE(result);
 }
@@ -121,7 +121,7 @@ TYPED_TEST(PartitionHelpers, CanCheckNonConsecutiveRanges)
     bool result = true;
 
     gko::kernels::reference::partition_helpers::check_consecutive_ranges(
-        this->ref, range_start_ends, &result);
+        this->ref, range_start_ends, result);
 
     ASSERT_FALSE(result);
 }

diff --git a/test/distributed/partition_helper_kernels.cpp b/test/distributed/partition_helper_kernels.cpp
@@ -175,7 +175,7 @@ TYPED_TEST(PartitionHelpers, CanCheckConsecutiveRanges)
     bool result = false;
 
     gko::kernels::EXEC_NAMESPACE::partition_helpers::check_consecutive_ranges(
-        this->exec, offsets, &result);
+        this->exec, offsets, result);
 
     ASSERT_TRUE(result);
 }
@@ -191,7 +191,7 @@ TYPED_TEST(PartitionHelpers, CanCheckNonConsecutiveRanges)
     bool result = true;
 
     gko::kernels::EXEC_NAMESPACE::partition_helpers::check_consecutive_ranges(
-        this->exec, start_ends, &result);
+        this->exec, start_ends, result);
 
     ASSERT_FALSE(result);
 }
@@ -204,7 +204,7 @@ TYPED_TEST(PartitionHelpers, CanCheckConsecutiveRangesWithSingleRange)
     bool result = false;
 
     gko::kernels::EXEC_NAMESPACE::partition_helpers::check_consecutive_ranges(
-        this->exec, start_ends, &result);
+        this->exec, start_ends, result);
 
     ASSERT_TRUE(result);
 }
@@ -217,7 +217,7 @@ TYPED_TEST(PartitionHelpers, CanCheckConsecutiveRangesWithSingleElement)
     bool result = false;
 
     gko::kernels::EXEC_NAMESPACE::partition_helpers::check_consecutive_ranges(
-        this->exec, start_ends, &result);
+        this->exec, start_ends, result);
 
     ASSERT_TRUE(result);
 }