Skip to content

Commit

Permalink
Fix shadowed variable in velox/common/file/FileInputStream.cpp (faceb…
Browse files Browse the repository at this point in the history
…ookincubator#11275)

Summary:
Pull Request resolved: facebookincubator#11275

Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so.

This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug.

**What's a shadowed variable?**

Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs.

This diff fixes such an issue by renaming the variable.

 - If you approve of this diff, please use the "Accept & Ship" button :-)

Reviewed By: meyering

Differential Revision: D64398704

fbshipit-source-id: f99d0fe237393213d710fa4a101407597714fc81
  • Loading branch information
r-barnes authored and facebook-github-bot committed Oct 16, 2024
1 parent 5a5bf8e commit 4f7e570
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion velox/common/file/FileInputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ void FileInputStream::readNextRange() {
readBytes = readSize();
VELOX_CHECK_LT(
0, readBytes, "Read past end of FileInputStream {}", fileSize_);
NanosecondTimer timer{&readTimeNs};
NanosecondTimer timer_2{&readTimeNs};
file_->pread(fileOffset_, readBytes, buffer()->asMutable<char>());
}
}
Expand Down
4 changes: 2 additions & 2 deletions velox/common/memory/HashStringAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,8 @@ HashStringAllocator::finishWrite(
// and the block was extended. Calculate the new position.
if (state_.startPosition().header->isContinued()) {
auto* header = state_.startPosition().header;
const auto offset = state_.startPosition().offset();
const auto extra = offset - header->usableSize();
const auto offset_2 = state_.startPosition().offset();
const auto extra = offset_2 - header->usableSize();
if (extra > 0) {
auto* newHeader = header->nextContinued();
auto* newPosition = newHeader->begin() + extra;
Expand Down
14 changes: 7 additions & 7 deletions velox/exec/SetAccumulator.h
Original file line number Diff line number Diff line change
Expand Up @@ -276,14 +276,14 @@ struct ComplexTypeSetAccumulator {
void addValues(
const ArrayVector& arrayVector,
vector_size_t index,
const DecodedVector& values,
const DecodedVector& values_2,
HashStringAllocator* allocator) {
VELOX_DCHECK(!arrayVector.isNullAt(index));
const auto size = arrayVector.sizeAt(index);
const auto offset = arrayVector.offsetAt(index);

for (auto i = 0; i < size; ++i) {
addValue(values, offset + i, allocator);
addValue(values_2, offset + i, allocator);
}
}

Expand All @@ -304,29 +304,29 @@ struct ComplexTypeSetAccumulator {
void addNonNullValues(
const ArrayVector& arrayVector,
vector_size_t index,
const DecodedVector& values,
const DecodedVector& values_2,
HashStringAllocator* allocator) {
VELOX_DCHECK(!arrayVector.isNullAt(index));
const auto size = arrayVector.sizeAt(index);
const auto offset = arrayVector.offsetAt(index);

for (auto i = 0; i < size; ++i) {
addNonNullValue(values, offset + i, allocator);
addNonNullValue(values_2, offset + i, allocator);
}
}

size_t size() const {
return base.size();
}

vector_size_t extractValues(BaseVector& values, vector_size_t offset) {
vector_size_t extractValues(BaseVector& values_2, vector_size_t offset) {
for (const auto& position : base.uniqueValues) {
AddressableNonNullValueList::read(
position.first, values, offset + position.second);
position.first, values_2, offset + position.second);
}

if (base.nullIndex.has_value()) {
values.setNull(offset + base.nullIndex.value(), true);
values_2.setNull(offset + base.nullIndex.value(), true);
}

return base.uniqueValues.size() + (base.nullIndex.has_value() ? 1 : 0);
Expand Down
4 changes: 2 additions & 2 deletions velox/functions/FunctionRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ std::optional<bool> isDeterministic(const std::string& functionName) {
return std::nullopt;
}

for (const auto& [metadata, _] : simpleFunctions) {
if (!metadata.deterministic) {
for (const auto& [metadata_2, _] : simpleFunctions) {
if (!metadata_2.deterministic) {
return false;
}
}
Expand Down

0 comments on commit 4f7e570

Please sign in to comment.