diff --git a/include/ceres/codegen/internal/expression_ref.h b/include/ceres/codegen/internal/expression_ref.h index 1bdc3b537..5a13d761e 100644 --- a/include/ceres/codegen/internal/expression_ref.h +++ b/include/ceres/codegen/internal/expression_ref.h @@ -33,6 +33,7 @@ #define CERES_PUBLIC_EXPRESSION_REF_H_ #include + #include "ceres/codegen/internal/expression.h" #include "ceres/codegen/internal/types.h" @@ -86,23 +87,6 @@ struct ExpressionRef { ExpressionRef(const ExpressionRef& other); ExpressionRef& operator=(const ExpressionRef& other); - // Similar to the copy assignment above, but if 'this' is uninitialized, we - // can remove the copy and therefore eliminate one expression in the graph. - // For example: - // T c; - // c = a + b; - // will generate - // v_2 = v_0 + v_1 - // instead of an additional assigment from the temporary 'a + b' to 'c'. In - // C++ this concept is called "Copy Elision". This is used by the compiler to - // eliminate copies, for example, in a function that returns an object by - // value. We implement it ourself here, because large parts of copy elision - // are implementation defined, which means that every compiler can do it - // differently. More information on copy elision can be found here: - // https://en.cppreference.com/w/cpp/language/copy_elision - ExpressionRef(ExpressionRef&& other); - ExpressionRef& operator=(ExpressionRef&& other); - // Compound operators ExpressionRef& operator+=(const ExpressionRef& x); ExpressionRef& operator-=(const ExpressionRef& x); diff --git a/internal/ceres/codegen/expression_ref_test.cc b/internal/ceres/codegen/expression_ref_test.cc index 30672ad84..0a70d42e3 100644 --- a/internal/ceres/codegen/expression_ref_test.cc +++ b/internal/ceres/codegen/expression_ref_test.cc @@ -34,6 +34,7 @@ #define CERES_CODEGEN #include "ceres/codegen/internal/expression_ref.h" + #include "ceres/codegen/internal/expression_graph.h" #include "gtest/gtest.h" @@ -112,18 +113,6 @@ TEST(ExpressionRef, AssignmentCreate) { EXPECT_EQ(reference, graph); } -TEST(ExpressionRef, MoveAssignmentCreate) { - StartRecordingExpressions(); - T a = 2; - T b = std::move(a); - auto graph = StopRecordingExpressions(); - EXPECT_EQ(graph.Size(), 1); - - ExpressionGraph reference; - reference.InsertBack(Expression::CreateCompileTimeConstant(2)); - EXPECT_EQ(reference, graph); -} - TEST(ExpressionRef, MoveAssignment) { StartRecordingExpressions(); T a = 1; diff --git a/internal/ceres/expression_ref.cc b/internal/ceres/expression_ref.cc index 52b7e0ba9..dda412ae9 100644 --- a/internal/ceres/expression_ref.cc +++ b/internal/ceres/expression_ref.cc @@ -76,41 +76,6 @@ ExpressionRef& ExpressionRef::operator=(const ExpressionRef& other) { return *this; } -ExpressionRef::ExpressionRef(ExpressionRef&& other) { - *this = std::move(other); -} - -ExpressionRef& ExpressionRef::operator=(ExpressionRef&& other) { - // Assigning an uninitialized variable to another variable is an error. - CHECK(other.IsInitialized()) << "Uninitialized Assignment."; - - if (IsInitialized()) { - // Create assignment from other -> this - AddExpressionToGraph(Expression::CreateAssignment(id, other.id)); - } else { - // Special case: 'this' is uninitialized and other is an rvalue. - // -> Implement copy elision by only setting the reference - // This reduces the number of generated expressions roughly by a factor - // of 2. For example, in the following statement: - // T c = a + b; - // The result of 'a + b' is an rvalue reference to ExpressionRef. - // Therefore, the move constructor of 'c' is called. Since 'c' is also - // uninitialized, this branch here is taken and the copy is removed. After - // this function 'c' will just point to the temporary created by the 'a + - // b' expression. This is valid, because we don't have any scoping - // information and therefore assume global scope for all temporary - // variables. The generated code for the single statement above, is: - // v_2 = v_0 + v_1; // With c.id = 2 - // Without this move constructor the following two lines would be - // generated: - // v_2 = v_0 + v_1; - // v_3 = v_2; // With c.id = 3 - id = other.id; - } - other.id = kInvalidExpressionId; - return *this; -} - // Compound operators ExpressionRef& ExpressionRef::operator+=(const ExpressionRef& x) { *this = *this + x;