Moved error-handling to a separate class - Diagnostics. We were earlier returning errors as tokens which did not work very well when error occured while parsing a preprocessor directive. Now all returned tokens are valid. Errors are reported via an abstract Diagnostics interface. Updated unit-tests with the new scheme.

Review URL: https://codereview.appspot.com/6203089

git-svn-id: https://angleproject.googlecode.com/svn/trunk@1087 736b8ea6-26fd-11df-bfd4-992fa37f6226

10 files changed, 170 insertions, 109 deletions

diff --git a/tests/build_tests.gyp b/tests/build_tests.gyp
index 646a38a4..7828eb36 100644
--- a/tests/build_tests.gyp
+++ b/tests/build_tests.gyp
@@ -47,6 +47,7 @@
         'preprocessor_tests/identifier_test.cpp',
         'preprocessor_tests/input_test.cpp',
         'preprocessor_tests/location_test.cpp',
+        'preprocessor_tests/MockDiagnostics.h',
         'preprocessor_tests/number_test.cpp',
         'preprocessor_tests/operator_test.cpp',
         'preprocessor_tests/token_test.cpp',
diff --git a/tests/preprocessor_tests/MockDiagnostics.h b/tests/preprocessor_tests/MockDiagnostics.h
new file mode 100644
index 00000000..45980031
--- /dev/null
+++ b/tests/preprocessor_tests/MockDiagnostics.h
@@ -0,0 +1,20 @@
+//
+// Copyright (c) 2012 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+
+#ifndef PREPROCESSOR_TESTS_MOCK_DIAGNOSTICS_H_
+#define PREPROCESSOR_TESTS_MOCK_DIAGNOSTICS_H_
+
+#include "gmock/gmock.h"
+#include "Diagnostics.h"
+
+class MockDiagnostics : public pp::Diagnostics
+{
+  public:
+    MOCK_METHOD3(print,
+        void(ID id, const pp::SourceLocation& loc, const std::string& text));
+};
+
+#endif  // PREPROCESSOR_TESTS_MOCK_DIAGNOSTICS_H_
diff --git a/tests/preprocessor_tests/char_test.cpp b/tests/preprocessor_tests/char_test.cpp
index b54cbf76..1123e0f7 100644
--- a/tests/preprocessor_tests/char_test.cpp
+++ b/tests/preprocessor_tests/char_test.cpp
@@ -8,6 +8,8 @@
 #include <climits>
 
 #include "gtest/gtest.h"
+
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
@@ -23,81 +25,75 @@ static const char kPunctuators[] = {
 static const int kNumPunctuators =
     sizeof(kPunctuators) / sizeof(kPunctuators[0]);
 
+bool isPunctuator(char c)
+{
+    static const char* kPunctuatorBeg = kPunctuators;
+    static const char* kPunctuatorEnd = kPunctuators + kNumPunctuators;
+    return std::find(kPunctuatorBeg, kPunctuatorEnd, c) != kPunctuatorEnd;
+}
+
 static const char kWhitespaces[] = {' ', '\t', '\v', '\f', '\n', '\r'};
 static const int kNumWhitespaces =
     sizeof(kWhitespaces) / sizeof(kWhitespaces[0]);
 
+bool isWhitespace(char c)
+{
+    static const char* kWhitespaceBeg = kWhitespaces;
+    static const char* kWhitespaceEnd = kWhitespaces + kNumWhitespaces;
+    return std::find(kWhitespaceBeg, kWhitespaceEnd, c) != kWhitespaceEnd;
+}
+
 TEST_P(CharTest, Identified)
 {
     std::string str(1, GetParam());
     const char* cstr = str.c_str();
     int length = 1;
 
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     // Note that we pass the length param as well because the invalid
     // string may contain the null character.
     ASSERT_TRUE(preprocessor.init(1, &cstr, &length));
 
-    pp::Token token;
-    int ret = preprocessor.lex(&token);
+    int expectedType = pp::Token::LAST;
+    std::string expectedValue;
 
-    // Handle identifier.
-    if ((cstr[0] == '_') ||
-        ((cstr[0] >= 'a') && (cstr[0] <= 'z')) ||
-        ((cstr[0] >= 'A') && (cstr[0] <= 'Z')))
+    if (str[0] == '#')
     {
-        EXPECT_EQ(pp::Token::IDENTIFIER, ret);
-        EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
-        EXPECT_EQ(cstr[0], token.value[0]);
-        return;
+        // Lone '#' is ignored.
     }
-
-    // Handle numbers.
-    if (cstr[0] >= '0' && cstr[0] <= '9')
+    else if ((str[0] == '_') ||
+             ((str[0] >= 'a') && (str[0] <= 'z')) ||
+             ((str[0] >= 'A') && (str[0] <= 'Z')))
     {
-        EXPECT_EQ(pp::Token::CONST_INT, ret);
-        EXPECT_EQ(pp::Token::CONST_INT, token.type);
-        EXPECT_EQ(cstr[0], token.value[0]);
-        return;
+        expectedType = pp::Token::IDENTIFIER;
+        expectedValue = str;
     }
-
-    // Handle punctuators.
-    const char* lastIter = kPunctuators + kNumPunctuators;
-    const char* iter = std::find(kPunctuators, lastIter, cstr[0]);
-    if (iter != lastIter)
+    else if (str[0] >= '0' && str[0] <= '9')
     {
-        EXPECT_EQ(cstr[0], ret);
-        EXPECT_EQ(cstr[0], token.type);
-        EXPECT_TRUE(token.value.empty());
-        return;
+        expectedType = pp::Token::CONST_INT;
+        expectedValue = str;
     }
-
-    // Handle whitespace.
-    lastIter = kWhitespaces + kNumWhitespaces;
-    iter = std::find(kWhitespaces, lastIter, cstr[0]);
-    if (iter != lastIter)
+    else if (isPunctuator(str[0]))
+    {
+        expectedType = str[0];
+    }
+    else if (isWhitespace(str[0]))
     {
         // Whitespace is ignored.
-        EXPECT_EQ(pp::Token::LAST, ret);
-        EXPECT_EQ(pp::Token::LAST, token.type);
-        EXPECT_TRUE(token.value.empty());
-        return;
     }
-
-    // Handle number sign.
-    if (cstr[0] == '#')
+    else
     {
-        // Lone '#' is ignored.
-        EXPECT_EQ(pp::Token::LAST, ret);
-        EXPECT_EQ(pp::Token::LAST, token.type);
-        EXPECT_TRUE(token.value.empty());
-        return;
+        // Everything else is invalid.
+        using testing::_;
+        EXPECT_CALL(diagnostics,
+            print(pp::Diagnostics::INVALID_CHARACTER, _, str));
     }
 
-    // Everything else is invalid.
-    EXPECT_EQ(pp::Token::INVALID_CHARACTER, ret);
-    EXPECT_EQ(pp::Token::INVALID_CHARACTER, token.type);
-    EXPECT_EQ(cstr[0], token.value[0]);
+    pp::Token token;
+    preprocessor.lex(&token);
+    EXPECT_EQ(expectedType, token.type);
+    EXPECT_EQ(expectedValue, token.value);
 };
 
 // Note +1 for the max-value in range. It is there because the max-value
diff --git a/tests/preprocessor_tests/comment_test.cpp b/tests/preprocessor_tests/comment_test.cpp
index 6b03afc0..c8b2bcfa 100644
--- a/tests/preprocessor_tests/comment_test.cpp
+++ b/tests/preprocessor_tests/comment_test.cpp
@@ -5,6 +5,8 @@
 //
 
 #include "gtest/gtest.h"
+
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
@@ -16,10 +18,12 @@ TEST_P(CommentTest, CommentIgnored)
 {
     const char* str = GetParam();
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
-    EXPECT_EQ(pp::Token::LAST, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::LAST, token.type);
 }
 
@@ -42,10 +46,12 @@ TEST(BlockComment, CommentReplacedWithSpace)
 {
     const char* str = "/*foo*/bar";
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
-    EXPECT_EQ(pp::Token::IDENTIFIER, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
     EXPECT_STREQ("bar", token.value.c_str());
     EXPECT_TRUE(token.hasLeadingSpace());
@@ -55,9 +61,13 @@ TEST(BlockComment, UnterminatedComment)
 {
     const char* str = "/*foo";
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
-    EXPECT_EQ(pp::Token::EOF_IN_COMMENT, preprocessor.lex(&token));
-    EXPECT_EQ(pp::Token::EOF_IN_COMMENT, token.type);
+
+    using testing::_;
+    EXPECT_CALL(diagnostics, print(pp::Diagnostics::EOF_IN_COMMENT, _, _));
+
+    pp::Token token;
+    preprocessor.lex(&token);
 }
diff --git a/tests/preprocessor_tests/identifier_test.cpp b/tests/preprocessor_tests/identifier_test.cpp
index 107cd919..0c30288c 100644
--- a/tests/preprocessor_tests/identifier_test.cpp
+++ b/tests/preprocessor_tests/identifier_test.cpp
@@ -5,15 +5,19 @@
 //
 
 #include "gtest/gtest.h"
+
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
 static void PreprocessAndVerifyIdentifier(const char* str)
 {
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
-    EXPECT_EQ(pp::Token::IDENTIFIER, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
     EXPECT_STREQ(str, token.value.c_str());
 }
diff --git a/tests/preprocessor_tests/input_test.cpp b/tests/preprocessor_tests/input_test.cpp
index 17774aee..8297232e 100644
--- a/tests/preprocessor_tests/input_test.cpp
+++ b/tests/preprocessor_tests/input_test.cpp
@@ -5,27 +5,33 @@
 //
 
 #include "gtest/gtest.h"
+
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
 TEST(InputTest, NegativeCount)
 {
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     EXPECT_FALSE(preprocessor.init(-1, NULL, NULL));
 }
 
 TEST(InputTest, ZeroCount)
 {
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     EXPECT_TRUE(preprocessor.init(0, NULL, NULL));
-    EXPECT_EQ(pp::Token::LAST, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::LAST, token.type);
 }
 
 TEST(InputTest, NullString)
 {
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     EXPECT_FALSE(preprocessor.init(1, NULL, NULL));
 }
 
diff --git a/tests/preprocessor_tests/location_test.cpp b/tests/preprocessor_tests/location_test.cpp
index c8a5f38f..3b3c5a75 100644
--- a/tests/preprocessor_tests/location_test.cpp
+++ b/tests/preprocessor_tests/location_test.cpp
@@ -5,18 +5,21 @@
 //
 
 #include "gtest/gtest.h"
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
 static void PreprocessAndVerifyLocation(int count,
                                         const char* const string[],
                                         const int length[],
-                                        pp::Token::Location location)
+                                        const pp::SourceLocation& location)
 {
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(count, string, length));
-    EXPECT_EQ(pp::Token::IDENTIFIER, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
     EXPECT_STREQ("foo", token.value.c_str());
 
@@ -27,7 +30,7 @@ static void PreprocessAndVerifyLocation(int count,
 TEST(LocationTest, String0_Line1)
 {
     const char* str = "foo";
-    pp::Token::Location loc;
+    pp::SourceLocation loc;
     loc.file = 0;
     loc.line = 1;
 
@@ -38,7 +41,7 @@ TEST(LocationTest, String0_Line1)
 TEST(LocationTest, String0_Line2)
 {
     const char* str = "\nfoo";
-    pp::Token::Location loc;
+    pp::SourceLocation loc;
     loc.file = 0;
     loc.line = 2;
 
@@ -49,7 +52,7 @@ TEST(LocationTest, String0_Line2)
 TEST(LocationTest, String1_Line1)
 {
     const char* const str[] = {"\n\n", "foo"};
-    pp::Token::Location loc;
+    pp::SourceLocation loc;
     loc.file = 1;
     loc.line = 1;
 
@@ -60,7 +63,7 @@ TEST(LocationTest, String1_Line1)
 TEST(LocationTest, String1_Line2)
 {
     const char* const str[] = {"\n\n", "\nfoo"};
-    pp::Token::Location loc;
+    pp::SourceLocation loc;
     loc.file = 1;
     loc.line = 2;
 
@@ -71,7 +74,7 @@ TEST(LocationTest, String1_Line2)
 TEST(LocationTest, NewlineInsideCommentCounted)
 {
     const char* str = "/*\n\n*/foo";
-    pp::Token::Location loc;
+    pp::SourceLocation loc;
     loc.file = 0;
     loc.line = 3;
 
@@ -83,15 +86,16 @@ TEST(LocationTest, ErrorLocationAfterComment)
 {
     const char* str = "/*\n\n*/@";
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
-    EXPECT_EQ(pp::Token::INVALID_CHARACTER, preprocessor.lex(&token));
-    EXPECT_EQ(pp::Token::INVALID_CHARACTER, token.type);
-    EXPECT_STREQ("@", token.value.c_str());
 
-    EXPECT_EQ(0, token.location.file);
-    EXPECT_EQ(3, token.location.line);
+    pp::Diagnostics::ID id(pp::Diagnostics::INVALID_CHARACTER);
+    pp::SourceLocation loc(0, 3);
+    EXPECT_CALL(diagnostics, print(id, loc, "@"));
+
+    pp::Token token;
+    preprocessor.lex(&token);
 }
 
 // The location of a token straddling two or more strings is that of the
@@ -100,7 +104,7 @@ TEST(LocationTest, ErrorLocationAfterComment)
 TEST(LocationTest, TokenStraddlingTwoStrings)
 {
     const char* const str[] = {"f", "oo"};
-    pp::Token::Location loc;
+    pp::SourceLocation loc;
     loc.file = 0;
     loc.line = 1;
 
@@ -111,7 +115,7 @@ TEST(LocationTest, TokenStraddlingTwoStrings)
 TEST(LocationTest, TokenStraddlingThreeStrings)
 {
     const char* const str[] = {"f", "o", "o"};
-    pp::Token::Location loc;
+    pp::SourceLocation loc;
     loc.file = 0;
     loc.line = 1;
 
diff --git a/tests/preprocessor_tests/number_test.cpp b/tests/preprocessor_tests/number_test.cpp
index 6e7c267a..17df22a4 100644
--- a/tests/preprocessor_tests/number_test.cpp
+++ b/tests/preprocessor_tests/number_test.cpp
@@ -5,6 +5,8 @@
 //
 
 #include "gtest/gtest.h"
+
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
@@ -18,12 +20,15 @@ TEST_P(InvalidNumberTest, InvalidNumberIdentified)
 {
     const char* str = GetParam();
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
-    EXPECT_EQ(pp::Token::INVALID_NUMBER, preprocessor.lex(&token));
-    EXPECT_EQ(pp::Token::INVALID_NUMBER, token.type);
-    EXPECT_STREQ(str, token.value.c_str());
+
+    using testing::_;
+    EXPECT_CALL(diagnostics, print(pp::Diagnostics::INVALID_NUMBER, _, str));
+
+    pp::Token token;
+    preprocessor.lex(&token);
 }
 
 INSTANTIATE_TEST_CASE_P(InvalidIntegers, InvalidNumberTest,
@@ -48,10 +53,12 @@ TEST_P(IntegerTest, IntegerIdentified)
     str.push_back(std::tr1::get<1>(GetParam()));  // digit.
     const char* cstr = str.c_str();
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &cstr, 0));
-    EXPECT_EQ(pp::Token::CONST_INT, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::CONST_INT, token.type);
     EXPECT_STREQ(cstr, token.value.c_str());
 }
@@ -85,10 +92,12 @@ INSTANTIATE_TEST_CASE_P(HexadecimalInteger_A_F,
 
 static void PreprocessAndVerifyFloat(const char* str)
 {
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
-    EXPECT_EQ(pp::Token::CONST_FLOAT, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::CONST_FLOAT, token.type);
     EXPECT_STREQ(str, token.value.c_str());
 }
diff --git a/tests/preprocessor_tests/operator_test.cpp b/tests/preprocessor_tests/operator_test.cpp
index 9acb7f70..a619bb70 100644
--- a/tests/preprocessor_tests/operator_test.cpp
+++ b/tests/preprocessor_tests/operator_test.cpp
@@ -5,6 +5,8 @@
 //
 
 #include "gtest/gtest.h"
+
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
@@ -24,10 +26,12 @@ TEST_P(OperatorTest, Identified)
 {
     OperatorTestParam param = GetParam();
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &param.str, 0));
-    EXPECT_EQ(param.op, preprocessor.lex(&token));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     EXPECT_EQ(param.op, token.type);
 }
 
diff --git a/tests/preprocessor_tests/space_test.cpp b/tests/preprocessor_tests/space_test.cpp
index 8edb9873..8a60eb6a 100644
--- a/tests/preprocessor_tests/space_test.cpp
+++ b/tests/preprocessor_tests/space_test.cpp
@@ -5,6 +5,8 @@
 //
 
 #include "gtest/gtest.h"
+
+#include "MockDiagnostics.h"
 #include "Preprocessor.h"
 #include "Token.h"
 
@@ -29,11 +31,13 @@ TEST_P(SpaceCharTest, SpaceIgnored)
     str.append(identifier);
     const char* cstr = str.c_str();
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &cstr, 0));
+
+    pp::Token token;
     // Identifier "foo" is returned after ignoring the whitespace characters.
-    EXPECT_EQ(pp::Token::IDENTIFIER, preprocessor.lex(&token));
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
     EXPECT_STREQ(identifier, token.value.c_str());
     // The whitespace character is however recorded with the next token.
@@ -66,11 +70,13 @@ TEST_P(SpaceStringTest, SpaceIgnored)
     str.append(identifier);
     const char* cstr = str.c_str();
 
-    pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &cstr, 0));
+
+    pp::Token token;
+    preprocessor.lex(&token);
     // Identifier "foo" is returned after ignoring the whitespace characters.
-    EXPECT_EQ(pp::Token::IDENTIFIER, preprocessor.lex(&token));
     EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
     EXPECT_STREQ(identifier, token.value.c_str());
     // The whitespace character is however recorded with the next token.
@@ -93,23 +99,24 @@ TEST(SpaceTest, LeadingSpace)
     const char* str = " foo+ -bar";
 
     pp::Token token;
-    pp::Preprocessor preprocessor;
+    MockDiagnostics diagnostics;
+    pp::Preprocessor preprocessor(&diagnostics);
     ASSERT_TRUE(preprocessor.init(1, &str, 0));
 
-    EXPECT_EQ(pp::Token::IDENTIFIER, preprocessor.lex(&token));
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
     EXPECT_STREQ("foo", token.value.c_str());
     EXPECT_TRUE(token.hasLeadingSpace());
 
-    EXPECT_EQ('+', preprocessor.lex(&token));
+    preprocessor.lex(&token);
     EXPECT_EQ('+', token.type);
     EXPECT_FALSE(token.hasLeadingSpace());
 
-    EXPECT_EQ('-', preprocessor.lex(&token));
+    preprocessor.lex(&token);
     EXPECT_EQ('-', token.type);
     EXPECT_TRUE(token.hasLeadingSpace());
 
-    EXPECT_EQ(pp::Token::IDENTIFIER, preprocessor.lex(&token));
+    preprocessor.lex(&token);
     EXPECT_EQ(pp::Token::IDENTIFIER, token.type);
     EXPECT_STREQ("bar", token.value.c_str());
     EXPECT_FALSE(token.hasLeadingSpace());