diff --git a/export/planloader/planloader.cpp b/export/planloader/planloader.cpp index 549fbcc9..024e10a6 100644 --- a/export/planloader/planloader.cpp +++ b/export/planloader/planloader.cpp @@ -24,7 +24,7 @@ SerializedPlan* load_substrait_plan(const char* filename) { strncpy(newPlan->error_message, errMsg.data(), errMsg.length() + 1); return newPlan; } - ::substrait::proto::Plan plan = *planOrError; + const ::substrait::proto::Plan& plan = *planOrError; std::string text = plan.SerializeAsString(); newPlan->buffer = new unsigned char[text.length() + 1]; memcpy(newPlan->buffer, text.data(), text.length() + 1); diff --git a/src/substrait/common/PlanTransformerTool.cpp b/src/substrait/common/PlanTransformerTool.cpp index 8717916d..550c2136 100644 --- a/src/substrait/common/PlanTransformerTool.cpp +++ b/src/substrait/common/PlanTransformerTool.cpp @@ -38,7 +38,7 @@ int main(int argc, char* argv[]) { auto planOrError = io::substrait::loadPlan(argv[1]); if (!planOrError.ok()) { - std::cerr << planOrError.status() << std::endl; + std::cerr << planOrError.status() << '\n'; return EXIT_FAILURE; } @@ -46,7 +46,7 @@ int main(int argc, char* argv[]) { auto result = io::substrait::savePlan(*planOrError, argv[2], format); if (!result.ok()) { - std::cerr << result << std::endl; + std::cerr << result << '\n'; return EXIT_FAILURE; } diff --git a/src/substrait/expression/DecimalLiteral.cpp b/src/substrait/expression/DecimalLiteral.cpp index 52d2fe06..0abfd9b2 100644 --- a/src/substrait/expression/DecimalLiteral.cpp +++ b/src/substrait/expression/DecimalLiteral.cpp @@ -56,7 +56,10 @@ DecimalLiteral DecimalLiteral::fromString( } std::uint8_t valueBytes[16]; uint128ToBytes(v, valueBytes); - return {std::string((const char*)valueBytes, 16), precision, scale}; + return { + std::string(reinterpret_cast(valueBytes), 16), + precision, + scale}; } bool DecimalLiteral::isValid() { diff --git a/src/substrait/textplan/SymbolTable.cpp b/src/substrait/textplan/SymbolTable.cpp index 66d25579..f0d8a8d1 100644 --- a/src/substrait/textplan/SymbolTable.cpp +++ b/src/substrait/textplan/SymbolTable.cpp @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: Apache-2.0 */ #include "substrait/textplan/SymbolTable.h" +#include #include #include #include @@ -27,7 +28,7 @@ const std::string& symbolTypeName(SymbolType type) { "kSourceDetail", "kField", }; - auto typeNum = int32_t(type); + auto typeNum = static_cast(type); if (typeNum == -1) { static std::string unknown = "kUnknown"; return unknown; @@ -136,9 +137,7 @@ void SymbolTable::setParentQueryLocation( int highestIndex = -1; for (const auto& sym : symbols_) { if (sym->parentQueryLocation == location) { - if (sym->parentQueryIndex > highestIndex) { - highestIndex = sym->parentQueryIndex; - } + highestIndex = std::max(sym->parentQueryIndex, highestIndex); } } symbols_[index]->parentQueryIndex = highestIndex + 1; @@ -236,7 +235,7 @@ std::string SymbolTable::toDebugString() const { if (!relationData->subQueryPipelines.empty()) { result << " SQC=" << relationData->subQueryPipelines.size(); } - result << std::endl; + result << '\n'; int32_t fieldNum = 0; for (const auto& field : relationData->fieldReferences) { @@ -248,7 +247,7 @@ std::string SymbolTable::toDebugString() const { if (!field->alias.empty()) { result << " " << field->alias; } - result << std::endl; + result << '\n'; } for (const auto& field : relationData->generatedFieldReferences) { @@ -266,7 +265,7 @@ std::string SymbolTable::toDebugString() const { } else if (!field->alias.empty()) { result << " " << field->alias; } - result << std::endl; + result << '\n'; } int32_t outputFieldNum = 0; @@ -279,12 +278,12 @@ std::string SymbolTable::toDebugString() const { if (!field->alias.empty()) { result << " " << field->alias; } - result << std::endl; + result << '\n'; } textAlreadyWritten = true; } if (textAlreadyWritten) { - result << std::endl; + result << '\n'; } return result.str(); } diff --git a/src/substrait/textplan/converter/Tool.cpp b/src/substrait/textplan/converter/Tool.cpp index 49dc43d3..63ad4a51 100644 --- a/src/substrait/textplan/converter/Tool.cpp +++ b/src/substrait/textplan/converter/Tool.cpp @@ -17,7 +17,7 @@ namespace { void convertPlanToText(const char* filename) { auto planOrError = loadPlan(filename); if (!planOrError.ok()) { - std::cerr << planOrError.status() << std::endl; + std::cerr << planOrError.status() << '\n'; return; } @@ -29,7 +29,7 @@ void convertPlanToText(const char* filename) { auto errors = result.getAllErrors(); if (!errors.empty()) { for (const auto& err : errors) { - std::cerr << err << std::endl; + std::cerr << err << '\n'; } } std::cout << textResult; diff --git a/src/substrait/textplan/converter/tests/BinaryToTextPlanConversionTest.cpp b/src/substrait/textplan/converter/tests/BinaryToTextPlanConversionTest.cpp index 935143e8..4443c858 100644 --- a/src/substrait/textplan/converter/tests/BinaryToTextPlanConversionTest.cpp +++ b/src/substrait/textplan/converter/tests/BinaryToTextPlanConversionTest.cpp @@ -685,7 +685,7 @@ TEST_F(BinaryToTextPlanConversionTest, FullSample) { ASSERT_TRUE(jsonOrError.ok()); auto planOrError = loadFromJson(*jsonOrError); ASSERT_TRUE(planOrError.ok()); - auto plan = *planOrError; + const auto& plan = *planOrError; EXPECT_THAT(plan.extensions_size(), ::testing::Eq(7)); auto expectedOutputOrError = readFromFile("data/q6_first_stage.golden.splan"); diff --git a/src/substrait/textplan/parser/ParseText.cpp b/src/substrait/textplan/parser/ParseText.cpp index 7367125c..7c946b49 100644 --- a/src/substrait/textplan/parser/ParseText.cpp +++ b/src/substrait/textplan/parser/ParseText.cpp @@ -23,11 +23,11 @@ std::optional loadTextFile( std::string_view filename) { std::ifstream stream(std::string{filename}); if (stream.bad() || stream.fail()) { - std::cout << "Bad stream." << std::endl; + std::cout << "Bad stream." << '\n'; return std::nullopt; } if (!stream.is_open()) { - std::cout << "Stream is not open." << std::endl; + std::cout << "Stream is not open." << '\n'; return std::nullopt; } return {stream}; diff --git a/src/substrait/textplan/parser/Tool.cpp b/src/substrait/textplan/parser/Tool.cpp index 3c31bca6..136849c1 100644 --- a/src/substrait/textplan/parser/Tool.cpp +++ b/src/substrait/textplan/parser/Tool.cpp @@ -12,13 +12,13 @@ namespace { void readText(const char* filename) { auto stream = io::substrait::textplan::loadTextFile(filename); if (!stream.has_value()) { - std::cerr << "An error occurred while reading: " << filename << std::endl; + std::cerr << "An error occurred while reading: " << filename << '\n'; return; } auto parseResult = io::substrait::textplan::parseStream(&*stream); if (!parseResult.successful()) { for (const std::string& msg : parseResult.getAllErrors()) { - std::cout << msg << std::endl; + std::cout << msg << '\n'; } return; } @@ -28,7 +28,7 @@ void readText(const char* filename) { parseResult.getSymbolTable(), &errorListener); if (errorListener.hasErrors()) { for (const std::string& msg : errorListener.getErrorMessages()) { - std::cout << msg << std::endl; + std::cout << msg << '\n'; } return; } diff --git a/src/substrait/textplan/tests/ParseResultMatchers.cpp b/src/substrait/textplan/tests/ParseResultMatchers.cpp index 7fe12bfd..10477d91 100644 --- a/src/substrait/textplan/tests/ParseResultMatchers.cpp +++ b/src/substrait/textplan/tests/ParseResultMatchers.cpp @@ -56,7 +56,7 @@ bool stringEqSquashingWhitespace( << have.substr(atHave - have.begin(), 20) << "|" << " should be |" << expected.substr(atExpected - expected.begin(), 20) << "|" - << std::endl; + << '\n'; } return false; } @@ -164,7 +164,7 @@ class HasSymbolsMatcher { expectedSymbolsSorted.end(), std::back_inserter(extraSymbols)); if (!extraSymbols.empty()) { - *listener << std::endl << " with extra symbols: "; + *listener << '\n' << " with extra symbols: "; for (const auto& symbol : extraSymbols) { *listener << " \"" << symbol << "\""; } @@ -314,7 +314,7 @@ class HasSymbolsWithTypesMatcher { std::vector extraSymbols = orderedSetDifference(actualSymbols, expectedSymbols_); if (!extraSymbols.empty()) { - *listener << std::endl << " with extra symbols"; + *listener << '\n' << " with extra symbols"; DescribeTypes(listener); *listener << ": "; for (const auto& symbol : extraSymbols) { diff --git a/src/substrait/textplan/tests/RoundtripTest.cpp b/src/substrait/textplan/tests/RoundtripTest.cpp index 08ea9d97..75a73a60 100644 --- a/src/substrait/textplan/tests/RoundtripTest.cpp +++ b/src/substrait/textplan/tests/RoundtripTest.cpp @@ -38,7 +38,7 @@ std::string addLineNumbers(const std::string& text) { int lineNum = 0; std::string line; while (std::getline(input, line, '\n')) { - result << std::setw(4) << ++lineNum << " " << line << std::endl; + result << std::setw(4) << ++lineNum << " " << line << '\n'; } return result.str(); } @@ -72,13 +72,13 @@ std::vector getTestCases() { GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(RoundTripBinaryToTextFixture); TEST_P(RoundTripBinaryToTextFixture, RoundTrip) { - auto filename = GetParam(); + const auto& filename = GetParam(); auto jsonOrError = readFromFile(filename); ASSERT_TRUE(jsonOrError.ok()); auto planOrError = loadFromJson(*jsonOrError); ASSERT_TRUE(planOrError.ok()); - auto plan = *planOrError; + const auto& plan = *planOrError; auto textResult = parseBinaryPlan(plan); auto textSymbols = textResult.getSymbolTable().getSymbols(); @@ -89,10 +89,10 @@ TEST_P(RoundTripBinaryToTextFixture, RoundTrip) { textResult.addErrors(errorListener.getErrorMessages()); ASSERT_THAT(textResult, ParsesOk()) - << std::endl - << "Initial result:" << std::endl - << addLineNumbers(outputText) << std::endl - << textResult.getSymbolTable().toDebugString() << std::endl; + << '\n' + << "Initial result:" << '\n' + << addLineNumbers(outputText) << '\n' + << textResult.getSymbolTable().toDebugString() << '\n'; auto stream = loadTextString(outputText); auto result = parseStream(&stream); @@ -108,10 +108,10 @@ TEST_P(RoundTripBinaryToTextFixture, RoundTrip) { AsBinaryPlan(IgnoringFields( {"substrait.proto.RelCommon.Emit.output_mapping"}, EqualsProto(normalizedPlan))))) - << std::endl - << "Intermediate result:" << std::endl - << addLineNumbers(outputText) << std::endl - << result.getSymbolTable().toDebugString() << std::endl; + << '\n' + << "Intermediate result:" << '\n' + << addLineNumbers(outputText) << '\n' + << result.getSymbolTable().toDebugString() << '\n'; } INSTANTIATE_TEST_SUITE_P( diff --git a/src/substrait/textplan/tests/SymbolTableTest.cpp b/src/substrait/textplan/tests/SymbolTableTest.cpp index 042419ad..14eab260 100644 --- a/src/substrait/textplan/tests/SymbolTableTest.cpp +++ b/src/substrait/textplan/tests/SymbolTableTest.cpp @@ -102,7 +102,7 @@ TEST_F(SymbolTableTest, LocationsUnchangedAfterCopy) { auto* ptr3 = &plan.extensions(0); const SymbolTable& table2 = table; - auto symbols = table2.getSymbols(); + const auto& symbols = table2.getSymbols(); ASSERT_THAT( symbolNames(symbols), ::testing::ElementsAre("symbol1", "symbol2", "symbol3"));