commit c54f02e06ff3a16c7efe1e6a5b4f2bec829159a2
author John Stiles <johnstiles@google.com> Wed May 17 11:13:39 2023 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Wed May 17 17:14:34 2023 +0000
tree ade738ca7384d99850e333006ca992946e0d5a5a
parent bdee8fd5888f2b72ad1fe82c7aa2535e7358748d

Disallow calling DSLWriter::Var more than once.

Previously, DSLVars would keep track of their `fInitialized`
state, and calling Var twice would just return the already-
initialized values. However, the only remaining use case of
calling Var() a second time was in `Parser::addToSymbolTable`.
Now the only caller of `Var` is `Declaration`, and it isn't
called more than once.

This allows us to remove the `fInitialized` flag.

Change-Id: Ibf5a4797aff72bebdd58a85ed8816eb87ecba650
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/699596
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

src/sksl/SkSLParser.cpp

diff --git a/src/sksl/SkSLParser.cpp b/src/sksl/SkSLParser.cpp
index 7fe6063..2a41821 100644
--- a/src/sksl/SkSLParser.cpp
+++ b/src/sksl/SkSLParser.cpp
@@ -148,8 +148,8 @@
 }
 
 void Parser::addToSymbolTable(DSLVarBase& var, Position pos) {
-    if (SkSL::Variable* skslVar = DSLWriter::Var(var)) {
-        this->symbolTable()->addWithoutOwnership(skslVar);
+    if (var.fVar) {
+        this->symbolTable()->addWithoutOwnership(var.fVar);
     }
 }
 

src/sksl/dsl/DSLVar.h

diff --git a/src/sksl/dsl/DSLVar.h b/src/sksl/dsl/DSLVar.h
index 8f37703..9462941 100644
--- a/src/sksl/dsl/DSLVar.h
+++ b/src/sksl/dsl/DSLVar.h
@@ -42,10 +42,6 @@
 
     Position fModifiersPos;
     SkSL::Modifiers fModifiers;
-    // We only need to keep track of the type here so that we can create the SkSL::Variable. For
-    // predefined variables this field is unnecessary, so we don't bother tracking it and just set
-    // it to kVoid; in other words, you shouldn't generally be relying on this field to be correct.
-    // If you need to determine the variable's type, look at DSLWriter::Var(...)->type() instead.
     DSLType fType;
     std::unique_ptr<SkSL::Statement> fDeclaration;
     SkSL::Variable* fVar = nullptr;
@@ -54,7 +50,6 @@
     DSLExpression fInitialValue;
     Position fPosition;
     VariableStorage fStorage;
-    bool fInitialized = false;
 };
 
 /**

src/sksl/dsl/priv/DSLWriter.cpp

diff --git a/src/sksl/dsl/priv/DSLWriter.cpp b/src/sksl/dsl/priv/DSLWriter.cpp
index 0ffba79..df92cdb 100644
--- a/src/sksl/dsl/priv/DSLWriter.cpp
+++ b/src/sksl/dsl/priv/DSLWriter.cpp
@@ -25,31 +25,25 @@
 namespace dsl {
 
 SkSL::Variable* DSLWriter::Var(DSLVarBase& var) {
-    // fInitialized is true if we have attempted to create a var, whether or not we actually
-    // succeeded. If it's true, we don't want to try again, to avoid reporting the same error
-    // multiple times.
-    if (!var.fInitialized) {
-        // We haven't even attempted to create a var yet, so fVar and fDeclaration ought to be null
-        SkASSERT(!var.fVar);
-        SkASSERT(!var.fDeclaration);
+    SkASSERT(var.fStorage != SkSL::VariableStorage::kParameter);  // use CreateParameterVar instead
 
-        var.fInitialized = true;
-        std::unique_ptr<SkSL::Variable> skslvar = SkSL::Variable::Convert(ThreadContext::Context(),
-                                                                          var.fPosition,
-                                                                          var.fModifiersPos,
-                                                                          var.fModifiers,
-                                                                          &var.fType.skslType(),
-                                                                          var.fNamePosition,
-                                                                          var.fName,
-                                                                          var.fStorage);
-        if (var.fStorage != SkSL::VariableStorage::kParameter) {
-            var.fDeclaration = VarDeclaration::Convert(ThreadContext::Context(),
-                                                       std::move(skslvar),
-                                                       var.fInitialValue.releaseIfPossible());
-            if (var.fDeclaration) {
-                var.fVar = var.fDeclaration->as<VarDeclaration>().var();
-            }
-        }
+    // We haven't attempted to create a var yet; fVar and fDeclaration ought to be null.
+    SkASSERT(!var.fVar);
+    SkASSERT(!var.fDeclaration);
+
+    std::unique_ptr<SkSL::Variable> skslvar = SkSL::Variable::Convert(ThreadContext::Context(),
+                                                                      var.fPosition,
+                                                                      var.fModifiersPos,
+                                                                      var.fModifiers,
+                                                                      &var.fType.skslType(),
+                                                                      var.fNamePosition,
+                                                                      var.fName,
+                                                                      var.fStorage);
+    var.fDeclaration = VarDeclaration::Convert(ThreadContext::Context(),
+                                               std::move(skslvar),
+                                               var.fInitialValue.releaseIfPossible());
+    if (var.fDeclaration) {
+        var.fVar = var.fDeclaration->as<VarDeclaration>().var();
     }
     return var.fVar;
 }