From 6032b98c53f5654d936997d7112ab154c82b5778 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Tue, 15 Mar 2016 16:52:40 -0400 Subject: [PATCH] Change the interface for getWord(). * It's redundant to provide two mutable spv_position to getWord(). * getWord() should take string pointer by the style guide. --- source/text.cpp | 16 ++++++------ source/text_handler.cpp | 66 +++++++++++++++++++++---------------------------- source/text_handler.h | 4 +-- test/TextWordGet.cpp | 41 ++++++++++++++++-------------- test/UnitSPIRV.h | 2 +- 5 files changed, 61 insertions(+), 68 deletions(-) diff --git a/source/text.cpp b/source/text.cpp index c9fc8e9..ee90689 100644 --- a/source/text.cpp +++ b/source/text.cpp @@ -448,7 +448,7 @@ spv_result_t encodeInstructionStartingWithImmediate( libspirv::AssemblyContext* context, spv_instruction_t* pInst) { std::string firstWord; spv_position_t nextPosition = {}; - auto error = context->getWord(firstWord, &nextPosition); + auto error = context->getWord(&firstWord, &nextPosition); if (error) return context->diagnostic(error) << "Internal Error"; if ((error = encodeImmediate(context, firstWord.c_str(), pInst))) { @@ -461,7 +461,7 @@ spv_result_t encodeInstructionStartingWithImmediate( // Otherwise, there must be an operand that's either a literal, an ID, or // an immediate. std::string operandValue; - if ((error = context->getWord(operandValue, &nextPosition))) + if ((error = context->getWord(&operandValue, &nextPosition))) return context->diagnostic(error) << "Internal Error"; if (operandValue == "=") @@ -498,7 +498,7 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar, std::string firstWord; spv_position_t nextPosition = {}; - spv_result_t error = context->getWord(firstWord, &nextPosition); + spv_result_t error = context->getWord(&firstWord, &nextPosition); if (error) return context->diagnostic() << "Internal Error"; std::string opcodeName; @@ -521,7 +521,7 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar, if (context->advance()) return context->diagnostic() << "Expected '=', found end of stream."; std::string equal_sign; - error = context->getWord(equal_sign, &nextPosition); + error = context->getWord(&equal_sign, &nextPosition); if ("=" != equal_sign) return context->diagnostic() << "'=' expected after result id."; @@ -529,7 +529,7 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar, context->setPosition(nextPosition); if (context->advance()) return context->diagnostic() << "Expected opcode, found end of stream."; - error = context->getWord(opcodeName, &nextPosition); + error = context->getWord(&opcodeName, &nextPosition); if (error) return context->diagnostic(error) << "Internal Error"; if (!context->startsWithOp()) { return context->diagnostic() << "Invalid Opcode prefix '" << opcodeName @@ -543,8 +543,8 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar, spv_opcode_desc opcodeEntry; error = grammar.lookupOpcode(pInstName, &opcodeEntry); if (error) { - return context->diagnostic(error) << "Invalid Opcode name '" - << opcodeName << "'"; + return context->diagnostic(error) << "Invalid Opcode name '" << opcodeName + << "'"; } if (opcodeEntry->hasResult && result_id.empty()) { return context->diagnostic() @@ -611,7 +611,7 @@ spv_result_t spvTextEncodeOpcode(const libspirv::AssemblyGrammar& grammar, } std::string operandValue; - error = context->getWord(operandValue, &nextPosition); + error = context->getWord(&operandValue, &nextPosition); if (error) return context->diagnostic(error) << "Internal Error"; error = spvTextEncodeOperand(grammar, context, type, operandValue.c_str(), diff --git a/source/text_handler.cpp b/source/text_handler.cpp index 17453fe..cc4a42b 100644 --- a/source/text_handler.cpp +++ b/source/text_handler.cpp @@ -104,35 +104,28 @@ spv_result_t advance(spv_text text, spv_position position) { return SPV_SUCCESS; } -/// @brief Fetch the next word from the text stream. -/// -/// A word ends at the next comment or whitespace. However, double-quoted -/// strings remain intact, and a backslash always escapes the next character. -/// -/// @param[in] text stream to read from -/// @param[in] position current position in text stream -/// @param[out] word returned word -/// @param[out] endPosition one past the end of the returned word -/// -/// @return result code -spv_result_t getWord(spv_text text, spv_position position, std::string& word, - spv_position endPosition) { +// Fetches the next word from the given text stream starting from the given +// *position. On success, writes the decoded word into *word and updates +// *position to the location past the returned word. +// +// A word ends at the next comment or whitespace. However, double-quoted +// strings remain intact, and a backslash always escapes the next character. +spv_result_t getWord(spv_text text, spv_position position, std::string* word) { if (!text->str || !text->length) return SPV_ERROR_INVALID_TEXT; - if (!position || !endPosition) return SPV_ERROR_INVALID_POINTER; + if (!position) return SPV_ERROR_INVALID_POINTER; - *endPosition = *position; + const size_t start_index = position->index; bool quoting = false; bool escaping = false; // NOTE: Assumes first character is not white space! while (true) { - if (endPosition->index >= text->length) { - word.assign(text->str + position->index, - static_cast(endPosition->index - position->index)); + if (position->index >= text->length) { + word->assign(text->str + start_index, text->str + position->index); return SPV_SUCCESS; } - const char ch = text->str[endPosition->index]; + const char ch = text->str[position->index]; if (ch == '\\') escaping = !escaping; else { @@ -147,9 +140,7 @@ spv_result_t getWord(spv_text text, spv_position position, std::string& word, if (escaping || quoting) break; // Fall through. case '\0': { // NOTE: End of word found! - word.assign( - text->str + position->index, - static_cast(endPosition->index - position->index)); + word->assign(text->str + start_index, text->str + position->index); return SPV_SUCCESS; } default: @@ -158,8 +149,8 @@ spv_result_t getWord(spv_text text, spv_position position, std::string& word, escaping = false; } - endPosition->column++; - endPosition->index++; + position->column++; + position->index++; } } @@ -195,9 +186,10 @@ spv_result_t AssemblyContext::advance() { return ::advance(text_, ¤t_position_); } -spv_result_t AssemblyContext::getWord(std::string& word, - spv_position endPosition) { - return ::getWord(text_, ¤t_position_, word, endPosition); +spv_result_t AssemblyContext::getWord(std::string* word, + spv_position next_position) { + *next_position = current_position_; + return ::getWord(text_, next_position, word); } bool AssemblyContext::startsWithOp() { @@ -205,23 +197,21 @@ bool AssemblyContext::startsWithOp() { } bool AssemblyContext::isStartOfNewInst() { - spv_position_t nextPosition = current_position_; - if (::advance(text_, &nextPosition)) return false; - if (::startsWithOp(text_, &nextPosition)) return true; + spv_position_t pos = current_position_; + if (::advance(text_, &pos)) return false; + if (::startsWithOp(text_, &pos)) return true; std::string word; - spv_position_t startPosition = current_position_; - if (::getWord(text_, &startPosition, word, &nextPosition)) return false; + pos = current_position_; + if (::getWord(text_, &pos, &word)) return false; if ('%' != word.front()) return false; - if (::advance(text_, &nextPosition)) return false; - startPosition = nextPosition; - if (::getWord(text_, &startPosition, word, &nextPosition)) return false; + if (::advance(text_, &pos)) return false; + if (::getWord(text_, &pos, &word)) return false; if ("=" != word) return false; - if (::advance(text_, &nextPosition)) return false; - startPosition = nextPosition; - if (::startsWithOp(text_, &startPosition)) return true; + if (::advance(text_, &pos)) return false; + if (::startsWithOp(text_, &pos)) return true; return false; } diff --git a/source/text_handler.h b/source/text_handler.h index c0a3df4..03ef7b3 100644 --- a/source/text_handler.h +++ b/source/text_handler.h @@ -145,9 +145,9 @@ class AssemblyContext { // Returns SPV_SUCCESS on success. spv_result_t advance(); - // Sets word to the next word in the input text. Fills endPosition with + // Sets word to the next word in the input text. Fills next_position with // the next location past the end of the word. - spv_result_t getWord(std::string& word, spv_position endPosition); + spv_result_t getWord(std::string* word, spv_position next_position); // Returns true if the next word in the input is the start of a new Opcode. bool startsWithOp(); diff --git a/test/TextWordGet.cpp b/test/TextWordGet.cpp index 56338ca..7abfee5 100644 --- a/test/TextWordGet.cpp +++ b/test/TextWordGet.cpp @@ -39,8 +39,9 @@ using spvtest::AutoText; TEST(TextWordGet, NullTerminator) { std::string word; spv_position_t endPosition = {}; - ASSERT_EQ(SPV_SUCCESS, AssemblyContext(AutoText("Word"), nullptr) - .getWord(word, &endPosition)); + ASSERT_EQ( + SPV_SUCCESS, + AssemblyContext(AutoText("Word"), nullptr).getWord(&word, &endPosition)); ASSERT_EQ(4u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(4u, endPosition.index); @@ -51,7 +52,7 @@ TEST(TextWordGet, TabTerminator) { std::string word; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, AssemblyContext(AutoText("Word\t"), nullptr) - .getWord(word, &endPosition)); + .getWord(&word, &endPosition)); ASSERT_EQ(4u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(4u, endPosition.index); @@ -61,8 +62,9 @@ TEST(TextWordGet, TabTerminator) { TEST(TextWordGet, SpaceTerminator) { std::string word; spv_position_t endPosition = {}; - ASSERT_EQ(SPV_SUCCESS, AssemblyContext(AutoText("Word "), nullptr) - .getWord(word, &endPosition)); + ASSERT_EQ( + SPV_SUCCESS, + AssemblyContext(AutoText("Word "), nullptr).getWord(&word, &endPosition)); ASSERT_EQ(4u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(4u, endPosition.index); @@ -72,8 +74,9 @@ TEST(TextWordGet, SpaceTerminator) { TEST(TextWordGet, SemicolonTerminator) { std::string word; spv_position_t endPosition = {}; - ASSERT_EQ(SPV_SUCCESS, AssemblyContext(AutoText("Wo;rd"), nullptr) - .getWord(word, &endPosition)); + ASSERT_EQ( + SPV_SUCCESS, + AssemblyContext(AutoText("Wo;rd"), nullptr).getWord(&word, &endPosition)); ASSERT_EQ(2u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(2u, endPosition.index); @@ -87,7 +90,7 @@ TEST(TextWordGet, NoTerminator) { spv_text_t text = {full_text.data(), len}; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, - AssemblyContext(&text, nullptr).getWord(word, &endPosition)); + AssemblyContext(&text, nullptr).getWord(&word, &endPosition)); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(len, endPosition.column); ASSERT_EQ(len, endPosition.index); @@ -104,7 +107,7 @@ TEST(TextWordGet, MultipleWords) { std::string word; for (uint32_t wordIndex = 0; wordIndex < 4; ++wordIndex) { - ASSERT_EQ(SPV_SUCCESS, data.getWord(word, &endPosition)); + ASSERT_EQ(SPV_SUCCESS, data.getWord(&word, &endPosition)); ASSERT_EQ(strlen(words[wordIndex]), endPosition.column - data.position().column); ASSERT_EQ(0u, endPosition.line); @@ -128,7 +131,7 @@ TEST(TextWordGet, QuotesAreKept) { std::string word; spv_position_t endPosition = {}; - ASSERT_EQ(SPV_SUCCESS, data.getWord(word, &endPosition)); + ASSERT_EQ(SPV_SUCCESS, data.getWord(&word, &endPosition)); EXPECT_EQ(8u, endPosition.column); EXPECT_EQ(0u, endPosition.line); EXPECT_EQ(8u, endPosition.index); @@ -138,7 +141,7 @@ TEST(TextWordGet, QuotesAreKept) { data.setPosition(endPosition); data.seekForward(1); - ASSERT_EQ(SPV_SUCCESS, data.getWord(word, &endPosition)); + ASSERT_EQ(SPV_SUCCESS, data.getWord(&word, &endPosition)); EXPECT_EQ(23u, endPosition.column); EXPECT_EQ(0u, endPosition.line); EXPECT_EQ(23u, endPosition.index); @@ -152,7 +155,7 @@ TEST(TextWordGet, QuotesBetweenWordsActLikeGlue) { std::string word; spv_position_t endPosition = {}; - ASSERT_EQ(SPV_SUCCESS, data.getWord(word, &endPosition)); + ASSERT_EQ(SPV_SUCCESS, data.getWord(&word, &endPosition)); EXPECT_EQ(16u, endPosition.column); EXPECT_EQ(0u, endPosition.line); EXPECT_EQ(16u, endPosition.index); @@ -162,7 +165,7 @@ TEST(TextWordGet, QuotesBetweenWordsActLikeGlue) { data.setPosition(endPosition); data.seekForward(1); - ASSERT_EQ(SPV_SUCCESS, data.getWord(word, &endPosition)); + ASSERT_EQ(SPV_SUCCESS, data.getWord(&word, &endPosition)); EXPECT_EQ(22u, endPosition.column); EXPECT_EQ(0u, endPosition.line); EXPECT_EQ(22u, endPosition.index); @@ -175,7 +178,7 @@ TEST(TextWordGet, QuotingWhitespace) { std::string word; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, - AssemblyContext(input, nullptr).getWord(word, &endPosition)); + AssemblyContext(input, nullptr).getWord(&word, &endPosition)); EXPECT_EQ(input.str.length(), endPosition.column); EXPECT_EQ(0u, endPosition.line); EXPECT_EQ(input.str.length(), endPosition.index); @@ -187,7 +190,7 @@ TEST(TextWordGet, QuoteAlone) { std::string word; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, - AssemblyContext(input, nullptr).getWord(word, &endPosition)); + AssemblyContext(input, nullptr).getWord(&word, &endPosition)); ASSERT_EQ(1u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(1u, endPosition.index); @@ -199,7 +202,7 @@ TEST(TextWordGet, EscapeAlone) { std::string word; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, - AssemblyContext(input, nullptr).getWord(word, &endPosition)); + AssemblyContext(input, nullptr).getWord(&word, &endPosition)); ASSERT_EQ(1u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(1u, endPosition.index); @@ -211,7 +214,7 @@ TEST(TextWordGet, EscapeAtEndOfInput) { std::string word; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, - AssemblyContext(input, nullptr).getWord(word, &endPosition)); + AssemblyContext(input, nullptr).getWord(&word, &endPosition)); ASSERT_EQ(5u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(5u, endPosition.index); @@ -223,7 +226,7 @@ TEST(TextWordGet, Escaping) { std::string word; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, - AssemblyContext(input, nullptr).getWord(word, &endPosition)); + AssemblyContext(input, nullptr).getWord(&word, &endPosition)); ASSERT_EQ(10u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(10u, endPosition.index); @@ -235,7 +238,7 @@ TEST(TextWordGet, EscapingEscape) { std::string word; spv_position_t endPosition = {}; ASSERT_EQ(SPV_SUCCESS, - AssemblyContext(input, nullptr).getWord(word, &endPosition)); + AssemblyContext(input, nullptr).getWord(&word, &endPosition)); ASSERT_EQ(6u, endPosition.column); ASSERT_EQ(0u, endPosition.line); ASSERT_EQ(6u, endPosition.index); diff --git a/test/UnitSPIRV.h b/test/UnitSPIRV.h index 2e902b0..49eb2e2 100644 --- a/test/UnitSPIRV.h +++ b/test/UnitSPIRV.h @@ -169,7 +169,7 @@ inline std::vector MakeVector(std::string input) { // A type for easily creating spv_text_t values, with an implicit conversion to // spv_text. struct AutoText { - explicit AutoText(std::string value) + explicit AutoText(const std::string& value) : str(value), text({str.data(), str.size()}) {} operator spv_text() { return &text; } std::string str; -- 2.7.4