Skip to content

Commit

Permalink
refactor : Change C-style casts to C++-style (Part 4)
Browse files Browse the repository at this point in the history
As per the security guideline in
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es49-if-you-must-use-a-cast-use-a-named-cast

Covers the findings in velox/functions
  • Loading branch information
aditi-pandit committed Dec 4, 2024
1 parent 28c319e commit ed180ee
Show file tree
Hide file tree
Showing 18 changed files with 77 additions and 45 deletions.
7 changes: 4 additions & 3 deletions velox/functions/CoverageUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ void printCoverageMap(
// Split scalar functions into 'numScalarColumns' columns. Put all aggregate
// functions into one column.
auto numRows = std::max(
{(size_t)std::ceil((double)scalarCnt / numScalarColumns),
{static_cast<size_t>(
std::ceil(static_cast<double>(scalarCnt) / numScalarColumns)),
aggCnt,
windowCnt});

Expand Down Expand Up @@ -426,8 +427,8 @@ void printVeloxFunctions(
auto scalarCnt = scalarNames.size();
auto aggCnt = aggNames.size();
auto windowCnt = windowNames.size();
auto numRows =
std::max({(size_t)std::ceil(scalarCnt / 3.0), aggCnt, windowCnt});
auto numRows = std::max(
{static_cast<size_t>(std::ceil(scalarCnt / 3.0)), aggCnt, windowCnt});

auto printName = [&](const std::string& name) {
return linkBlockList.count(name) == 0 ? toFuncLink(name, domain) : name;
Expand Down
14 changes: 9 additions & 5 deletions velox/functions/prestosql/Sequence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,24 @@ T add(T value, K step, int32_t sequence);

template <>
int64_t add(int64_t value, int64_t step, int32_t sequence) {
const auto delta = (int128_t)step * (int128_t)sequence;
const auto delta =
static_cast<int128_t>(step) * static_cast<int128_t>(sequence);
// Since step is calcuated from start and stop,
// the sum of 'value' and 'add' is within int64_t.
return value + delta;
}

template <>
int32_t add(int32_t value, int64_t step, int32_t sequence) {
const auto delta = (int128_t)step * (int128_t)sequence;
const auto delta =
static_cast<int128_t>(step) * static_cast<int128_t>(sequence);
return value + delta;
}

template <>
Timestamp add(Timestamp value, int64_t step, int32_t sequence) {
const auto delta = (int128_t)step * (int128_t)sequence;
const auto delta =
static_cast<int128_t>(step) * static_cast<int128_t>(sequence);
return Timestamp::fromMillis(value.toMillis() + delta);
}

Expand Down Expand Up @@ -181,8 +184,9 @@ class SequenceFunction : public exec::VectorFunction {
if (isYearMonth) {
sequenceCount = getStepCount(start, stop, step);
} else {
sequenceCount =
((int128_t)toInt64(stop) - (int128_t)toInt64(start)) / step +
sequenceCount = (static_cast<int128_t>(toInt64(stop)) -
static_cast<int128_t>(toInt64(start))) /
step +
1; // prevent overflow
}
VELOX_USER_CHECK_LE(
Expand Down
2 changes: 1 addition & 1 deletion velox/functions/prestosql/WidthBucketArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class WidthBucketArrayFunctionConstantBins : public exec::VectorFunction {
VELOX_USER_CHECK(!std::isnan(operand), "Operand cannot be NaN");

int lower = 0;
int upper = (int)bins.size();
int upper = static_cast<int>(bins.size());
while (lower < upper) {
int index = (lower + upper) / 2;
auto bin = bins.at(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,17 @@ struct ApproxMostFrequentAggregate : exec::Aggregate {
auto rowVec = (*result)->as<RowVector>();
VELOX_CHECK(rowVec);
rowVec->childAt(0) = std::make_shared<ConstantVector<int64_t>>(
rowVec->pool(), numGroups, false, BIGINT(), int64_t(buckets_));
rowVec->pool(),
numGroups,
false,
BIGINT(),
static_cast<int64_t>(buckets_));
rowVec->childAt(1) = std::make_shared<ConstantVector<int64_t>>(
rowVec->pool(), numGroups, false, BIGINT(), int64_t(capacity_));
rowVec->pool(),
numGroups,
false,
BIGINT(),
static_cast<int64_t>(capacity_));
auto values = rowVec->childAt(2)->as<ArrayVector>();
auto counts = rowVec->childAt(3)->as<ArrayVector>();
rowVec->resize(numGroups);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,13 +324,17 @@ class ApproxPercentileAggregate : public exec::Aggregate {
BaseVector::wrapInConstant(numGroups, 0, std::move(array));
rowResult->childAt(kPercentilesIsArray) =
std::make_shared<ConstantVector<bool>>(
pool, numGroups, false, BOOLEAN(), bool(percentiles_->isArray));
pool,
numGroups,
false,
BOOLEAN(),
static_cast<bool>(percentiles_->isArray));
rowResult->childAt(kAccuracy) = std::make_shared<ConstantVector<double>>(
pool,
numGroups,
accuracy_ == kMissingNormalizedValue,
DOUBLE(),
double(accuracy_));
static_cast<double>(accuracy_));
auto k = rowResult->childAt(kK)->asFlatVector<int32_t>();
auto n = rowResult->childAt(kN)->asFlatVector<int64_t>();
auto minValue = rowResult->childAt(kMinValue)->asFlatVector<T>();
Expand Down
2 changes: 1 addition & 1 deletion velox/functions/prestosql/aggregates/CountAggregate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class CountAggregate : public SimpleNumericAggregate<bool, int64_t, int64_t> {
folly::Range<const vector_size_t*> indices) override {
for (auto i : indices) {
// result of count is never null
*value<int64_t>(groups[i]) = (int64_t)0;
*value<int64_t>(groups[i]) = static_cast<int64_t>(0);
}
}

Expand Down
9 changes: 5 additions & 4 deletions velox/functions/prestosql/aggregates/CovarianceAggregates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,10 @@ struct CovarAccumulator {
double deltaMeanX = meanXOther - meanX();
double deltaMeanY = meanYOther - meanY();
c2_ += c2Other +
deltaMeanX * deltaMeanY * count() * countOther / (double)newCount;
meanX_ += deltaMeanX * countOther / (double)newCount;
meanY_ += deltaMeanY * countOther / (double)newCount;
deltaMeanX * deltaMeanY * count() * countOther /
static_cast<double>(newCount);
meanX_ += deltaMeanX * countOther / static_cast<double>(newCount);
meanY_ += deltaMeanY * countOther / static_cast<double>(newCount);
count_ = newCount;
}

Expand Down Expand Up @@ -674,7 +675,7 @@ class CovarianceAggregate : public exec::Aggregate {
auto* accumulator = this->accumulator(group);
if (TResultAccessor::hasResult(*accumulator)) {
clearNull(rawNulls, i);
rawValues[i] = (T)TResultAccessor::result(*accumulator);
rawValues[i] = static_cast<T>(TResultAccessor::result(*accumulator));
} else {
vector->setNull(i, true);
}
Expand Down
7 changes: 4 additions & 3 deletions velox/functions/prestosql/aggregates/EntropyAggregates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ struct EntropyAccumulator {
if (count == 0) {
return;
}
sumC_ += (double)count;
sumCLogC_ += (double)count * std::log(count);
sumC_ += static_cast<double>(count);
sumCLogC_ += static_cast<double>(count * std::log(count));
}

void merge(double sumCOther, double sumCLogCOther) {
Expand Down Expand Up @@ -125,7 +125,8 @@ class EntropyAggregate : public exec::Aggregate {
// The "sum" is the constant value times the number of rows.
// Use double to prevent overflows (this is the same as what is done in
// updateNonNullValue).
const auto sum = (double)numRows * (double)value;
const auto sum =
static_cast<double>(numRows) * static_cast<double>(value);
EntropyAccumulator accData(sum, sum * std::log(value));
updateNonNullValue(group, accData);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class MaxSizeForStatsAggregate
result = value;
},
mayPushdown,
(int64_t)0);
static_cast<int64_t>(0));
}

void addSingleGroupRawInput(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class SumDataSizeForStatsAggregate
result = checkedPlus(result, value);
},
mayPushdown,
(int64_t)0);
static_cast<int64_t>(0));
}

void addSingleGroupRawInput(
Expand Down
7 changes: 4 additions & 3 deletions velox/functions/prestosql/aggregates/VarianceAggregates.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ struct VarianceAccumulator {
int64_t newCount = countOther + count();
double delta = meanOther - mean();
double newMean = mean() + delta / newCount * countOther;
m2_ += m2Other + delta * delta * countOther * count() / (double)newCount;
m2_ += m2Other +
delta * delta * countOther * count() / static_cast<double>(newCount);
count_ = newCount;
mean_ = newMean;
}
Expand Down Expand Up @@ -218,7 +219,7 @@ class VarianceAggregate : public exec::Aggregate {
if (!decodedRaw_.isNullAt(0)) {
const T value = decodedRaw_.valueAt<T>(0);
const auto numRows = rows.countSelected();
VarianceAccumulator accData(numRows, (double)value);
VarianceAccumulator accData(numRows, static_cast<double>(value));
updateNonNullValue(group, accData);
}
} else if (decodedRaw_.mayHaveNulls()) {
Expand Down Expand Up @@ -384,7 +385,7 @@ class VarianceAggregate : public exec::Aggregate {
exec::Aggregate::clearNull(group);
}
VarianceAccumulator* thisAccData = accumulator(group);
thisAccData->update((double)value);
thisAccData->update(static_cast<double>(value));
}

template <bool tableHasNulls = true>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class ArrayWriterBenchmark : public functions::test::FunctionBenchmarkBase {
}

vector_size_t size = 1'000;
size_t totalItemsCount = (size) * (size + 1) / 2;
size_t totalItemsCount = static_cast<size_t>((size) * (size + 1) / 2);

auto makeInput() {
std::vector<int64_t> inputData(size, 0);
Expand Down
2 changes: 1 addition & 1 deletion velox/functions/prestosql/window/CumeDist.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class CumeDistFunction : public exec::WindowFunction {
if (peerStart != currentPeerGroupStart_) {
currentPeerGroupStart_ = peerStart;
runningTotal_ += peerGroupEndsVector[i] - peerStart + 1;
cumeDist_ = double(runningTotal_) / numPartitionRows_;
cumeDist_ = static_cast<double>(runningTotal_) / numPartitionRows_;
}
rawValues[resultOffset + i] = cumeDist_;
}
Expand Down
11 changes: 6 additions & 5 deletions velox/functions/sparksql/DecimalArithmetic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,9 @@ struct DecimalAddSubtractBase {
// Adjust fractional parts to higher scale.
const auto higherScale = std::max(aScale, bScale);
const auto aFractionScaled =
increaseScale((int128_t)aFraction, higherScale - aScale);
increaseScale(static_cast<int128_t>(aFraction), higherScale - aScale);
const auto bFractionScaled =
increaseScale((int128_t)bFraction, higherScale - bScale);
increaseScale(static_cast<int128_t>(bFraction), higherScale - bScale);

int128_t fraction;
bool carryToLeft = false;
Expand Down Expand Up @@ -205,12 +205,13 @@ struct DecimalAddSubtractBase {
// Adjust fractional parts to higher scale.
const auto higherScale = std::max(aScale, bScale);
const auto aFractionScaled =
increaseScale((int128_t)aFraction, higherScale - aScale);
increaseScale(static_cast<int128_t>(aFraction), higherScale - aScale);
const auto bFractionScaled =
increaseScale((int128_t)bFraction, higherScale - bScale);
increaseScale(static_cast<int128_t>(bFraction), higherScale - bScale);

// No need to consider overflow because two inputs are opposite.
int128_t whole = (int128_t)aWhole + (int128_t)bWhole;
int128_t whole =
static_cast<int128_t>(aWhole) + static_cast<int128_t>(bWhole);
int128_t fraction = aFractionScaled + bFractionScaled;

// If the whole and fractional parts have different signs, adjust them to
Expand Down
3 changes: 2 additions & 1 deletion velox/functions/sparksql/MakeTimestamp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ std::optional<Timestamp> makeTimeStampFromDecodedArgs(

// Micros has at most 8 digits (2 for seconds + 6 for microseconds),
// thus it's safe to cast micros from int64_t to int32_t.
auto localMicros = util::fromTime(hour, minute, 0, (int32_t)micros);
auto localMicros =
util::fromTime(hour, minute, 0, static_cast<int32_t>(micros));
return util::fromDatetime(daysSinceEpoch.value(), localMicros);
}

Expand Down
2 changes: 1 addition & 1 deletion velox/functions/sparksql/aggregates/AverageAggregate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class AverageAggregate
vector->setNull(i, true);
} else {
this->clearNull(rawNulls, i);
rawValues[i] = (TResult)sumCount->sum / sumCount->count;
rawValues[i] = static_cast<TResult>(sumCount->sum) / sumCount->count;
}
}
}
Expand Down
23 changes: 16 additions & 7 deletions velox/functions/sparksql/specialforms/DecimalRound.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ class DecimalRoundFunction : public exec::VectorFunction {
uint8_t resultScale)
: scale_(
scale >= 0
? std::min(scale, (int32_t)LongDecimalType::kMaxPrecision)
: std::max(scale, -(int32_t)LongDecimalType::kMaxPrecision)),
? std::min(
scale,
static_cast<int32_t>(LongDecimalType::kMaxPrecision))
: std::max(
scale,
-static_cast<int32_t>(LongDecimalType::kMaxPrecision))),
inputPrecision_(inputPrecision),
inputScale_(inputScale),
resultPrecision_(resultPrecision),
Expand All @@ -54,7 +58,7 @@ class DecimalRoundFunction : public exec::VectorFunction {
VELOX_USER_CHECK_GT(
rescale, 0, "A non-negative rescale value is expected.");
return DecimalUtil::kPowersOfTen[std::min(
rescale, (int32_t)LongDecimalType::kMaxPrecision)];
rescale, static_cast<int32_t>(LongDecimalType::kMaxPrecision))];
};
if (scale_ < 0) {
divideFactor_ = rescaleFactor(inputScale_ - scale_);
Expand Down Expand Up @@ -179,18 +183,23 @@ DecimalRoundCallToSpecialForm::getResultPrecisionScale(
// rounding, and the result scale is 0.
const auto newPrecision = std::max(
integralLeastNumDigits,
-std::max(roundScale, -(int32_t)LongDecimalType::kMaxPrecision) + 1);
-std::max(
roundScale, -static_cast<int32_t>(LongDecimalType::kMaxPrecision)) +
1);
// We have to accept the risk of overflow as we can't exceed the max
// precision.
return {std::min(newPrecision, (int32_t)LongDecimalType::kMaxPrecision), 0};
return {
std::min(
newPrecision, static_cast<int32_t>(LongDecimalType::kMaxPrecision)),
0};
}
const uint8_t newScale = std::min((int32_t)scale, roundScale);
const uint8_t newScale = std::min(static_cast<int32_t>(scale), roundScale);
// We have to accept the risk of overflow as we cannot exceed the max
// precision.
return {
std::min(
integralLeastNumDigits + newScale,
(int32_t)LongDecimalType::kMaxPrecision),
static_cast<int32_t>(LongDecimalType::kMaxPrecision)),
newScale};
}

Expand Down
7 changes: 4 additions & 3 deletions velox/functions/sparksql/specialforms/MakeDecimal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ class MakeDecimalFunction final : public exec::VectorFunction {
[&](vector_size_t row) { rawResults[row] = constant; });
}
} else {
rows.applyToSelected(
[&](vector_size_t row) { rawResults[row] = (int128_t)constant; });
rows.applyToSelected([&](vector_size_t row) {
rawResults[row] = static_cast<int128_t>(constant);
});
}
}

Expand Down Expand Up @@ -105,7 +106,7 @@ class MakeDecimalFunction final : public exec::VectorFunction {
});
} else {
rows.applyToSelected([&](vector_size_t row) {
rawResults[row] = (int128_t)rawValues[row];
rawResults[row] = static_cast<int128_t>(rawValues[row]);
});
}
}
Expand Down

0 comments on commit ed180ee

Please sign in to comment.