ast: Remove Variable constructor that doesn't take a source

set_source() will be removed, so sources will only be specifiable at construction time.

Bug: tint:390
Change-Id: I5c79efd3fa501ebd9308f7f93cfb77bc12198047
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35009
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>

src/ast/builder.cc

@@ -32,7 +32,7 @@ Builder::~Builder() = default;
 Variable* Builder::Var(const std::string& name,
                        StorageClass storage,
                        type::Type* type) {
-  auto* var = create<Variable>(name, storage, type);
+  auto* var = create<Variable>(Source{}, name, storage, type);
   OnVariableBuilt(var);
   return var;
 }
@@ -40,7 +40,7 @@ Variable* Builder::Var(const std::string& name,
 Variable* Builder::Const(const std::string& name,
                          StorageClass storage,
                          type::Type* type) {
-  auto* var = create<Variable>(name, storage, type);
+  auto* var = create<Variable>(Source{}, name, storage, type);
   var->set_is_const(true);
   OnVariableBuilt(var);
   return var;

src/ast/decorated_variable_test.cc

@@ -34,7 +34,7 @@ using DecoratedVariableTest = TestHelper;
 
 TEST_F(DecoratedVariableTest, Creation) {
   type::I32 t;
-  auto* var = create<Variable>("my_var", StorageClass::kFunction, &t);
+  auto* var = create<Variable>(Source{}, "my_var", StorageClass::kFunction, &t);
   DecoratedVariable dv(var);
 
   EXPECT_EQ(dv.name(), "my_var");
@@ -63,7 +63,7 @@ TEST_F(DecoratedVariableTest, CreationWithSource) {
 
 TEST_F(DecoratedVariableTest, NoDecorations) {
   type::I32 t;
-  auto* var = create<Variable>("my_var", StorageClass::kFunction, &t);
+  auto* var = create<Variable>(Source{}, "my_var", StorageClass::kFunction, &t);
   DecoratedVariable dv(var);
   EXPECT_FALSE(dv.HasLocationDecoration());
   EXPECT_FALSE(dv.HasBuiltinDecoration());
@@ -72,7 +72,7 @@ TEST_F(DecoratedVariableTest, NoDecorations) {
 
 TEST_F(DecoratedVariableTest, WithDecorations) {
   type::F32 t;
-  auto* var = create<Variable>("my_var", StorageClass::kFunction, &t);
+  auto* var = create<Variable>(Source{}, "my_var", StorageClass::kFunction, &t);
   DecoratedVariable dv(var);
 
   VariableDecorationList decos;
@@ -89,7 +89,7 @@ TEST_F(DecoratedVariableTest, WithDecorations) {
 
 TEST_F(DecoratedVariableTest, ConstantId) {
   type::F32 t;
-  auto* var = create<Variable>("my_var", StorageClass::kFunction, &t);
+  auto* var = create<Variable>(Source{}, "my_var", StorageClass::kFunction, &t);
   DecoratedVariable dv(var);
 
   VariableDecorationList decos;
@@ -101,20 +101,21 @@ TEST_F(DecoratedVariableTest, ConstantId) {
 
 TEST_F(DecoratedVariableTest, IsValid) {
   type::I32 t;
-  auto* var = create<Variable>("my_var", StorageClass::kNone, &t);
+  auto* var = create<Variable>(Source{}, "my_var", StorageClass::kNone, &t);
   DecoratedVariable dv(var);
   EXPECT_TRUE(dv.IsValid());
 }
 
 TEST_F(DecoratedVariableTest, IsDecorated) {
   type::I32 t;
-  DecoratedVariable dv(create<Variable>("my_var", StorageClass::kNone, &t));
+  DecoratedVariable dv(
+      create<Variable>(Source{}, "my_var", StorageClass::kNone, &t));
   EXPECT_TRUE(dv.Is<DecoratedVariable>());
 }
 
 TEST_F(DecoratedVariableTest, to_str) {
   type::F32 t;
-  auto* var = create<Variable>("my_var", StorageClass::kFunction, &t);
+  auto* var = create<Variable>(Source{}, "my_var", StorageClass::kFunction, &t);
   DecoratedVariable dv(var);
   dv.set_constructor(create<IdentifierExpression>("expr"));
 

src/ast/function_test.cc

@@ -37,7 +37,8 @@ TEST_F(FunctionTest, Creation) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
   auto* var = params[0];
 
   Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
@@ -53,7 +54,8 @@ TEST_F(FunctionTest, Creation_WithSource) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
 
   Function f(Source{Source::Location{20, 2}}, "func", params, &void_type,
              create<BlockStatement>(), FunctionDecorationList{});
@@ -66,7 +68,7 @@ TEST_F(FunctionTest, AddDuplicateReferencedVariables) {
   type::Void void_type;
   type::I32 i32;
 
-  Variable v("var", StorageClass::kInput, &i32);
+  Variable v(Source{}, "var", StorageClass::kInput, &i32);
   Function f(Source{}, "func", VariableList{}, &void_type,
              create<BlockStatement>(), FunctionDecorationList{});
 
@@ -77,7 +79,7 @@ TEST_F(FunctionTest, AddDuplicateReferencedVariables) {
   f.add_referenced_module_variable(&v);
   ASSERT_EQ(f.referenced_module_variables().size(), 1u);
 
-  Variable v2("var2", StorageClass::kOutput, &i32);
+  Variable v2(Source{}, "var2", StorageClass::kOutput, &i32);
   f.add_referenced_module_variable(&v2);
   ASSERT_EQ(f.referenced_module_variables().size(), 2u);
   EXPECT_EQ(f.referenced_module_variables()[1], &v2);
@@ -87,19 +89,21 @@ TEST_F(FunctionTest, GetReferenceLocations) {
   type::Void void_type;
   type::I32 i32;
 
-  DecoratedVariable loc1(create<Variable>("loc1", StorageClass::kInput, &i32));
+  DecoratedVariable loc1(
+      create<Variable>(Source{}, "loc1", StorageClass::kInput, &i32));
   loc1.set_decorations({create<LocationDecoration>(0, Source{})});
 
-  DecoratedVariable loc2(create<Variable>("loc2", StorageClass::kInput, &i32));
+  DecoratedVariable loc2(
+      create<Variable>(Source{}, "loc2", StorageClass::kInput, &i32));
   loc2.set_decorations({create<LocationDecoration>(1, Source{})});
 
   DecoratedVariable builtin1(
-      create<Variable>("builtin1", StorageClass::kInput, &i32));
+      create<Variable>(Source{}, "builtin1", StorageClass::kInput, &i32));
   builtin1.set_decorations(
       {create<BuiltinDecoration>(Builtin::kPosition, Source{})});
 
   DecoratedVariable builtin2(
-      create<Variable>("builtin2", StorageClass::kInput, &i32));
+      create<Variable>(Source{}, "builtin2", StorageClass::kInput, &i32));
   builtin2.set_decorations(
       {create<BuiltinDecoration>(Builtin::kFragDepth, Source{})});
 
@@ -124,19 +128,21 @@ TEST_F(FunctionTest, GetReferenceBuiltins) {
   type::Void void_type;
   type::I32 i32;
 
-  DecoratedVariable loc1(create<Variable>("loc1", StorageClass::kInput, &i32));
+  DecoratedVariable loc1(
+      create<Variable>(Source{}, "loc1", StorageClass::kInput, &i32));
   loc1.set_decorations({create<LocationDecoration>(0, Source{})});
 
-  DecoratedVariable loc2(create<Variable>("loc2", StorageClass::kInput, &i32));
+  DecoratedVariable loc2(
+      create<Variable>(Source{}, "loc2", StorageClass::kInput, &i32));
   loc2.set_decorations({create<LocationDecoration>(1, Source{})});
 
   DecoratedVariable builtin1(
-      create<Variable>("builtin1", StorageClass::kInput, &i32));
+      create<Variable>(Source{}, "builtin1", StorageClass::kInput, &i32));
   builtin1.set_decorations(
       {create<BuiltinDecoration>(Builtin::kPosition, Source{})});
 
   DecoratedVariable builtin2(
-      create<Variable>("builtin2", StorageClass::kInput, &i32));
+      create<Variable>(Source{}, "builtin2", StorageClass::kInput, &i32));
   builtin2.set_decorations(
       {create<BuiltinDecoration>(Builtin::kFragDepth, Source{})});
 
@@ -176,7 +182,8 @@ TEST_F(FunctionTest, IsValid) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
 
   auto* block = create<BlockStatement>();
   block->append(create<DiscardStatement>());
@@ -192,7 +199,8 @@ TEST_F(FunctionTest, IsValid_EmptyName) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
 
   Function f(Source{}, "", params, &void_type, create<BlockStatement>(),
              FunctionDecorationList{});
@@ -203,7 +211,8 @@ TEST_F(FunctionTest, IsValid_MissingReturnType) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
 
   Function f(Source{}, "func", params, nullptr, create<BlockStatement>(),
              FunctionDecorationList{});
@@ -215,7 +224,8 @@ TEST_F(FunctionTest, IsValid_NullParam) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
   params.push_back(nullptr);
 
   Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
@@ -227,7 +237,8 @@ TEST_F(FunctionTest, IsValid_InvalidParam) {
   type::Void void_type;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, nullptr));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, nullptr));
 
   Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
              FunctionDecorationList{});
@@ -239,7 +250,8 @@ TEST_F(FunctionTest, IsValid_NullBodyStatement) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
 
   auto* block = create<BlockStatement>();
   block->append(create<DiscardStatement>());
@@ -256,7 +268,8 @@ TEST_F(FunctionTest, IsValid_InvalidBodyStatement) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
 
   auto* block = create<BlockStatement>();
   block->append(create<DiscardStatement>());
@@ -316,7 +329,8 @@ TEST_F(FunctionTest, ToStr_WithParams) {
   type::I32 i32;
 
   VariableList params;
-  params.push_back(create<Variable>("var", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var", StorageClass::kNone, &i32));
 
   auto* block = create<BlockStatement>();
   block->append(create<DiscardStatement>());
@@ -355,8 +369,10 @@ TEST_F(FunctionTest, TypeName_WithParams) {
   type::F32 f32;
 
   VariableList params;
-  params.push_back(create<Variable>("var1", StorageClass::kNone, &i32));
-  params.push_back(create<Variable>("var2", StorageClass::kNone, &f32));
+  params.push_back(
+      create<Variable>(Source{}, "var1", StorageClass::kNone, &i32));
+  params.push_back(
+      create<Variable>(Source{}, "var2", StorageClass::kNone, &f32));
 
   Function f(Source{}, "func", params, &void_type, create<BlockStatement>(),
              FunctionDecorationList{});

src/ast/module_test.cc

@@ -67,7 +67,7 @@ TEST_F(ModuleTest, IsValid_Empty) {
 
 TEST_F(ModuleTest, IsValid_GlobalVariable) {
   type::F32 f32;
-  auto* var = create<Variable>("var", StorageClass::kInput, &f32);
+  auto* var = create<Variable>(Source{}, "var", StorageClass::kInput, &f32);
 
   Module m;
   m.AddGlobalVariable(var);
@@ -81,7 +81,7 @@ TEST_F(ModuleTest, IsValid_Null_GlobalVariable) {
 }
 
 TEST_F(ModuleTest, IsValid_Invalid_GlobalVariable) {
-  auto* var = create<Variable>("var", StorageClass::kInput, nullptr);
+  auto* var = create<Variable>(Source{}, "var", StorageClass::kInput, nullptr);
 
   Module m;
   m.AddGlobalVariable(var);

src/ast/variable.cc

@@ -27,9 +27,6 @@ namespace ast {
 
 Variable::Variable() = default;
 
-Variable::Variable(const std::string& name, StorageClass sc, type::Type* type)
-    : Base(), name_(name), storage_class_(sc), type_(type) {}
-
 Variable::Variable(const Source& source,
                    const std::string& name,
                    StorageClass sc,
@@ -41,9 +38,8 @@ Variable::Variable(Variable&&) = default;
 Variable::~Variable() = default;
 
 Variable* Variable::Clone(CloneContext* ctx) const {
-  auto* cloned =
-      ctx->mod->create<Variable>(name(), storage_class(), ctx->Clone(type()));
-  cloned->set_source(ctx->Clone(source()));
+  auto* cloned = ctx->mod->create<Variable>(
+      ctx->Clone(source()), name(), storage_class(), ctx->Clone(type()));
   cloned->set_constructor(ctx->Clone(constructor()));
   cloned->set_is_const(is_const());
   return cloned;

src/ast/variable.h

@@ -78,11 +78,6 @@ namespace ast {
 /// The storage class for a formal parameter is always StorageClass::kNone.
 class Variable : public Castable<Variable, Node> {
  public:
-  /// Create a variable
-  /// @param name the variables name
-  /// @param sc the variable storage class
-  /// @param type the value type
-  Variable(const std::string& name, StorageClass sc, type::Type* type);
   /// Create a variable
   /// @param source the variable source
   /// @param name the variables name

src/ast/variable_decl_statement_test.cc

@@ -26,7 +26,7 @@ using VariableDeclStatementTest = TestHelper;
 
 TEST_F(VariableDeclStatementTest, Creation) {
   type::F32 f32;
-  auto* var = create<Variable>("a", StorageClass::kNone, &f32);
+  auto* var = create<Variable>(Source{}, "a", StorageClass::kNone, &f32);
 
   VariableDeclStatement stmt(var);
   EXPECT_EQ(stmt.variable(), var);
@@ -34,7 +34,7 @@ TEST_F(VariableDeclStatementTest, Creation) {
 
 TEST_F(VariableDeclStatementTest, Creation_WithSource) {
   type::F32 f32;
-  auto* var = create<Variable>("a", StorageClass::kNone, &f32);
+  auto* var = create<Variable>(Source{}, "a", StorageClass::kNone, &f32);
 
   VariableDeclStatement stmt(Source{Source::Location{20, 2}}, var);
   auto src = stmt.source();
@@ -44,7 +44,7 @@ TEST_F(VariableDeclStatementTest, Creation_WithSource) {
 
 TEST_F(VariableDeclStatementTest, IsVariableDecl) {
   type::F32 f32;
-  auto* var = create<Variable>("a", StorageClass::kNone, &f32);
+  auto* var = create<Variable>(Source{}, "a", StorageClass::kNone, &f32);
 
   VariableDeclStatement s(var);
   EXPECT_TRUE(s.Is<VariableDeclStatement>());
@@ -52,14 +52,14 @@ TEST_F(VariableDeclStatementTest, IsVariableDecl) {
 
 TEST_F(VariableDeclStatementTest, IsValid) {
   type::F32 f32;
-  auto* var = create<Variable>("a", StorageClass::kNone, &f32);
+  auto* var = create<Variable>(Source{}, "a", StorageClass::kNone, &f32);
   VariableDeclStatement stmt(var);
   EXPECT_TRUE(stmt.IsValid());
 }
 
 TEST_F(VariableDeclStatementTest, IsValid_InvalidVariable) {
   type::F32 f32;
-  auto* var = create<Variable>("", StorageClass::kNone, &f32);
+  auto* var = create<Variable>(Source{}, "", StorageClass::kNone, &f32);
   VariableDeclStatement stmt(var);
   EXPECT_FALSE(stmt.IsValid());
 }
@@ -71,7 +71,7 @@ TEST_F(VariableDeclStatementTest, IsValid_NullVariable) {
 
 TEST_F(VariableDeclStatementTest, ToStr) {
   type::F32 f32;
-  auto* var = create<Variable>("a", StorageClass::kNone, &f32);
+  auto* var = create<Variable>(Source{}, "a", StorageClass::kNone, &f32);
 
   VariableDeclStatement stmt(Source{Source::Location{20, 2}}, var);
   std::ostringstream out;

src/ast/variable_test.cc

@@ -27,7 +27,7 @@ using VariableTest = TestHelper;
 
 TEST_F(VariableTest, Creation) {
   type::I32 t;
-  Variable v("my_var", StorageClass::kFunction, &t);
+  Variable v(Source{}, "my_var", StorageClass::kFunction, &t);
 
   EXPECT_EQ(v.name(), "my_var");
   EXPECT_EQ(v.storage_class(), StorageClass::kFunction);
@@ -68,43 +68,43 @@ TEST_F(VariableTest, CreationEmpty) {
 
 TEST_F(VariableTest, IsValid) {
   type::I32 t;
-  Variable v{"my_var", StorageClass::kNone, &t};
+  Variable v{Source{}, "my_var", StorageClass::kNone, &t};
   EXPECT_TRUE(v.IsValid());
 }
 
 TEST_F(VariableTest, IsValid_WithConstructor) {
   type::I32 t;
-  Variable v{"my_var", StorageClass::kNone, &t};
+  Variable v{Source{}, "my_var", StorageClass::kNone, &t};
   v.set_constructor(create<IdentifierExpression>("ident"));
   EXPECT_TRUE(v.IsValid());
 }
 
 TEST_F(VariableTest, IsValid_MissinName) {
   type::I32 t;
-  Variable v{"", StorageClass::kNone, &t};
+  Variable v{Source{}, "", StorageClass::kNone, &t};
   EXPECT_FALSE(v.IsValid());
 }
 
 TEST_F(VariableTest, IsValid_MissingType) {
-  Variable v{"x", StorageClass::kNone, nullptr};
+  Variable v{Source{}, "x", StorageClass::kNone, nullptr};
   EXPECT_FALSE(v.IsValid());
 }
 
 TEST_F(VariableTest, IsValid_MissingBoth) {
-  Variable v("", StorageClass::kNone, nullptr);
+  Variable v{Source{}, "", StorageClass::kNone, nullptr};
   EXPECT_FALSE(v.IsValid());
 }
 
 TEST_F(VariableTest, IsValid_InvalidConstructor) {
   type::I32 t;
-  Variable v{"my_var", StorageClass::kNone, &t};
+  Variable v{Source{}, "my_var", StorageClass::kNone, &t};
   v.set_constructor(create<IdentifierExpression>(""));
   EXPECT_FALSE(v.IsValid());
 }
 
 TEST_F(VariableTest, to_str) {
   type::F32 t;
-  Variable v{"my_var", StorageClass::kFunction, &t};
+  Variable v{Source{}, "my_var", StorageClass::kFunction, &t};
   std::ostringstream out;
   v.to_str(out, 2);
   EXPECT_EQ(out.str(), R"(  Variable{