[2372] even further simplification of option handling

src/lib/dns/master_lexer.cc

@@ -53,7 +53,6 @@ struct MasterLexer::MasterLexerImpl {
     std::vector<InputSourcePtr> sources_;
     InputSource* source_;       // current source
     size_t paren_count_;
-    Options options_;
     bool last_was_eol_;
     Token token_;
 };
@@ -160,13 +159,15 @@ State::getParenCount(const MasterLexer& lexer) const {
 class Start : public State {
 public:
     Start() {}
-    virtual const State* handle(MasterLexer& lexer) const;
+    virtual const State* handle(MasterLexer& lexer,
+                                MasterLexer::Options options) const;
 };
 
 class CRLF : public State {
 public:
     CRLF() {}
-    virtual const State* handle(MasterLexer& lexer) const {
+    virtual const State* handle(MasterLexer& lexer, MasterLexer::Options) const
+    {
         // We've just seen '\r'.  If this is part of a sequence of '\r\n',
         // we combine them as a single END-OF-LINE.  Otherwise we treat the
         // single '\r' as an EOL and continue tokeniziation from the character
@@ -191,7 +192,9 @@ public:
 class String : public State {
 public:
     String() {}
-    virtual const State* handle(MasterLexer& /*lexer*/) const {
+    virtual const State* handle(MasterLexer& /*lexer*/,
+                                MasterLexer::Options /*options*/) const
+    {
         return (NULL);
     }
 };
@@ -215,9 +218,8 @@ State::getInstance(ID state_id) {
 }
 
 const State*
-Start::handle(MasterLexer& lexer) const {
-    MasterLexer::Options options = getLexerImpl(lexer)->options_;
-    size_t& paren_count = getLexerImpl(lexer)->paren_count_;
+Start::handle(MasterLexer& lexer, MasterLexer::Options options) const {
+    size_t& paren_count = getLexerImpl(lexer)->paren_count_; // shortcut
 
     while (true) {
         const int c = getLexerImpl(lexer)->skipComment(
@@ -260,9 +262,7 @@ Start::handle(MasterLexer& lexer) const {
                 getLexerImpl(lexer)->token_ = Token(Token::UNBALANCED_PAREN);
                 return (NULL);
             }
-            if (--paren_count == 0) {
-                options = getLexerImpl(lexer)->options_;
-            }
+            --paren_count;
             continue;
         } else {
             // Note: in #2373 we should probably ungetChar().
@@ -272,13 +272,6 @@ Start::handle(MasterLexer& lexer) const {
     }
 }
 
-const State*
-State::getStartInstance(MasterLexer& lexer, MasterLexer::Options options)
-{
-    lexer.impl_->options_ = options;
-    return (&START_STATE);
-}
-
 } // namespace master_lexer_internal
 
 } // end of namespace dns

src/lib/dns/master_lexer_state.h

@@ -25,11 +25,8 @@ class InputSource;
 
 class State {
 public:
-    virtual const State* handle(MasterLexer& lexer) const = 0;
-
-    // Note: Pass options mainly for the convenience of tests.
-    static const State* getStartInstance(MasterLexer& lexer,
-                                         MasterLexer::Options options);
+    virtual const State* handle(MasterLexer& lexer,
+                                MasterLexer::Options options) const = 0;
 
     /// Specific states are basically hidden within the implementation,
     /// but we'd like to allow tests to examine them, so we provide

src/lib/dns/tests/master_lexer_state_unittest.cc

@@ -30,8 +30,7 @@ class MasterLexerStateTest : public ::testing::Test {
 protected:
     MasterLexerStateTest() : common_options(MasterLexer::INITIAL_WS),
                              s_null(NULL),
-                             s_start(*State::getStartInstance(
-                                         lexer, common_options)),
+                             s_start(State::getInstance(State::Start)),
                              s_crlf(State::getInstance(State::CRLF)),
                              s_string(State::getInstance(State::String)),
                              options(MasterLexer::NONE),
@@ -65,18 +64,18 @@ eofCheck(const State& state, MasterLexer& lexer) {
 TEST_F(MasterLexerStateTest, startAndEnd) {
     // A simple case: the input is empty, so we begin with start and
     // are immediately done.
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     eofCheck(s_start, lexer);
 }
 
 TEST_F(MasterLexerStateTest, startToEOL) {
     ss << "\n";
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_TRUE(s_start.wasLastEOL(lexer));
     EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
 
     // The next lexer session will reach EOF.  Same eof check should pass.
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     eofCheck(s_start, lexer);
 }
 
@@ -85,20 +84,17 @@ TEST_F(MasterLexerStateTest, space) {
     // twice; at the second iteration, it's a white space at the beginning
     // of line, but since we don't specify INITIAL_WS option, it's treated as
     // normal space and ignored.
-    State::getStartInstance(lexer, MasterLexer::NONE);
     for (size_t i = 0; i < 2; ++i) {
         ss << " \t\n";
-        EXPECT_EQ(s_null, s_start.handle(lexer));
+        EXPECT_EQ(s_null, s_start.handle(lexer, MasterLexer::NONE));
         EXPECT_TRUE(s_start.wasLastEOL(lexer));
         EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
     }
 
     // Now we specify the INITIAL_WS option.  It will be recognized and the
     // corresponding token will be returned.
-    State::getStartInstance(lexer, common_options);
     ss << " ";
-    options = MasterLexer::INITIAL_WS;
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, MasterLexer::INITIAL_WS));
     EXPECT_FALSE(s_start.wasLastEOL(lexer));
     EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType());
 }
@@ -106,55 +102,55 @@ TEST_F(MasterLexerStateTest, space) {
 TEST_F(MasterLexerStateTest, parentheses) {
     ss << "\n(\na\n )\n "; // 1st \n is to check if 'was EOL' is set to false
 
-    EXPECT_EQ(s_null, s_start.handle(lexer)); // handle \n
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // handle \n
 
     // Now handle '('.  It skips \n and recognize 'a' as string
     EXPECT_EQ(0, s_start.getParenCount(lexer)); // check pre condition
-    EXPECT_EQ(&s_string, s_start.handle(lexer));
+    EXPECT_EQ(&s_string, s_start.handle(lexer, common_options));
     EXPECT_EQ(1, s_start.getParenCount(lexer)); // check post condition
     EXPECT_FALSE(s_start.wasLastEOL(lexer));
 
     // skip 'a' (note: until #2373 it's actually skipped as part of the '('
     // handling)
-    s_string.handle(lexer);
+    s_string.handle(lexer, common_options);
 
     // Then handle ')'.  '\n' before ')' isn't recognized because
     // it's canceled due to the '('.  Likewise, the space after the '\n'
     // shouldn't be recognized but should be just ignored.
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_EQ(0, s_start.getParenCount(lexer));
 
     // Now, temporarily disabled options are restored: Both EOL and the
     // initial WS are recognized
     EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType());
 }
 
 TEST_F(MasterLexerStateTest, nestedParentheses) {
     // This is an unusual, but allowed (in this implementation) case.
     ss << "(a(b)\n c)\n ";
-    EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '('
-    s_string.handle(lexer);                      // consume 'a'
-    EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '('
-    s_string.handle(lexer);                     // consume 'b'
+    EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume '('
+    s_string.handle(lexer, common_options);                      // consume 'a'
+    EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume '('
+    s_string.handle(lexer, common_options);                     // consume 'b'
     EXPECT_EQ(2, s_start.getParenCount(lexer)); // now the count is 2
 
     // Close the inner most parentheses.  count will be decreased, but option
     // shouldn't be restored yet, so the intermediate EOL or initial WS won't
     // be recognized.
-    EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume ')'
-    s_string.handle(lexer);                      // consume 'c'
+    EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume ')'
+    s_string.handle(lexer, common_options);                      // consume 'c'
     EXPECT_EQ(1, s_start.getParenCount(lexer));
 
     // Close the outermost parentheses.  count will be reset to 0, and original
     // options are restored.
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
 
     // Now, temporarily disabled options are restored: Both EOL and the
     // initial WS are recognized
     EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType());
 }
 
@@ -163,13 +159,13 @@ TEST_F(MasterLexerStateTest, unbalancedParentheses) {
     // correctly canceled after detecting the error.
     ss << "\n)";
 
-    EXPECT_EQ(s_null, s_start.handle(lexer)); // consume '\n'
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // consume '\n'
     EXPECT_TRUE(s_start.wasLastEOL(lexer)); // this \n was remembered
 
     // Now checking ')'.  The result should be error, count shouldn't be
     // changed.  "last EOL" should be canceled.
     EXPECT_EQ(0, s_start.getParenCount(lexer));
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_EQ(0, s_start.getParenCount(lexer));
     ASSERT_EQ(Token::ERROR, s_start.getToken(lexer).getType());
     EXPECT_EQ(Token::UNBALANCED_PAREN, s_start.getToken(lexer).getErrorCode());
@@ -177,10 +173,10 @@ TEST_F(MasterLexerStateTest, unbalancedParentheses) {
 
     // Reach EOF with a dangling open parenthesis.
     ss << "(a";
-    EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '('
-    s_string.handle(lexer);                      // consume 'a'
+    EXPECT_EQ(&s_string, s_start.handle(lexer, common_options)); // consume '('
+    s_string.handle(lexer, common_options);                      // consume 'a'
     EXPECT_EQ(1, s_start.getParenCount(lexer));
-    EXPECT_EQ(s_null, s_start.handle(lexer));    // reach EOF
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));    // reach EOF
     ASSERT_EQ(Token::ERROR, s_start.getToken(lexer).getType());
     EXPECT_EQ(Token::UNBALANCED_PAREN, s_start.getToken(lexer).getErrorCode());
     EXPECT_EQ(0, s_start.getParenCount(lexer)); // should be reset to 0
@@ -191,12 +187,12 @@ TEST_F(MasterLexerStateTest, startToComment) {
     // the rest of the line, and recognize the new line.  Note that the
     // second ';' is simply ignored.
     ss << "  ;a;\n";
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
 
     // Likewise, but the comment ends with EOF.
     ss << ";a;";
-    EXPECT_EQ(s_null, s_start.handle(lexer));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_EQ(Token::END_OF_FILE, s_start.getToken(lexer).getType());
 }
 
@@ -205,39 +201,42 @@ TEST_F(MasterLexerStateTest, commentAfterParen) {
     // other tests should also ensure that it works correctly, but we
     // check it explicitly.
     ss << "( ;this is a comment\na)\n";
-    EXPECT_EQ(&s_string, s_start.handle(lexer)); // consume '(', skip comments
-    s_string.handle(lexer);                      // consume 'a'
-    EXPECT_EQ(s_null, s_start.handle(lexer));    // consume ')', see '\n'
+    // consume '(', skip comments, consume 'a', then consume ')'
+    EXPECT_EQ(&s_string, s_start.handle(lexer, common_options));
+    s_string.handle(lexer, common_options);
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));
     EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
 }
 
 TEST_F(MasterLexerStateTest, crlf) {
     // A sequence of \r, \n is recognized as a single 'end-of-line'
     ss << "\r\n";
-    EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));   // recognize '\n'
+    EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r'
+    EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options));   // recognize '\n'
     EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
     EXPECT_TRUE(s_start.wasLastEOL(lexer));
 
     // Single '\r' (not followed by \n) is recognized as a single 'end-of-line'
     ss << "\r ";                // then there will be "initial WS"
-    State::getStartInstance(lexer, common_options); // specify usual options
-    EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));   // recognize ' ', "unget" it
-    EXPECT_EQ(s_null, s_start.handle(lexer)); // re-recognize ' '
+    EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r'
+    // see ' ', "unget" it
+    EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options)); // recognize ' '
     EXPECT_EQ(Token::INITIAL_WS, s_start.getToken(lexer).getType());
 
     ss << "\r;comment\na";
-    EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));   // skip comments, recognize '\n'
+    EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r'
+    // skip comments, recognize '\n'
+    EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options));
     EXPECT_EQ(Token::END_OF_LINE, s_start.getToken(lexer).getType());
-    EXPECT_EQ(&s_string, s_start.handle(lexer));
+    EXPECT_EQ(&s_string, s_start.handle(lexer, common_options));
 
     // \r then EOF
     ss << "\r";
-    EXPECT_EQ(&s_crlf, s_start.handle(lexer)); // recognize '\r'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));   // see EOF, then "unget" it
-    EXPECT_EQ(s_null, s_start.handle(lexer));  // re-recognize EOF
+    EXPECT_EQ(&s_crlf, s_start.handle(lexer, common_options)); // recognize '\r'
+    // see EOF, then "unget" it
+    EXPECT_EQ(s_null, s_crlf.handle(lexer, common_options));
+    EXPECT_EQ(s_null, s_start.handle(lexer, common_options));  // recognize EOF
     EXPECT_EQ(Token::END_OF_FILE, s_start.getToken(lexer).getType());
 }