refactored SkSL VarDeclaration handling

src/sksl/SkSLParser.cpp

 /*
  * Copyright 2016 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
  
 #include "stdio.h"
 #include "SkSLParser.h"
 #include "SkSLToken.h"
@@ skipping 256 matching lines @@
         return std::unique_ptr<ASTDeclaration>(new ASTFunction(name.fPosition, std::move(type), 
                                                                std::move(name.fText), 
                                                                std::move(parameters), 
                                                                std::move(body)));
     } else {
         return this->varDeclarationEnd(modifiers, std::move(type), name.fText);
     }
 }
 
 /* modifiers type IDENTIFIER varDeclarationEnd */
-std::unique_ptr<ASTVarDeclaration> Parser::varDeclaration() {
+std::unique_ptr<ASTVarDeclarations> Parser::varDeclarations() {
     ASTModifiers modifiers = this->modifiers();
     std::unique_ptr<ASTType> type(this->type());
     if (!type) {
         return nullptr;
     }
     Token name;
     if (!this->expect(Token::IDENTIFIER, "an identifier", &name)) {
         return nullptr;
     }
     return this->varDeclarationEnd(modifiers, std::move(type), std::move(name.fText));
 }
 
 /* STRUCT IDENTIFIER LBRACE varDeclaration* RBRACE */
 std::unique_ptr<ASTType> Parser::structDeclaration() {
     if (!this->expect(Token::STRUCT, "'struct'")) {
         return nullptr;
     }
     Token name;
     if (!this->expect(Token::IDENTIFIER, "an identifier", &name)) {
         return nullptr;
     }
     if (!this->expect(Token::LBRACE, "'{'")) {
         return nullptr;
     }
     std::vector<Type::Field> fields;
     while (this->peek().fKind != Token::RBRACE) {
-        std::unique_ptr<ASTVarDeclaration> decl = this->varDeclaration();
+        std::unique_ptr<ASTVarDeclarations> decl = this->varDeclarations();
         if (!decl) {
             return nullptr;
         }
-        for (size_t i = 0; i < decl->fNames.size(); i++) {
+        for (const auto& var : decl->fVars) {
             auto type = (const Type*) fTypes[decl->fType->fName];
-            for (int j = (int) decl->fSizes[i].size() - 1; j >= 0; j--) {
-                if (decl->fSizes[i][j]->fKind != ASTExpression::kInt_Kind) {
+            for (int i = (int) var.fSizes.size() - 1; i >= 0; i--) {
+                if (var.fSizes[i]->fKind != ASTExpression::kInt_Kind) {
                     this->error(decl->fPosition, "array size in struct field must be a constant");
                 }
-                uint64_t columns = ((ASTIntLiteral&) *decl->fSizes[i][j]).fValue;
+                uint64_t columns = ((ASTIntLiteral&) *var.fSizes[i]).fValue;
                 std::string name = type->name() + "[" + to_string(columns) + "]";
                 type = new Type(name, Type::kArray_Kind, *type, (int) columns);
                 fTypes.takeOwnership((Type*) type);
             }
-            fields.push_back(Type::Field(decl->fModifiers, decl->fNames[i], type));
-            if (decl->fValues[i]) {
+            fields.push_back(Type::Field(decl->fModifiers, var.fName, type));
+            if (var.fValue) {
                 this->error(decl->fPosition, "initializers are not permitted on struct fields");
             }
         }
     }
     if (!this->expect(Token::RBRACE, "'}'")) {
         return nullptr;
     }
     fTypes.add(name.fText, std::unique_ptr<Type>(new Type(name.fText, fields)));
     return std::unique_ptr<ASTType>(new ASTType(name.fPosition, name.fText, 
                                                 ASTType::kStruct_Kind));
 }
 
 /* structDeclaration ((IDENTIFIER varDeclarationEnd) | SEMICOLON) */
-std::unique_ptr<ASTVarDeclaration> Parser::structVarDeclaration(ASTModifiers modifiers) {
+std::unique_ptr<ASTVarDeclarations> Parser::structVarDeclaration(ASTModifiers modifiers) {
     std::unique_ptr<ASTType> type = this->structDeclaration();
     if (!type) {
         return nullptr;
     }
     if (peek().fKind == Token::IDENTIFIER) {
         Token name = this->nextToken();
-        std::unique_ptr<ASTVarDeclaration> result = this->varDeclarationEnd(modifiers, 
-                                                                            std::move(type), 
-                                                                            std::move(name.fText));
+        std::unique_ptr<ASTVarDeclarations> result = this->varDeclarationEnd(modifiers, 
+                                                                             std::move(type), 
+                                                                             std::move(name.fText));
         if (result) {
-            for (size_t i = 0; i < result->fValues.size(); i++) {
-                if (result->fValues[i]) {
-                    this->error(result->fValues[i]->fPosition, 
+            for (const auto& var : result->fVars) {
+                if (var.fValue) {
+                    this->error(var.fValue->fPosition, 
                                 "struct variables cannot be initialized");
                 }
             }
         }
         return result;
     }
     this->expect(Token::SEMICOLON, "';'");
     return nullptr;
 }
 
 /* (LBRACKET expression? RBRACKET)* (EQ expression)? (COMMA IDENTIFER 
    (LBRACKET expression? RBRACKET)* (EQ expression)?)* SEMICOLON */
-std::unique_ptr<ASTVarDeclaration> Parser::varDeclarationEnd(ASTModifiers mods,
-                                                             std::unique_ptr<ASTType> type,
-                                                             std::string name) {
-    std::vector<std::string> names;
-    std::vector<std::vector<std::unique_ptr<ASTExpression>>> sizes;
-    names.push_back(name);
+std::unique_ptr<ASTVarDeclarations> Parser::varDeclarationEnd(ASTModifiers mods,
+                                                              std::unique_ptr<ASTType> type,
+                                                              std::string name) {
+    std::vector<ASTVarDeclaration> vars;
     std::vector<std::unique_ptr<ASTExpression>> currentVarSizes;
     while (this->peek().fKind == Token::LBRACKET) {
         this->nextToken();
         if (this->peek().fKind == Token::RBRACKET) {
             this->nextToken();
-            currentVarSizes.push_back(nullptr);
+            //currentVarSizes->push_back(nullptr);

[dogben, 2016/09/07 17:35:42]

Why is this commented out?

[ethannicholas, 2016/09/07 19:46:38]

That was an accident, oops.

         } else {
             std::unique_ptr<ASTExpression> size(this->expression());
             if (!size) {
                 return nullptr;
             }
             currentVarSizes.push_back(std::move(size));
             if (!this->expect(Token::RBRACKET, "']'")) {
                 return nullptr;
             }
         }
     }
-    sizes.push_back(std::move(currentVarSizes));
-    std::vector<std::unique_ptr<ASTExpression>> values;
+    std::unique_ptr<ASTExpression> value;
     if (this->peek().fKind == Token::EQ) {
         this->nextToken();
-        std::unique_ptr<ASTExpression> value(this->expression());
+        value = this->expression();
         if (!value) {
             return nullptr;
         }
-        values.push_back(std::move(value));
-    } else {
-        values.push_back(nullptr);
     }
+    vars.emplace_back(std::move(name), std::move(currentVarSizes), std::move(value));
     while (this->peek().fKind == Token::COMMA) {
         this->nextToken();
         Token name;
         if (!this->expect(Token::IDENTIFIER, "an identifier", &name)) {
             return nullptr;
         }
-        names.push_back(name.fText);
         currentVarSizes.clear();
         while (this->peek().fKind == Token::LBRACKET) {
             this->nextToken();
             if (this->peek().fKind == Token::RBRACKET) {
                 this->nextToken();
                 currentVarSizes.push_back(nullptr);
             } else {
                 std::unique_ptr<ASTExpression> size(this->expression());
                 if (!size) {
                     return nullptr;
                 }
                 currentVarSizes.push_back(std::move(size));
                 if (!this->expect(Token::RBRACKET, "']'")) {
                     return nullptr;
                 }
             }
         }
-        sizes.push_back(std::move(currentVarSizes));
         if (this->peek().fKind == Token::EQ) {
             this->nextToken();
-            std::unique_ptr<ASTExpression> value(this->expression());
+            value = this->expression();
             if (!value) {
                 return nullptr;
             }
-            values.push_back(std::move(value));
         } else {
-            values.push_back(nullptr);
+            value.release();

[dogben, 2016/09/07 17:35:42]

1. This should be reset, not release.
2. nit: move below currentVarSizes.clear() (or move both to the top of the loop)

[ethannicholas, 2016/09/07 19:46:38]

Done.

         }
+        vars.emplace_back(std::move(name.fText), std::move(currentVarSizes), std::move(value));
     }
     if (!this->expect(Token::SEMICOLON, "';'")) {
         return nullptr;
     }
-    return std::unique_ptr<ASTVarDeclaration>(new ASTVarDeclaration(std::move(mods),
-                                                                    std::move(type),
-                                                                    std::move(names),
-                                                                    std::move(sizes),
-                                                                    std::move(values)));
+    return std::unique_ptr<ASTVarDeclarations>(new ASTVarDeclarations(std::move(mods),
+                                                                      std::move(type),
+                                                                      std::move(vars)));
 }
 
 /* modifiers type IDENTIFIER (LBRACKET INT_LITERAL RBRACKET)? */
 std::unique_ptr<ASTParameter> Parser::parameter() {
     ASTModifiers modifiers = this->modifiersWithDefaults(ASTModifiers::kIn_Flag);
     std::unique_ptr<ASTType> type = this->type();
     if (!type) {
         return nullptr;
     }
     Token name;
@@ skipping 158 matching lines @@
         case Token::LBRACE:
             return this->block();
         case Token::SEMICOLON:
             this->nextToken(); 
             return std::unique_ptr<ASTStatement>(new ASTBlock(start.fPosition, 
                                                      std::vector<std::unique_ptr<ASTStatement>>()));
         case Token::CONST:   // fall through
         case Token::HIGHP:   // fall through
         case Token::MEDIUMP: // fall through
         case Token::LOWP: {
-            auto decl = this->varDeclaration();
+            auto decl = this->varDeclarations();
             if (!decl) {
                 return nullptr;
             }
             return std::unique_ptr<ASTStatement>(new ASTVarDeclarationStatement(std::move(decl)));
         }
         case Token::IDENTIFIER:
             if (this->isType(start.fText)) {
-                auto decl = this->varDeclaration();
+                auto decl = this->varDeclarations();
                 if (!decl) {
                     return nullptr;
                 }
                 return std::unique_ptr<ASTStatement>(new ASTVarDeclarationStatement(
                                                                                   std::move(decl)));
             }
             // fall through
         default:
             return this->expressionStatement();
     }    
@@ skipping 20 matching lines @@
         return nullptr;
     }
     if (peek().fKind != Token::LBRACE) {
         // we only get into interfaceBlock if we found a top-level identifier which was not a type.
         // 99% of the time, the user was not actually intending to create an interface block, so 
         // it's better to report it as an unknown type
         this->error(name.fPosition, "no type named '" + name.fText + "'");
         return nullptr;
     }
     this->nextToken();
-    std::vector<std::unique_ptr<ASTVarDeclaration>> decls; 
+    std::vector<std::unique_ptr<ASTVarDeclarations>> decls; 
     while (this->peek().fKind != Token::RBRACE) {
-        std::unique_ptr<ASTVarDeclaration> decl = this->varDeclaration();
+        std::unique_ptr<ASTVarDeclarations> decl = this->varDeclarations();
         if (!decl) {
             return nullptr;
         }
         decls.push_back(std::move(decl));
     }
     this->nextToken();
     std::string valueName;
     if (this->peek().fKind == Token::IDENTIFIER) {
         valueName = this->nextToken().fText;
     }
@@ skipping 102 matching lines @@
     if (!this->expect(Token::LPAREN, "'('")) {
         return nullptr;
     }
     std::unique_ptr<ASTStatement> initializer;
     Token nextToken = this->peek();
     switch (nextToken.fKind) {
         case Token::SEMICOLON: 
             break;
         case Token::CONST:
             initializer = std::unique_ptr<ASTStatement>(new ASTVarDeclarationStatement(
-                                                                           this->varDeclaration()));
+                                                                          this->varDeclarations()));
             break;
         case Token::IDENTIFIER: 
             if (this->isType(nextToken.fText)) {
                 initializer = std::unique_ptr<ASTStatement>(new ASTVarDeclarationStatement(
-                                                                           this->varDeclaration()));
+                                                                          this->varDeclarations()));
                 break;
             }
             // fall through
         default:
             initializer = this->expressionStatement();
     }
     std::unique_ptr<ASTExpression> test;
     if (this->peek().fKind != Token::SEMICOLON) {
         test = this->expression();
         if (!test) {
@@ skipping 601 matching lines @@
 bool Parser::identifier(std::string* dest) {
     Token t;
     if (this->expect(Token::IDENTIFIER, "identifier", &t)) {
         *dest = t.fText; 
         return true;
     }
     return false;
 }
 
 } // namespace