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.

diff --git a/source/text.cpp b/source/text.cpp
index c9fc8e9..ee90689 100644 (file)
--- 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 (file)
--- 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<size_t>(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<size_t>(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_, &current_position_);
 }
 
-spv_result_t AssemblyContext::getWord(std::string& word,
-                                      spv_position endPosition) {
-  return ::getWord(text_, &current_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 (file)
--- 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 (file)
--- 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 (file)
--- a/test/UnitSPIRV.h
+++ b/test/UnitSPIRV.h
@@ -169,7 +169,7 @@ inline std::vector<uint32_t> 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;