Merge restricting gko::share to rvalue references

Previously, `gko::share()` is allowed to act on regular references,
which leads some users to incorrectly use `gko::share`.
By restricting it to rvalue references, it is harder to misuse this
function while still allowing the same functionality with
`gko::share(gko::give(val))` or `gko::share(std::move(val))` (both do
the exact same thing).

Note: This does break the public interface as it is no longer possible
      to call `gko::share` with lvalues of any sort. However, I do
      consider it a bug that it was possible previously.
      In its previous form, writing errors accidentally is too easy,
      especially when examples are extended / modified.

Related PR: #1020

core/preconditioner/isai.cpp

@@ -225,8 +225,7 @@ void Isai<IsaiType, ValueType, IndexType>::generate_inverse(
             auto rhs_copy = gko::clone(exec->get_master(), excess_rhs);
             std::shared_ptr<LinOpFactory> excess_solver_factory;
             if (parameters_.excess_solver_factory) {
-                excess_solver_factory =
-                    share(parameters_.excess_solver_factory);
+                excess_solver_factory = parameters_.excess_solver_factory;
                 excess_solution->copy_from(excess_rhs.get());
             } else if (is_general || is_spd) {
                 excess_solver_factory =

core/test/base/utils.cpp

@@ -163,25 +163,41 @@ TEST(Share, SharesSharedPointer)
     std::shared_ptr<Derived> p(new Derived());
     auto plain = p.get();
 
-    auto shared = gko::share(p);
+    auto shared = gko::share(std::move(p));
 
     ::testing::StaticAssertTypeEq<decltype(shared), std::shared_ptr<Derived>>();
     ASSERT_EQ(plain, shared.get());
 }
 
 
+TEST(Share, SharesTemporarySharedPointer)
+{
+    auto shared = gko::share(std::make_shared<Derived>());
+
+    ::testing::StaticAssertTypeEq<decltype(shared), std::shared_ptr<Derived>>();
+}
+
+
 TEST(Share, SharesUniquePointer)
 {
     std::unique_ptr<Derived> p(new Derived());
     auto plain = p.get();
 
-    auto shared = gko::share(p);
+    auto shared = gko::share(std::move(p));
 
     ::testing::StaticAssertTypeEq<decltype(shared), std::shared_ptr<Derived>>();
     ASSERT_EQ(plain, shared.get());
 }
 
 
+TEST(Share, SharesTemporaryUniquePointer)
+{
+    auto shared = gko::share(std::make_unique<Derived>());
+
+    ::testing::StaticAssertTypeEq<decltype(shared), std::shared_ptr<Derived>>();
+}
+
+
 TEST(Give, GivesSharedPointer)
 {
     std::shared_ptr<Derived> p(new Derived());

core/test/preconditioner/isai.cpp

@@ -261,80 +261,75 @@ TYPED_TEST(IsaiFactory, CanSetExcessSolverFactoryU)
 TYPED_TEST(IsaiFactory, ThrowsWrongDimensionA)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = Csr::create(this->exec, gko::dim<2>{1, 2}, 1);
+    auto mtx = gko::share(Csr::create(this->exec, gko::dim<2>{1, 2}, 1));
 
-    ASSERT_THROW(this->general_isai_factory->generate(gko::share(mtx)),
+    ASSERT_THROW(this->general_isai_factory->generate(mtx),
                  gko::DimensionMismatch);
 }
 
 
 TYPED_TEST(IsaiFactory, ThrowsWrongDimensionSpd)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = Csr::create(this->exec, gko::dim<2>{1, 2}, 1);
+    auto mtx = gko::share(Csr::create(this->exec, gko::dim<2>{1, 2}, 1));
 
-    ASSERT_THROW(this->spd_isai_factory->generate(gko::share(mtx)),
-                 gko::DimensionMismatch);
+    ASSERT_THROW(this->spd_isai_factory->generate(mtx), gko::DimensionMismatch);
 }
 
 
 TYPED_TEST(IsaiFactory, ThrowsWrongDimensionL)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = Csr::create(this->exec, gko::dim<2>{1, 2}, 1);
+    auto mtx = gko::share(Csr::create(this->exec, gko::dim<2>{1, 2}, 1));
 
-    ASSERT_THROW(this->lower_isai_factory->generate(gko::share(mtx)),
+    ASSERT_THROW(this->lower_isai_factory->generate(mtx),
                  gko::DimensionMismatch);
 }
 
 
 TYPED_TEST(IsaiFactory, ThrowsWrongDimensionU)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = Csr::create(this->exec, gko::dim<2>{1, 2}, 1);
+    auto mtx = gko::share(Csr::create(this->exec, gko::dim<2>{1, 2}, 1));
 
-    ASSERT_THROW(this->upper_isai_factory->generate(gko::share(mtx)),
+    ASSERT_THROW(this->upper_isai_factory->generate(mtx),
                  gko::DimensionMismatch);
 }
 
 
 TYPED_TEST(IsaiFactory, ThrowsNoConversionCsrA)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = DummyOperator::create(this->exec, gko::dim<2>{2, 2});
+    auto mtx = gko::share(DummyOperator::create(this->exec, gko::dim<2>{2, 2}));
 
-    ASSERT_THROW(this->general_isai_factory->generate(gko::share(mtx)),
-                 gko::NotSupported);
+    ASSERT_THROW(this->general_isai_factory->generate(mtx), gko::NotSupported);
 }
 
 
 TYPED_TEST(IsaiFactory, ThrowsNoConversionCsrSpd)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = DummyOperator::create(this->exec, gko::dim<2>{2, 2});
+    auto mtx = gko::share(DummyOperator::create(this->exec, gko::dim<2>{2, 2}));
 
-    ASSERT_THROW(this->spd_isai_factory->generate(gko::share(mtx)),
-                 gko::NotSupported);
+    ASSERT_THROW(this->spd_isai_factory->generate(mtx), gko::NotSupported);
 }
 
 
 TYPED_TEST(IsaiFactory, ThrowsNoConversionCsrL)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = DummyOperator::create(this->exec, gko::dim<2>{2, 2});
+    auto mtx = gko::share(DummyOperator::create(this->exec, gko::dim<2>{2, 2}));
 
-    ASSERT_THROW(this->lower_isai_factory->generate(gko::share(mtx)),
-                 gko::NotSupported);
+    ASSERT_THROW(this->lower_isai_factory->generate(mtx), gko::NotSupported);
 }
 
 
 TYPED_TEST(IsaiFactory, ThrowsNoConversionCsrU)
 {
     using Csr = typename TestFixture::Csr;
-    auto mtx = DummyOperator::create(this->exec, gko::dim<2>{2, 2});
+    auto mtx = gko::share(DummyOperator::create(this->exec, gko::dim<2>{2, 2}));
 
-    ASSERT_THROW(this->upper_isai_factory->generate(gko::share(mtx)),
-                 gko::NotSupported);
+    ASSERT_THROW(this->upper_isai_factory->generate(mtx), gko::NotSupported);
 }
 
 

core/test/solver/ir.cpp

@@ -76,7 +76,7 @@ class Ir : public ::testing::Test {
 
     std::shared_ptr<const gko::Executor> exec;
     std::shared_ptr<Mtx> mtx;
-    std::unique_ptr<typename Solver::Factory> ir_factory;
+    std::shared_ptr<typename Solver::Factory> ir_factory;
     std::unique_ptr<gko::LinOp> solver;
 
     static void assert_same_matrices(const Mtx* m1, const Mtx* m2)
@@ -374,7 +374,7 @@ TYPED_TEST(Ir, DefaultSmootherBuildWithSolver)
 {
     using value_type = typename TestFixture::value_type;
     using Solver = typename TestFixture::Solver;
-    auto solver = gko::as<Solver>(share(this->solver));
+    auto solver = gko::as<Solver>(share(std::move(this->solver)));
 
     auto smoother_factory = gko::solver::build_smoother<value_type>(solver);
     auto criteria =
@@ -394,7 +394,7 @@ TYPED_TEST(Ir, DefaultSmootherBuildWithFactory)
 {
     using value_type = typename TestFixture::value_type;
     using Solver = typename TestFixture::Solver;
-    auto factory = share(this->ir_factory);
+    auto factory = this->ir_factory;
 
     auto smoother_factory = gko::solver::build_smoother<value_type>(factory);
     auto criteria =
@@ -413,7 +413,7 @@ TYPED_TEST(Ir, SmootherBuildWithSolver)
 {
     using value_type = typename TestFixture::value_type;
     using Solver = typename TestFixture::Solver;
-    auto solver = gko::as<Solver>(share(this->solver));
+    auto solver = gko::as<Solver>(gko::share(std::move(this->solver)));
 
     auto smoother_factory =
         gko::solver::build_smoother<value_type>(solver, 3, value_type{0.5});
@@ -434,7 +434,7 @@ TYPED_TEST(Ir, SmootherBuildWithFactory)
 {
     using value_type = typename TestFixture::value_type;
     using Solver = typename TestFixture::Solver;
-    auto factory = share(this->ir_factory);
+    auto factory = this->ir_factory;
 
     auto smoother_factory =
         gko::solver::build_smoother<value_type>(factory, 3, value_type{0.5});

core/test/stop/combined.cpp

@@ -150,21 +150,23 @@ TEST_F(Combined, FunctionCanThrowAllNullptr)
 
 TEST_F(Combined, FunctionCanThrowFirstIsInvalid)
 {
-    auto stop =
-        gko::stop::Iteration::build().with_max_iters(test_iterations).on(exec_);
+    auto stop = gko::share(gko::stop::Iteration::build()
+                               .with_max_iters(test_iterations)
+                               .on(exec_));
     std::vector<std::shared_ptr<const gko::stop::CriterionFactory>>
-        criterion_vec{nullptr, gko::share(stop)};
+        criterion_vec{nullptr, stop};
 
     ASSERT_THROW(gko::stop::combine(criterion_vec), gko::NotSupported);
 }
 
 
 TEST_F(Combined, FunctionCanIgnoreNullptr)
 {
-    auto stop =
-        gko::stop::Iteration::build().with_max_iters(test_iterations).on(exec_);
+    auto stop = gko::share(gko::stop::Iteration::build()
+                               .with_max_iters(test_iterations)
+                               .on(exec_));
     std::vector<std::shared_ptr<const gko::stop::CriterionFactory>>
-        criterion_vec{gko::share(stop), nullptr};
+        criterion_vec{stop, nullptr};
     auto combined = gko::stop::combine(criterion_vec);
 
     ASSERT_NO_THROW(combined->generate(nullptr, nullptr, nullptr));

examples/adaptiveprecision-blockjacobi/adaptiveprecision-blockjacobi.cpp

@@ -107,11 +107,11 @@ int main(int argc, char* argv[])
     // copy b again
     b->copy_from(host_x.get());
     const RealValueType reduction_factor = 1e-7;
-    auto iter_stop =
-        gko::stop::Iteration::build().with_max_iters(10000u).on(exec);
-    auto tol_stop = gko::stop::ResidualNorm<ValueType>::build()
-                        .with_reduction_factor(reduction_factor)
-                        .on(exec);
+    auto iter_stop = gko::share(
+        gko::stop::Iteration::build().with_max_iters(10000u).on(exec));
+    auto tol_stop = gko::share(gko::stop::ResidualNorm<ValueType>::build()
+                                   .with_reduction_factor(reduction_factor)
+                                   .on(exec));
 
     std::shared_ptr<const gko::log::Convergence<ValueType>> logger =
         gko::log::Convergence<ValueType>::create(exec);
@@ -121,7 +121,7 @@ int main(int argc, char* argv[])
     // Create solver factory
     auto solver_gen =
         cg::build()
-            .with_criteria(gko::share(iter_stop), gko::share(tol_stop))
+            .with_criteria(iter_stop, tol_stop)
             // Add preconditioner, these 2 lines are the only
             // difference from the simple solver example
             .with_preconditioner(bj::build()

examples/ilu-preconditioned-solver/ilu-preconditioned-solver.cpp

@@ -95,7 +95,7 @@ int main(int argc, char* argv[])
     auto par_ilu_fact =
         gko::factorization::ParIlu<ValueType, IndexType>::build().on(exec);
     // Generate concrete factorization for input matrix
-    auto par_ilu = par_ilu_fact->generate(A);
+    auto par_ilu = gko::share(par_ilu_fact->generate(A));
 
     // Generate an ILU preconditioner factory by setting lower and upper
     // triangular solver - in this case the exact triangular solves
@@ -106,7 +106,7 @@ int main(int argc, char* argv[])
             .on(exec);
 
     // Use incomplete factors to generate ILU preconditioner
-    auto ilu_preconditioner = ilu_pre_factory->generate(gko::share(par_ilu));
+    auto ilu_preconditioner = gko::share(ilu_pre_factory->generate(par_ilu));
 
     // Use preconditioner inside GMRES solver factory
     // Generating a solver factory tied to a specific preconditioner makes sense
@@ -120,7 +120,7 @@ int main(int argc, char* argv[])
                 gko::stop::ResidualNorm<ValueType>::build()
                     .with_reduction_factor(reduction_factor)
                     .on(exec))
-            .with_generated_preconditioner(gko::share(ilu_preconditioner))
+            .with_generated_preconditioner(ilu_preconditioner)
             .on(exec);
 
     // Generate preconditioned solver for a specific target system

examples/ir-ilu-preconditioned-solver/ir-ilu-preconditioned-solver.cpp

@@ -105,21 +105,21 @@ int main(int argc, char* argv[])
     auto par_ilu_fact =
         gko::factorization::ParIlu<ValueType, IndexType>::build().on(exec);
     // Generate concrete factorization for input matrix
-    auto par_ilu = par_ilu_fact->generate(A);
+    auto par_ilu = gko::share(par_ilu_fact->generate(A));
 
     // Generate an iterative refinement factory to be used as a triangular
     // solver in the preconditioner application. The generated method is
     // equivalent to doing five block-Jacobi sweeps with a maximum block size
     // of 16.
-    auto bj_factory =
+    auto bj_factory = gko::share(
         bj::build()
             .with_max_block_size(16u)
             .with_storage_optimization(gko::precision_reduction::autodetect())
-            .on(exec);
+            .on(exec));
 
     auto trisolve_factory =
         ir::build()
-            .with_solver(share(bj_factory))
+            .with_solver(bj_factory)
             .with_criteria(
                 gko::stop::Iteration::build().with_max_iters(sweeps).on(exec))
             .on(exec);
@@ -134,15 +134,15 @@ int main(int argc, char* argv[])
             .on(exec);
 
     // Use incomplete factors to generate ILU preconditioner
-    auto ilu_preconditioner = ilu_pre_factory->generate(gko::share(par_ilu));
+    auto ilu_preconditioner = gko::share(ilu_pre_factory->generate(par_ilu));
 
     // Create stopping criteria for Gmres
     const RealValueType reduction_factor{1e-12};
-    auto iter_stop =
-        gko::stop::Iteration::build().with_max_iters(1000u).on(exec);
-    auto tol_stop = gko::stop::ResidualNorm<ValueType>::build()
-                        .with_reduction_factor(reduction_factor)
-                        .on(exec);
+    auto iter_stop = gko::share(
+        gko::stop::Iteration::build().with_max_iters(1000u).on(exec));
+    auto tol_stop = gko::share(gko::stop::ResidualNorm<ValueType>::build()
+                                   .with_reduction_factor(reduction_factor)
+                                   .on(exec));
 
     std::shared_ptr<const gko::log::Convergence<ValueType>> logger =
         gko::log::Convergence<ValueType>::create(exec);
@@ -155,8 +155,8 @@ int main(int argc, char* argv[])
     // solver+preconditioner combination is expected to be effective.
     auto ilu_gmres_factory =
         gmres::build()
-            .with_criteria(gko::share(iter_stop), gko::share(tol_stop))
-            .with_generated_preconditioner(gko::share(ilu_preconditioner))
+            .with_criteria(iter_stop, tol_stop)
+            .with_generated_preconditioner(ilu_preconditioner)
             .on(exec);
 
     // Generate preconditioned solver for a specific target system

examples/iterative-refinement/iterative-refinement.cpp

@@ -109,11 +109,12 @@ int main(int argc, char* argv[])
     b->copy_from(host_x.get());
     gko::size_type max_iters = 10000u;
     RealValueType outer_reduction_factor{1e-12};
-    auto iter_stop =
-        gko::stop::Iteration::build().with_max_iters(max_iters).on(exec);
-    auto tol_stop = gko::stop::ResidualNorm<ValueType>::build()
-                        .with_reduction_factor(outer_reduction_factor)
-                        .on(exec);
+    auto iter_stop = gko::share(
+        gko::stop::Iteration::build().with_max_iters(max_iters).on(exec));
+    auto tol_stop =
+        gko::share(gko::stop::ResidualNorm<ValueType>::build()
+                       .with_reduction_factor(outer_reduction_factor)
+                       .on(exec));
 
     std::shared_ptr<const gko::log::Convergence<ValueType>> logger =
         gko::log::Convergence<ValueType>::create(exec);
@@ -131,7 +132,7 @@ int main(int argc, char* argv[])
                             .with_reduction_factor(inner_reduction_factor)
                             .on(exec))
                     .on(exec))
-            .with_criteria(gko::share(iter_stop), gko::share(tol_stop))
+            .with_criteria(iter_stop, tol_stop)
             .on(exec);
     // Create solver
     auto solver = solver_gen->generate(A);