536 copying vector by assignment multiple times fails asserts

include/graphblas/bsp1d/io.hpp

@@ -504,6 +504,13 @@ namespace grb {
 	) noexcept {
 		const size_t n = size( x );
 		const size_t old_nnz = nnz( x );
+
+		// dynamic checks
+		if( (descr & descriptors::dense) && nnz( x ) < n ) {
+			return ILLEGAL;
+		}
+
+		// capacity check
 		if( capacity( x ) < n ) {
 			if( phase == RESIZE ) {
 				return resize( x, n );
@@ -517,16 +524,20 @@ namespace grb {
 			}
 		}
 
+		// handle trivial resize
 		assert( capacity( x ) == n );
 		if( phase == RESIZE ) {
 			return SUCCESS;
 		}
 
+		// dispatch
 		assert( phase == EXECUTE );
 		RC ret = internal::set_handle_use_index< descr >( x, old_nnz, val );
 		if( ret == SUCCESS ) {
 			internal::setDense( x );
 		}
+
+		// done
 		return ret;
 	}
 
@@ -622,7 +633,7 @@ namespace grb {
 			return MISMATCH;
 		}
 		if( descr & descriptors::dense ) {
-			if( nnz( y ) < size( y ) ) {
+			if( nnz( x ) < size( x ) || nnz( y ) < size( y ) ) {
 				return ILLEGAL;
 			}
 		}
@@ -699,7 +710,10 @@ namespace grb {
 			return MISMATCH;
 		}
 		if( descr & descriptors::dense ) {
-			if( nnz( y ) < size( y ) || nnz( mask ) < size( mask ) ) {
+			if( nnz( x ) < size( x ) ||
+				nnz( y ) < size( y ) ||
+				nnz( mask ) < size( mask )
+			) {
 				return ILLEGAL;
 			}
 		}
@@ -765,11 +779,21 @@ namespace grb {
 			return MISMATCH;
 		}
 
+		// dynamic checks
+		if( (descr & descriptors::dense) && nnz( x ) < size( x ) ) {
+			return ILLEGAL;
+		}
+		if( (descr & descriptors::dense) && nnz( mask ) < size( mask ) ) {
+			return ILLEGAL;
+		}
+
 		// on capacity pre-check, see above
 
 		// all OK, try to do assignment
-		RC ret = set< descr >( internal::getLocal( x ),
-			internal::getLocal( mask ), y, phase );
+		RC ret = set< descr >(
+			internal::getLocal( x ),
+			internal::getLocal( mask ), y, phase
+		);
 
 		if( collectives< BSP1D >::allreduce( ret, operators::any_or< RC >() )
 			!= SUCCESS

include/graphblas/bsp1d/vector.hpp

@@ -618,14 +618,15 @@ namespace grb {
 				const size_t bufferSize =
 					internal::Coordinates< _GRB_BSP1D_BACKEND >::bufferSize( _local_n ) +
 					internal::Coordinates< _GRB_BSP1D_BACKEND >::bufferSize( cap_in );
+				// allocate raw, assigned, and stack arrays
 				const RC rc = grb::utils::alloc(
 					"grb::Vector< T, BSP1D, C > (initialize)", sstream.str(),
-					_raw, cap_in, true, _raw_deleter,                      // allocate raw array
+					_raw, cap_in, true, _raw_deleter,
 					new_assigned,
 						internal::Coordinates< _GRB_BSP1D_BACKEND >::arraySize( cap_in ),
 						true,
-						_assigned_deleter,                             // allocate assigned array
-					_buffer, bufferSize, true, _buffer_deleter             // allocate (stack) buffer
+						_assigned_deleter,
+					_buffer, bufferSize, true, _buffer_deleter
 				);
 				// identify error and throw
 				if( rc == OUTOFMEM ) {
@@ -2439,6 +2440,26 @@ namespace grb {
 			// done
 		}
 
+		/**
+		 * Copy-assignment.
+		 *
+		 * Same performance semantics as #grb::set.
+		 *
+		 * \warning Errors will be thrown as standard C++ exceptions. Users who rather
+		 *          not deal with exceptions are encouraged to use #grb::set directly.
+		 *
+		 * \internal Dispatches to #grb::set.
+		 */
+		Vector< D, BSP1D, C > & operator=( Vector< D, BSP1D, C > &x ) {
+			const auto rc = set( *this, x );
+			if( rc != SUCCESS ) {
+				throw std::runtime_error( "grb::set inside copy-constructor: "
+					+ toString( rc )
+				);
+			}
+			return *this;
+		}
+
 		/**
 		 * Assign-from-temporary. This is a \f$ \Theta(1) \f$ operation.
 		 *

include/graphblas/reference/vector.hpp

@@ -651,7 +651,8 @@ namespace grb {
 				 * given container. If it is equal, this iterator will be set to its end
 				 * position.
 				 */
-				ConstIterator( const Vector< D, reference, MyCoordinates > &in,
+				ConstIterator(
+					const Vector< D, reference, MyCoordinates > &in,
 					size_t initial = 0,
 					size_t processID = 0, size_t numProcesses = 1
 				) noexcept :
@@ -860,7 +861,8 @@ namespace grb {
 			 */
 			Vector( const Vector< D, reference, MyCoordinates > &x ) : _raw( nullptr ) {
 #ifdef _DEBUG
-				std::cout << "In Vector< reference > copy-constructor\n";
+				std::cout << "In Vector< reference > copy-constructor. Copy source has ID "
+					<< x._id << "\n";
 #endif
 				initialize(
 					nullptr, nullptr, nullptr, false, nullptr,
@@ -885,6 +887,10 @@ namespace grb {
 			 * @see Vector for the user-level specfication.
 			 */
 			Vector( Vector< D, reference, MyCoordinates > &&x ) noexcept {
+#ifdef _DEBUG
+				std::cout << "Vector (reference) move-constructor called. Moving from ID "
+					<< x._id << "\n";
+#endif
 				// copy and move
 				_id = x._id;
 				_remove_id = x._remove_id;
@@ -900,19 +906,50 @@ namespace grb {
 				x._raw = nullptr;
 			}
 
-			/** Copy-constructor. */
+			/**
+			 * Copy-constructor.
+			 *
+			 * A call to this operator has the same performance semantics as a call to
+			 * #grb::set.
+			 *
+			 * \warning Relies on #grb::set. Any errors #grb::set would normally return,
+			 *          will, through this constructor, be thrown as standard C++
+			 *          exceptions instead.
+			 *
+			 * \internal Dispatches to #grb::set.
+			 */
 			Vector< D, reference, MyCoordinates > & operator=(
 				const Vector< D, reference, MyCoordinates > &x
-			) noexcept {
-				Vector< D, reference, MyCoordinates > replace( x );
-				*this = std::move( replace );
+			) {
+#ifdef _DEBUG
+				std::cout << "Vector (reference) copy-assignment called: copy " << x._id
+					<< " into " << _id << "\n";
+#endif
+				if( size( x ) != size( *this ) ) {
+					throw std::invalid_argument( "Can only copy-assign from equal-size vectors" );
+				}
+				const auto rc = set( *this, x );
+				if( rc != grb::SUCCESS ) {
+					throw std::runtime_error( grb::toString( rc ) );
+				}
 				return *this;
 			}
 
-			/** Assign-from-temporary. */
+			/**
+			 * Assign-from-temporary.
+			 *
+			 * A call to this operator has \f$ \mathcal{O}(1) \f$ performance semantics
+			 * in work and intra-process data movement. It has no costs in inter-process
+			 * data movement nor in inter-process synchronisations. No system calls shall
+			 * be made.
+			 */
 			Vector< D, reference, MyCoordinates > & operator=(
 				Vector< D, reference, MyCoordinates > &&x
 			) noexcept {
+#ifdef _DEBUG
+				std::cout << "Vector (reference) move-assignment called: move " << x._id
+					<< " into " << _id << "\n";
+#endif
 				_id = x._id;
 				_remove_id = x._remove_id;
 				_raw = x._raw;

tests/unit/CMakeLists.txt

@@ -122,6 +122,10 @@ add_grb_executables( matrixIterator matrixIterator.cpp
 	ADDITIONAL_LINK_LIBRARIES test_utils_headers
 )
 
+add_grb_executables( doubleAssign doubleAssign.cpp
+	BACKENDS reference reference_omp bsp1d hybrid hyperdags
+)
+
 add_grb_executables( matrixSet matrixSet.cpp
 	BACKENDS reference reference_omp bsp1d hybrid hyperdags
 )

tests/unit/doubleAssign.cpp

@@ -0,0 +1,102 @@
+
+/*
+ *   Copyright 2021 Huawei Technologies Co., Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <iostream>
+#include <sstream>
+#include <vector>
+
+#include <graphblas.hpp>
+
+
+using namespace grb;
+
+void grb_program( const size_t &n, grb::RC &rc ) {
+	assert( n > 0 );
+	{
+		Vector< double > a( n ), b( n );
+		rc = set( a, 1.2 );
+		rc = rc ? rc : set( b, 1.5 );
+		if( rc != grb::SUCCESS ) {
+			std::cerr << "Warning: first subtest initialision FAILED\n";
+			return;
+		}
+		a = b;
+		a = b;
+	}
+#if 0 // enable if/when operator= is defined for grb::Matrix
+	if( n > 17 ) {
+		Matrix< void > A( n, n, 1 ), B( n, n, 1 );
+		size_t anInteger = 17;
+		const size_t * const start = &anInteger;
+		rc = buildMatrixUnique( A, start, start, 1, SEQUENTIAL );
+		anInteger = 7;
+		rc = rc ? rc : buildMatrixUnique( B, start, start, 1, SEQUENTIAL );
+		A = B;
+		A = B;
+	} else {
+		std::cerr << "Warning: part of the test is disabled-- "
+			<< "please choose a larger size n\n";
+	}
+#endif
+	return;
+}
+
+int main( int argc, char ** argv ) {
+	// defaults
+	bool printUsage = false;
+	size_t in = 100;
+
+	// error checking
+	if( argc > 2 ) {
+		printUsage = true;
+	}
+	if( argc == 2 ) {
+		size_t read;
+		std::istringstream ss( argv[ 1 ] );
+		if( !(ss >> read) ) {
+			std::cerr << "Error parsing first argument\n";
+			printUsage = true;
+		} else if( !ss.eof() ) {
+			std::cerr << "Error parsing first argument\n";
+			printUsage = true;
+		} else {
+			// all OK
+			in = read;
+		}
+	}
+	if( printUsage ) {
+		std::cerr << "Usage: " << argv[ 0 ] << " [n]\n";
+		std::cerr << "  -n (optional, default is 100): an even integer, the test "
+			<< "size.\n";
+		return 1;
+	}
+
+	std::cout << "This is functional test " << argv[ 0 ] << "\n";
+	grb::Launcher< AUTOMATIC > launcher;
+	grb::RC out;
+	if( launcher.exec( &grb_program, in, out, true ) != SUCCESS ) {
+		std::cerr << "Launching test FAILED\n";
+		return 255;
+	}
+	if( out != SUCCESS ) {
+		std::cerr << "Test FAILED (" << grb::toString( out ) << ")" << std::endl;
+	} else {
+		std::cout << "Test OK" << std::endl;
+	}
+	return 0;
+}
+

tests/unit/unittests.sh

@@ -298,6 +298,13 @@ for MODE in debug ndebug; do
 				grep 'Test OK' ${TEST_OUT_DIR}/matrixIterator_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED"
 				echo " "
 
+				echo ">>>      [x]           [ ]       Testing double-assignment of ALP/GraphBLAS containers, i.e.,"
+				echo "                                 assigning one container another one (a=b), twice in a row."
+				$runner ${TEST_BIN_DIR}/doubleAssign_${MODE}_${BACKEND} 1337 &> ${TEST_OUT_DIR}/doubleAssign_${MODE}_${BACKEND}_${P}_${T}.log
+				head -1 ${TEST_OUT_DIR}/doubleAssign_${MODE}_${BACKEND}_${P}_${T}.log
+				grep -i 'test ok' ${TEST_OUT_DIR}/doubleAssign_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED"
+				echo " "
+
 				echo ">>>      [x]           [ ]       Testing copy and move constructors and assignment"
 				echo "                                 of the const_iterator of grb::Vector< double > of"
 				echo "                                 length 10 000 000."
@@ -306,7 +313,6 @@ for MODE in debug ndebug; do
 				grep 'Test OK' ${TEST_OUT_DIR}/copyAndAssignVectorIterator_${MODE}_${BACKEND}_${P}_${T}.log || echo "Test FAILED"
 				echo " "
 
-
 				echo ">>>      [x]           [ ]       Testing grb::eWiseMulAdd on a vector of"
 				echo "                                 doubles of size 7 000 000."
 				$runner ${TEST_BIN_DIR}/masked_muladd_${MODE}_${BACKEND} 7000000 &> ${TEST_OUT_DIR}/masked_muladd_large_${MODE}_${BACKEND}_${P}_${T}