Merge pull request #9091 from ethereum/phaserConversionWarnings

Fixing Phaser signedness conversion warnings

test/yulPhaser/Mutations.cpp

@@ -118,7 +118,7 @@ BOOST_AUTO_TEST_CASE(geneAddition_should_be_able_to_insert_before_first_position
 	BOOST_TEST(mutatedChromosome.length() > chromosome.length());
 
 	vector<string> suffix(
-		mutatedChromosome.optimisationSteps().end() - chromosome.length(),
+		mutatedChromosome.optimisationSteps().end() - static_cast<ptrdiff_t>(chromosome.length()),
 		mutatedChromosome.optimisationSteps().end()
 	);
 	BOOST_TEST(suffix == chromosome.optimisationSteps());
@@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(geneAddition_should_be_able_to_insert_after_last_position)
 
 	vector<string> prefix(
 		mutatedChromosome.optimisationSteps().begin(),
-		mutatedChromosome.optimisationSteps().begin() + chromosome.length()
+		mutatedChromosome.optimisationSteps().begin() + static_cast<ptrdiff_t>(chromosome.length())
 	);
 	BOOST_TEST(prefix == chromosome.optimisationSteps());
 }
@@ -179,8 +179,8 @@ BOOST_AUTO_TEST_CASE(alternativeMutations_should_choose_between_mutations_with_g
 	for (size_t i = 0; i < 10; ++i)
 	{
 		Chromosome mutatedChromosome = mutation(chromosome);
-		cCount += static_cast<int>(mutatedChromosome == Chromosome("c"));
-		fCount += static_cast<int>(mutatedChromosome == Chromosome("f"));
+		cCount += (mutatedChromosome == Chromosome("c") ? 1 : 0);
+		fCount += (mutatedChromosome == Chromosome("f") ? 1 : 0);
 	}
 
 	// This particular seed results in 7 "c"s out of 10 which looks plausible given the 80% chance.

test/yulPhaser/Population.cpp

@@ -135,7 +135,7 @@ BOOST_FIXTURE_TEST_CASE(makeRandom_should_get_chromosome_lengths_from_specified_
 	size_t maxLength = 5;
 	assert(chromosomeCount % maxLength == 0);
 
-	auto nextLength = [counter = 0, maxLength]() mutable { return counter++ % maxLength; };
+	auto nextLength = [counter = 0ul, maxLength]() mutable { return counter++ % maxLength; };
 	auto population = Population::makeRandom(m_fitnessMetric, chromosomeCount, nextLength);
 
 	// We can't rely on the order since the population sorts its chromosomes immediately but

test/yulPhaser/TestHelpers.cpp

@@ -72,9 +72,13 @@ size_t phaser::test::countDifferences(Chromosome const& _chromosome1, Chromosome
 {
 	size_t count = 0;
 	for (size_t i = 0; i < min(_chromosome1.length(), _chromosome2.length()); ++i)
-		count += static_cast<int>(_chromosome1.optimisationSteps()[i] != _chromosome2.optimisationSteps()[i]);
+		if (_chromosome1.optimisationSteps()[i] != _chromosome2.optimisationSteps()[i])
+			++count;
 
-	return count + abs(static_cast<int>(_chromosome1.length() - _chromosome2.length()));
+	return count + static_cast<size_t>(abs(
+		static_cast<long>(_chromosome1.length()) -
+		static_cast<long>(_chromosome2.length())
+	));
 }
 
 TemporaryDirectory::TemporaryDirectory(std::string const& _prefix):

tools/yulPhaser/GeneticAlgorithms.cpp

@@ -155,7 +155,7 @@ Population ClassicGeneticAlgorithm::select(Population _population, size_t _selec
 	vector<Individual> selectedIndividuals;
 	for (size_t i = 0; i < _selectionSize; ++i)
 	{
-		uint32_t ball = SimulationRNG::uniformInt(0, rouletteRange - 1);
+		size_t ball = SimulationRNG::uniformInt(0, rouletteRange - 1);
 
 		size_t cumulativeFitness = 0;
 		for (auto const& individual: _population.individuals())

tools/yulPhaser/Mutations.cpp

@@ -126,12 +126,12 @@ ChromosomePair fixedPointSwap(
 
 	return {
 		Chromosome(
-			vector<string>(begin1, begin1 + _crossoverPoint) +
-			vector<string>(begin2 + _crossoverPoint, end2)
+			vector<string>(begin1, begin1 + static_cast<ptrdiff_t>(_crossoverPoint)) +
+			vector<string>(begin2 + static_cast<ptrdiff_t>(_crossoverPoint), end2)
 		),
 		Chromosome(
-			vector<string>(begin2, begin2 + _crossoverPoint) +
-			vector<string>(begin1 + _crossoverPoint, end1)
+			vector<string>(begin2, begin2 + static_cast<ptrdiff_t>(_crossoverPoint)) +
+			vector<string>(begin1 + static_cast<ptrdiff_t>(_crossoverPoint), end1)
 		),
 	};
 }
@@ -196,8 +196,8 @@ ChromosomePair fixedTwoPointSwap(
 	assert(_crossoverPoint2 <= _chromosome1.length());
 	assert(_crossoverPoint2 <= _chromosome2.length());
 
-	size_t lowPoint = min(_crossoverPoint1, _crossoverPoint2);
-	size_t highPoint = max(_crossoverPoint1, _crossoverPoint2);
+	auto lowPoint = static_cast<ptrdiff_t>(min(_crossoverPoint1, _crossoverPoint2));
+	auto highPoint = static_cast<ptrdiff_t>(max(_crossoverPoint1, _crossoverPoint2));
 
 	auto begin1 = _chromosome1.optimisationSteps().begin();
 	auto begin2 = _chromosome2.optimisationSteps().begin();
@@ -282,17 +282,17 @@ ChromosomePair uniformSwap(Chromosome const& _chromosome1, Chromosome const& _ch
 	if (_chromosome1.length() > minLength)
 	{
 		if (swapTail)
-			steps2.insert(steps2.end(), begin1 + minLength, end1);
+			steps2.insert(steps2.end(), begin1 + static_cast<ptrdiff_t>(minLength), end1);
 		else
-			steps1.insert(steps1.end(), begin1 + minLength, end1);
+			steps1.insert(steps1.end(), begin1 + static_cast<ptrdiff_t>(minLength), end1);
 	}
 
 	if (_chromosome2.length() > minLength)
 	{
 		if (swapTail)
-			steps1.insert(steps1.end(), begin2 + minLength, end2);
+			steps1.insert(steps1.end(), begin2 + static_cast<ptrdiff_t>(minLength), end2);
 		else
-			steps2.insert(steps2.end(), begin2 + minLength, end2);
+			steps2.insert(steps2.end(), begin2 + static_cast<ptrdiff_t>(minLength), end2);
 	}
 
 	return {Chromosome(steps1), Chromosome(steps2)};

tools/yulPhaser/PairSelections.cpp

@@ -30,7 +30,7 @@ vector<tuple<size_t, size_t>> RandomPairSelection::materialise(size_t _poolSize)
 	if (_poolSize < 2)
 		return {};
 
-	size_t count = static_cast<size_t>(round(_poolSize * m_selectionSize));
+	auto count = static_cast<size_t>(round(_poolSize * m_selectionSize));
 
 	vector<tuple<size_t, size_t>> selection;
 	for (size_t i = 0; i < count; ++i)
@@ -64,7 +64,10 @@ vector<tuple<size_t, size_t>> PairsFromRandomSubset::materialise(size_t _poolSiz
 			} while (selectedIndices.size() % 2 != 0);
 		}
 		else
-			selectedIndices.erase(selectedIndices.begin() + SimulationRNG::uniformInt(0, selectedIndices.size() - 1));
+			selectedIndices.erase(
+				selectedIndices.begin() +
+				static_cast<ptrdiff_t>(SimulationRNG::uniformInt(0, selectedIndices.size() - 1))
+			);
 	}
 	assert(selectedIndices.size() % 2 == 0);
 
@@ -73,14 +76,14 @@ vector<tuple<size_t, size_t>> PairsFromRandomSubset::materialise(size_t _poolSiz
 	{
 		size_t position1 = SimulationRNG::uniformInt(0, selectedIndices.size() - 1);
 		size_t value1 = selectedIndices[position1];
-		selectedIndices.erase(selectedIndices.begin() + position1);
+		selectedIndices.erase(selectedIndices.begin() + static_cast<ptrdiff_t>(position1));
 		size_t position2 = SimulationRNG::uniformInt(0, selectedIndices.size() - 1);
 		size_t value2 = selectedIndices[position2];
-		selectedIndices.erase(selectedIndices.begin() + position2);
+		selectedIndices.erase(selectedIndices.begin() + static_cast<ptrdiff_t>(position2));
 
-		selectedPairs.push_back({value1, value2});
+		selectedPairs.emplace_back(value1, value2);
 	}
-	assert(selectedIndices.size() == 0);
+	assert(selectedIndices.empty());
 
 	return selectedPairs;
 }

tools/yulPhaser/SimulationRNG.cpp

@@ -17,12 +17,17 @@
 
 #include <tools/yulPhaser/SimulationRNG.h>
 
+// NOTE: The code would work with std::random but the results for a given seed would not be reproducible
+// across different STL implementations. Boost does not guarantee this either but at least it has only one
+// implementation. Reproducibility is not a hard requirement for yul-phaser but it's nice to have.
 #include <boost/random/bernoulli_distribution.hpp>
 #include <boost/random/binomial_distribution.hpp>
 #include <boost/random/uniform_int_distribution.hpp>
 
 #include <ctime>
+#include <limits>
 
+using namespace std;
 using namespace solidity;
 using namespace solidity::phaser;
 
@@ -30,23 +35,27 @@ thread_local boost::random::mt19937 SimulationRNG::s_generator(SimulationRNG::ge
 
 bool SimulationRNG::bernoulliTrial(double _successProbability)
 {
-	boost::random::bernoulli_distribution<> distribution(_successProbability);
+	boost::random::bernoulli_distribution<double> distribution(_successProbability);
 
-	return static_cast<bool>(distribution(s_generator));
-}
-
-uint32_t SimulationRNG::uniformInt(uint32_t _min, uint32_t _max)
-{
-	boost::random::uniform_int_distribution<> distribution(_min, _max);
 	return distribution(s_generator);
 }
 
-uint32_t SimulationRNG::binomialInt(uint32_t _numTrials, double _successProbability)
+size_t SimulationRNG::uniformInt(size_t _min, size_t _max)
 {
-	boost::random::binomial_distribution<> distribution(_numTrials, _successProbability);
+	boost::random::uniform_int_distribution<size_t> distribution(_min, _max);
 	return distribution(s_generator);
 }
 
+size_t SimulationRNG::binomialInt(size_t _numTrials, double _successProbability)
+{
+	// NOTE: binomial_distribution<size_t> would not work because it internally tries to use abs()
+	// and fails to compile due to ambiguous conversion.
+	assert(_numTrials <= static_cast<size_t>(numeric_limits<long>::max()));
+
+	boost::random::binomial_distribution<long> distribution(static_cast<long>(_numTrials), _successProbability);
+	return static_cast<size_t>(distribution(s_generator));
+}
+
 uint32_t SimulationRNG::generateSeed()
 {
 	// This is not a secure way to seed the generator but it's good enough for simulation purposes.

tools/yulPhaser/SimulationRNG.h

@@ -38,8 +38,8 @@ class SimulationRNG
 {
 public:
 	static bool bernoulliTrial(double _successProbability);
-	static uint32_t uniformInt(uint32_t _min, uint32_t _max);
-	static uint32_t binomialInt(uint32_t _numTrials, double _successProbability);
+	static size_t uniformInt(size_t _min, size_t _max);
+	static size_t binomialInt(size_t _numTrials, double _successProbability);
 
 	/// Resets generator to a known state given by the @a seed. Given the same seed, a fixed
 	/// sequence of calls to the members generating random values is guaranteed to produce the