From a3bcde2c105efb6220152ddeacc99c14d497b8f9 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Mon, 2 Nov 2020 15:17:50 +0000
Subject: [PATCH] Rm line() & column() from wsgl::Token & ast::Node

Use `source().range.begin.line` and `source().range.begin.column` instead.

Bug: tint:282
Change-Id: I6c9bf8766d6db2c9d8e7e1b8bafb2eea93e065d8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31441
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
---
 src/ast/decorated_variable_test.cc    |  8 ++--
 src/ast/discard_statement_test.cc     |  8 ++--
 src/ast/fallthrough_statement_test.cc |  4 +-
 src/ast/node.h                        |  6 ---
 src/ast/struct_member_test.cc         |  8 ++--
 src/ast/struct_test.cc                | 12 ++---
 src/ast/variable_test.cc              | 12 ++---
 src/reader/wgsl/lexer_test.cc         | 68 ++++++++++++++-------------
 src/reader/wgsl/parser_impl.cc        |  4 +-
 src/reader/wgsl/token.h               |  6 ---
 src/reader/wgsl/token_test.cc         |  4 +-
 11 files changed, 66 insertions(+), 74 deletions(-)

diff --git a/src/ast/decorated_variable_test.cc b/src/ast/decorated_variable_test.cc
index 6023df8962..3774d1c57d 100644
--- a/src/ast/decorated_variable_test.cc
+++ b/src/ast/decorated_variable_test.cc
@@ -40,8 +40,8 @@ TEST_F(DecoratedVariableTest, Creation) {
   EXPECT_EQ(dv.name(), "my_var");
   EXPECT_EQ(dv.storage_class(), StorageClass::kFunction);
   EXPECT_EQ(dv.type(), &t);
-  EXPECT_EQ(dv.line(), 0u);
-  EXPECT_EQ(dv.column(), 0u);
+  EXPECT_EQ(dv.source().range.begin.line, 0u);
+  EXPECT_EQ(dv.source().range.begin.column, 0u);
 }
 
 TEST_F(DecoratedVariableTest, CreationWithSource) {
@@ -53,8 +53,8 @@ TEST_F(DecoratedVariableTest, CreationWithSource) {
   EXPECT_EQ(dv.name(), "i");
   EXPECT_EQ(dv.storage_class(), StorageClass::kPrivate);
   EXPECT_EQ(dv.type(), &t);
-  EXPECT_EQ(dv.line(), 27u);
-  EXPECT_EQ(dv.column(), 4u);
+  EXPECT_EQ(dv.source().range.begin.line, 27u);
+  EXPECT_EQ(dv.source().range.begin.column, 4u);
 }
 
 TEST_F(DecoratedVariableTest, NoDecorations) {
diff --git a/src/ast/discard_statement_test.cc b/src/ast/discard_statement_test.cc
index 34cc13cc98..4a9e7793c5 100644
--- a/src/ast/discard_statement_test.cc
+++ b/src/ast/discard_statement_test.cc
@@ -26,14 +26,14 @@ using DiscardStatementTest = testing::Test;
 
 TEST_F(DiscardStatementTest, Creation) {
   DiscardStatement stmt;
-  EXPECT_EQ(stmt.line(), 0u);
-  EXPECT_EQ(stmt.column(), 0u);
+  EXPECT_EQ(stmt.source().range.begin.line, 0u);
+  EXPECT_EQ(stmt.source().range.begin.column, 0u);
 }
 
 TEST_F(DiscardStatementTest, Creation_WithSource) {
   DiscardStatement stmt(Source{Source::Location{20, 2}});
-  EXPECT_EQ(stmt.line(), 20u);
-  EXPECT_EQ(stmt.column(), 2u);
+  EXPECT_EQ(stmt.source().range.begin.line, 20u);
+  EXPECT_EQ(stmt.source().range.begin.column, 2u);
 }
 
 TEST_F(DiscardStatementTest, IsDiscard) {
diff --git a/src/ast/fallthrough_statement_test.cc b/src/ast/fallthrough_statement_test.cc
index bae03846cc..e1ed7f2ecd 100644
--- a/src/ast/fallthrough_statement_test.cc
+++ b/src/ast/fallthrough_statement_test.cc
@@ -24,8 +24,8 @@ using FallthroughStatementTest = testing::Test;
 
 TEST_F(FallthroughStatementTest, Creation) {
   FallthroughStatement stmt;
-  EXPECT_EQ(stmt.line(), 0u);
-  EXPECT_EQ(stmt.column(), 0u);
+  EXPECT_EQ(stmt.source().range.begin.line, 0u);
+  EXPECT_EQ(stmt.source().range.begin.column, 0u);
 }
 
 TEST_F(FallthroughStatementTest, Creation_WithSource) {
diff --git a/src/ast/node.h b/src/ast/node.h
index 4eb57e4e1b..38adf7f22b 100644
--- a/src/ast/node.h
+++ b/src/ast/node.h
@@ -34,12 +34,6 @@ class Node {
   /// @param source the source data
   void set_source(const Source& source) { source_ = source; }
 
-  // TODO(bclayton): Deprecate - use source().range.begin instead
-  /// @returns the line the node was declared on
-  size_t line() const { return source_.range.begin.line; }
-  /// @returns the column the node was declared on
-  size_t column() const { return source_.range.begin.column; }
-
   /// @returns true if the node is valid
   virtual bool IsValid() const = 0;
 
diff --git a/src/ast/struct_member_test.cc b/src/ast/struct_member_test.cc
index 3fcade3584..d20ef7b0e8 100644
--- a/src/ast/struct_member_test.cc
+++ b/src/ast/struct_member_test.cc
@@ -37,8 +37,8 @@ TEST_F(StructMemberTest, Creation) {
   EXPECT_EQ(st.type(), &i32);
   EXPECT_EQ(st.decorations().size(), 1u);
   EXPECT_TRUE(st.decorations()[0]->IsOffset());
-  EXPECT_EQ(st.line(), 0u);
-  EXPECT_EQ(st.column(), 0u);
+  EXPECT_EQ(st.source().range.begin.line, 0u);
+  EXPECT_EQ(st.source().range.begin.column, 0u);
 }
 
 TEST_F(StructMemberTest, CreationWithSource) {
@@ -49,8 +49,8 @@ TEST_F(StructMemberTest, CreationWithSource) {
   EXPECT_EQ(st.name(), "a");
   EXPECT_EQ(st.type(), &i32);
   EXPECT_EQ(st.decorations().size(), 0u);
-  EXPECT_EQ(st.line(), 27u);
-  EXPECT_EQ(st.column(), 4u);
+  EXPECT_EQ(st.source().range.begin.line, 27u);
+  EXPECT_EQ(st.source().range.begin.column, 4u);
 }
 
 TEST_F(StructMemberTest, IsValid) {
diff --git a/src/ast/struct_test.cc b/src/ast/struct_test.cc
index 6bea6a11af..828f454aee 100644
--- a/src/ast/struct_test.cc
+++ b/src/ast/struct_test.cc
@@ -38,8 +38,8 @@ TEST_F(StructTest, Creation) {
   Struct s{std::move(members)};
   EXPECT_EQ(s.members().size(), 1u);
   EXPECT_TRUE(s.decorations().empty());
-  EXPECT_EQ(s.line(), 0u);
-  EXPECT_EQ(s.column(), 0u);
+  EXPECT_EQ(s.source().range.begin.line, 0u);
+  EXPECT_EQ(s.source().range.begin.column, 0u);
 }
 
 TEST_F(StructTest, Creation_WithDecorations) {
@@ -56,8 +56,8 @@ TEST_F(StructTest, Creation_WithDecorations) {
   EXPECT_EQ(s.members().size(), 1u);
   ASSERT_EQ(s.decorations().size(), 1u);
   EXPECT_EQ(s.decorations()[0], StructDecoration::kBlock);
-  EXPECT_EQ(s.line(), 0u);
-  EXPECT_EQ(s.column(), 0u);
+  EXPECT_EQ(s.source().range.begin.line, 0u);
+  EXPECT_EQ(s.source().range.begin.column, 0u);
 }
 
 TEST_F(StructTest, CreationWithSourceAndDecorations) {
@@ -75,8 +75,8 @@ TEST_F(StructTest, CreationWithSourceAndDecorations) {
   EXPECT_EQ(s.members().size(), 1u);
   ASSERT_EQ(s.decorations().size(), 1u);
   EXPECT_EQ(s.decorations()[0], StructDecoration::kBlock);
-  EXPECT_EQ(s.line(), 27u);
-  EXPECT_EQ(s.column(), 4u);
+  EXPECT_EQ(s.source().range.begin.line, 27u);
+  EXPECT_EQ(s.source().range.begin.column, 4u);
 }
 
 TEST_F(StructTest, IsValid) {
diff --git a/src/ast/variable_test.cc b/src/ast/variable_test.cc
index e1c80856c3..ad006e305b 100644
--- a/src/ast/variable_test.cc
+++ b/src/ast/variable_test.cc
@@ -32,8 +32,8 @@ TEST_F(VariableTest, Creation) {
   EXPECT_EQ(v.name(), "my_var");
   EXPECT_EQ(v.storage_class(), StorageClass::kFunction);
   EXPECT_EQ(v.type(), &t);
-  EXPECT_EQ(v.line(), 0u);
-  EXPECT_EQ(v.column(), 0u);
+  EXPECT_EQ(v.source().range.begin.line, 0u);
+  EXPECT_EQ(v.source().range.begin.column, 0u);
 }
 
 TEST_F(VariableTest, CreationWithSource) {
@@ -44,8 +44,8 @@ TEST_F(VariableTest, CreationWithSource) {
   EXPECT_EQ(v.name(), "i");
   EXPECT_EQ(v.storage_class(), StorageClass::kPrivate);
   EXPECT_EQ(v.type(), &t);
-  EXPECT_EQ(v.line(), 27u);
-  EXPECT_EQ(v.column(), 4u);
+  EXPECT_EQ(v.source().range.begin.line, 27u);
+  EXPECT_EQ(v.source().range.begin.column, 4u);
 }
 
 TEST_F(VariableTest, CreationEmpty) {
@@ -61,8 +61,8 @@ TEST_F(VariableTest, CreationEmpty) {
   EXPECT_EQ(v.name(), "a_var");
   EXPECT_EQ(v.storage_class(), StorageClass::kWorkgroup);
   EXPECT_EQ(v.type(), &t);
-  EXPECT_EQ(v.line(), 27u);
-  EXPECT_EQ(v.column(), 4u);
+  EXPECT_EQ(v.source().range.begin.line, 27u);
+  EXPECT_EQ(v.source().range.begin.column, 4u);
 }
 
 TEST_F(VariableTest, IsValid) {
diff --git a/src/reader/wgsl/lexer_test.cc b/src/reader/wgsl/lexer_test.cc
index bccd13f7f3..810cf51e61 100644
--- a/src/reader/wgsl/lexer_test.cc
+++ b/src/reader/wgsl/lexer_test.cc
@@ -38,8 +38,8 @@ TEST_F(LexerTest, Skips_Whitespace) {
 
   auto t = l.next();
   EXPECT_TRUE(t.IsIdentifier());
-  EXPECT_EQ(t.line(), 2u);
-  EXPECT_EQ(t.column(), 6u);
+  EXPECT_EQ(t.source().range.begin.line, 2u);
+  EXPECT_EQ(t.source().range.begin.column, 6u);
   EXPECT_EQ(t.to_str(), "ident");
 
   t = l.next();
@@ -55,14 +55,14 @@ ident1 #ends with comment
 
   auto t = l.next();
   EXPECT_TRUE(t.IsIdentifier());
-  EXPECT_EQ(t.line(), 2u);
-  EXPECT_EQ(t.column(), 1u);
+  EXPECT_EQ(t.source().range.begin.line, 2u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
   EXPECT_EQ(t.to_str(), "ident1");
 
   t = l.next();
   EXPECT_TRUE(t.IsIdentifier());
-  EXPECT_EQ(t.line(), 4u);
-  EXPECT_EQ(t.column(), 2u);
+  EXPECT_EQ(t.source().range.begin.line, 4u);
+  EXPECT_EQ(t.source().range.begin.column, 2u);
   EXPECT_EQ(t.to_str(), "ident2");
 
   t = l.next();
@@ -76,20 +76,20 @@ TEST_F(LexerTest, StringTest_Parse) {
   auto t = l.next();
   EXPECT_TRUE(t.IsIdentifier());
   EXPECT_EQ(t.to_str(), "id");
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(1u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
 
   t = l.next();
   EXPECT_TRUE(t.IsStringLiteral());
   EXPECT_EQ(t.to_str(), "this is string content");
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(4u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 4u);
 
   t = l.next();
   EXPECT_TRUE(t.IsIdentifier());
   EXPECT_EQ(t.to_str(), "id2");
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(29u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 29u);
 }
 
 TEST_F(LexerTest, StringTest_Unterminated) {
@@ -99,12 +99,14 @@ TEST_F(LexerTest, StringTest_Unterminated) {
   auto t = l.next();
   EXPECT_TRUE(t.IsIdentifier());
   EXPECT_EQ(t.to_str(), "id");
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(1u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
 
   t = l.next();
   EXPECT_TRUE(t.IsStringLiteral());
   EXPECT_EQ(t.to_str(), "this is string content");
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 4u);
 
   t = l.next();
   EXPECT_TRUE(t.IsEof());
@@ -127,8 +129,8 @@ TEST_P(FloatTest, Parse) {
   auto t = l.next();
   EXPECT_TRUE(t.IsFloatLiteral());
   EXPECT_EQ(t.to_f32(), params.result);
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(1u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
 
   t = l.next();
   EXPECT_TRUE(t.IsEof());
@@ -178,8 +180,8 @@ TEST_P(IdentifierTest, Parse) {
 
   auto t = l.next();
   EXPECT_TRUE(t.IsIdentifier());
-  EXPECT_EQ(t.line(), 1u);
-  EXPECT_EQ(t.column(), 1u);
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
   EXPECT_EQ(t.to_str(), GetParam());
 }
 INSTANTIATE_TEST_SUITE_P(
@@ -212,8 +214,8 @@ TEST_P(IntegerTest_HexSigned, Matches) {
 
   auto t = l.next();
   EXPECT_TRUE(t.IsSintLiteral());
-  EXPECT_EQ(t.line(), 1u);
-  EXPECT_EQ(t.column(), 1u);
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
   EXPECT_EQ(t.to_i32(), params.result);
 }
 INSTANTIATE_TEST_SUITE_P(
@@ -259,8 +261,8 @@ TEST_P(IntegerTest_HexUnsigned, Matches) {
 
   auto t = l.next();
   EXPECT_TRUE(t.IsUintLiteral());
-  EXPECT_EQ(t.line(), 1u);
-  EXPECT_EQ(t.column(), 1u);
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
   EXPECT_EQ(t.to_u32(), params.result);
 
   t = l.next();
@@ -302,8 +304,8 @@ TEST_P(IntegerTest_Unsigned, Matches) {
   auto t = l.next();
   EXPECT_TRUE(t.IsUintLiteral());
   EXPECT_EQ(t.to_u32(), params.result);
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(1u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
 }
 INSTANTIATE_TEST_SUITE_P(LexerTest,
                          IntegerTest_Unsigned,
@@ -329,8 +331,8 @@ TEST_P(IntegerTest_Signed, Matches) {
   auto t = l.next();
   EXPECT_TRUE(t.IsSintLiteral());
   EXPECT_EQ(t.to_i32(), params.result);
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(1u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
 }
 INSTANTIATE_TEST_SUITE_P(
     LexerTest,
@@ -371,11 +373,12 @@ TEST_P(PunctuationTest, Parses) {
 
   auto t = l.next();
   EXPECT_TRUE(t.Is(params.type));
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(1u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
 
   t = l.next();
-  EXPECT_EQ(1 + std::string(params.input).size(), t.column());
+  EXPECT_EQ(t.source().range.begin.column,
+            1 + std::string(params.input).size());
 }
 INSTANTIATE_TEST_SUITE_P(
     LexerTest,
@@ -421,11 +424,12 @@ TEST_P(KeywordTest, Parses) {
 
   auto t = l.next();
   EXPECT_TRUE(t.Is(params.type)) << params.input;
-  EXPECT_EQ(1u, t.line());
-  EXPECT_EQ(1u, t.column());
+  EXPECT_EQ(t.source().range.begin.line, 1u);
+  EXPECT_EQ(t.source().range.begin.column, 1u);
 
   t = l.next();
-  EXPECT_EQ(1 + std::string(params.input).size(), t.column());
+  EXPECT_EQ(t.source().range.begin.column,
+            1 + std::string(params.input).size());
 }
 INSTANTIATE_TEST_SUITE_P(
     LexerTest,
diff --git a/src/reader/wgsl/parser_impl.cc b/src/reader/wgsl/parser_impl.cc
index e8100f746b..c445f02bdf 100644
--- a/src/reader/wgsl/parser_impl.cc
+++ b/src/reader/wgsl/parser_impl.cc
@@ -132,8 +132,8 @@ ParserImpl::~ParserImpl() {
 }
 
 void ParserImpl::set_error(const Token& t, const std::string& err) {
-  auto prefix =
-      std::to_string(t.line()) + ":" + std::to_string(t.column()) + ": ";
+  auto prefix = std::to_string(t.source().range.begin.line) + ":" +
+                std::to_string(t.source().range.begin.column) + ": ";
 
   if (t.IsReservedKeyword()) {
     error_ = prefix + "reserved token (" + t.to_str() + ") found";
diff --git a/src/reader/wgsl/token.h b/src/reader/wgsl/token.h
index 9d920613b4..e6e5b2869b 100644
--- a/src/reader/wgsl/token.h
+++ b/src/reader/wgsl/token.h
@@ -778,12 +778,6 @@ class Token {
   /// @returns true if token is a 'workgroup_size'
   bool IsWorkgroupSize() const { return type_ == Type::kWorkgroupSize; }
 
-  // TODO(bclayton): Deprecate - use source().range.begin instead
-  /// @returns the source line of the token
-  size_t line() const { return source_.range.begin.line; }
-  /// @returns the source column of the token
-  size_t column() const { return source_.range.begin.column; }
-
   /// @returns the source information for this token
   Source source() const { return source_; }
 
diff --git a/src/reader/wgsl/token_test.cc b/src/reader/wgsl/token_test.cc
index 13d467eb3b..4911d05d2f 100644
--- a/src/reader/wgsl/token_test.cc
+++ b/src/reader/wgsl/token_test.cc
@@ -68,8 +68,8 @@ TEST_F(TokenTest, ReturnsMaxU32) {
 
 TEST_F(TokenTest, Source) {
   Token t(Token::Type::kUintLiteral, Source{Source::Location{3, 9}});
-  EXPECT_EQ(t.line(), 3u);
-  EXPECT_EQ(t.column(), 9u);
+  EXPECT_EQ(t.source().range.begin.line, 3u);
+  EXPECT_EQ(t.source().range.begin.column, 9u);
 }
 
 }  // namespace