Use accessors to refer to the ModifiersPool.

This is a first step towards moving the ModifiersPool out of the
IRGenerator and into the Context.

Change-Id: I8000923f32a9e3cc1878a09464680884b950e513
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404216
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

src/sksl/SkSLCompiler.cpp

@@ -185,7 +185,7 @@ Compiler::Compiler(const ShaderCapsClass* caps)
     // treat it as builtin (ie, no need to clone it into the Program).
     fPrivateSymbolTable->add(
             std::make_unique<Variable>(/*offset=*/-1,
-                                       fIRGenerator->fModifiers->addToPool(Modifiers()),
+                                       fIRGenerator->modifiersPool().add(Modifiers()),
                                        "sk_Caps",
                                        fContext->fTypes.fSkCaps.get(),
                                        /*builtin=*/false,
@@ -339,7 +339,7 @@ LoadedModule Compiler::loadModule(ProgramKind kind,
     fModifiers.push_back(std::move(ir.fModifiers));
 #else
     SkASSERT(data.fData && (data.fSize != 0));
-    Rehydrator rehydrator(fContext.get(), fIRGenerator->fModifiers.get(), base, this,
+    Rehydrator rehydrator(fContext.get(), fIRGenerator->fModifiers.get(), base,
                           data.fData, data.fSize);
     LoadedModule module = { kind, rehydrator.symbolTable(), rehydrator.elements() };
     fModifiers.push_back(fIRGenerator->releaseModifiers());

src/sksl/SkSLIRGenerator.cpp

@@ -81,7 +81,7 @@ public:
 
 IRGenerator::IRGenerator(const Context* context)
         : fContext(*context)
-        , fModifiers(new ModifiersPool()) {}
+        , fModifiers(std::make_unique<ModifiersPool>()) {}
 
 void IRGenerator::pushSymbolTable() {
     auto childSymTable = std::make_shared<SymbolTable>(std::move(fSymbolTable), fIsBuiltinCode);
@@ -358,8 +358,8 @@ std::unique_ptr<Variable> IRGenerator::convertVar(int offset, const Modifiers& m
         }
         type = fSymbolTable->addArrayDimension(type, arraySizeValue);
     }
-    return std::make_unique<Variable>(offset, fModifiers->addToPool(modifiers), name, type,
+    return std::make_unique<Variable>(offset, this->modifiersPool().add(modifiers), name,
-                                      fIsBuiltinCode, storage);
+                                      type, fIsBuiltinCode, storage);
 }
 
 std::unique_ptr<Statement> IRGenerator::convertVarDeclaration(std::unique_ptr<Variable> var,
@@ -489,7 +489,7 @@ std::unique_ptr<ModifiersDeclaration> IRGenerator::convertModifiersDeclaration(c
         !this->caps().gsInvocationsSupport()) {
         modifiers.fLayout.fMaxVertices *= fInvocations;
     }
-    return std::make_unique<ModifiersDeclaration>(fModifiers->addToPool(modifiers));
+    return std::make_unique<ModifiersDeclaration>(this->modifiersPool().add(modifiers));
 }
 
 std::unique_ptr<Statement> IRGenerator::convertIf(const ASTNode& n) {
@@ -687,7 +687,7 @@ std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<
     Modifiers invokeModifiers(invokeLayout, Modifiers::kHasSideEffects_Flag);
     const FunctionDeclaration* invokeDecl = fSymbolTable->add(std::make_unique<FunctionDeclaration>(
             /*offset=*/-1,
-            fModifiers->addToPool(invokeModifiers),
+            this->modifiersPool().add(invokeModifiers),
             "_invoke",
             std::vector<const Variable*>(),
             fContext.fTypes.fVoid.get(),
@@ -1056,8 +1056,8 @@ void IRGenerator::convertFunction(const ASTNode& f) {
         }
 
         const Variable* var = fSymbolTable->takeOwnershipOfSymbol(
-                std::make_unique<Variable>(param.fOffset, fModifiers->addToPool(m), pd.fName, type,
+                std::make_unique<Variable>(param.fOffset, this->modifiersPool().add(m), pd.fName,
-                                           fIsBuiltinCode, Variable::Storage::kParameter));
+                                           type, fIsBuiltinCode, Variable::Storage::kParameter));
         parameters.push_back(var);
     }
 
@@ -1177,7 +1177,7 @@ void IRGenerator::convertFunction(const ASTNode& f) {
                 if (match) {
                     if (*returnType != other->returnType()) {
                         FunctionDeclaration newDecl(f.fOffset,
-                                                    fModifiers->addToPool(funcData.fModifiers),
+                                                    this->modifiersPool().add(funcData.fModifiers),
                                                     funcData.fName,
                                                     parameters,
                                                     returnType,
@@ -1224,7 +1224,7 @@ void IRGenerator::convertFunction(const ASTNode& f) {
         // Create a new declaration.
         decl = fSymbolTable->add(
                 std::make_unique<FunctionDeclaration>(f.fOffset,
-                                                      fModifiers->addToPool(declModifiers),
+                                                      this->modifiersPool().add(declModifiers),
                                                       funcData.fName,
                                                       parameters,
                                                       returnType,
@@ -1344,7 +1344,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTNode
     }
     const Variable* var = old->takeOwnershipOfSymbol(
             std::make_unique<Variable>(intf.fOffset,
-                                       fModifiers->addToPool(id.fModifiers),
+                                       this->modifiersPool().add(id.fModifiers),
                                        id.fInstanceName.fLength ? id.fInstanceName : id.fTypeName,
                                        type,
                                        fIsBuiltinCode,
@@ -1415,7 +1415,7 @@ void IRGenerator::convertEnum(const ASTNode& e) {
         }
         value = IntLiteral::Make(fContext, e.fOffset, currentValue);
         ++currentValue;
-        auto var = std::make_unique<Variable>(e.fOffset, fModifiers->addToPool(modifiers),
+        auto var = std::make_unique<Variable>(e.fOffset, this->modifiersPool().add(modifiers),
                                               child.getString(), type, fIsBuiltinCode,
                                               Variable::Storage::kGlobal);
         // enum variables aren't really 'declared', but we have to create a declaration to store
@@ -2121,7 +2121,7 @@ void IRGenerator::start(const ParsedModule& base,
             m.fFlags = Modifiers::kIn_Flag;
             m.fLayout.fBuiltin = SK_INVOCATIONID_BUILTIN;
         }
-        auto var = std::make_unique<Variable>(/*offset=*/-1, fModifiers->addToPool(m),
+        auto var = std::make_unique<Variable>(/*offset=*/-1, this->modifiersPool().add(m),
                                               "sk_InvocationID", fContext.fTypes.fInt.get(),
                                               /*builtin=*/false, Variable::Storage::kGlobal);
         auto decl = VarDeclaration::Make(fContext, var.get(), fContext.fTypes.fInt.get(),
@@ -2171,10 +2171,11 @@ IRGenerator::IRBundle IRGenerator::finish() {
         }
     }
 
-    IRBundle result{std::move(*fProgramElements), std::move(*fSharedElements),
+    return IRBundle{std::move(*fProgramElements),
-                    this->releaseModifiers(), fSymbolTable, fInputs};
+                    std::move(*fSharedElements),
-    fSymbolTable = nullptr;
+                    this->releaseModifiers(),
-    return result;
+                    std::move(fSymbolTable),
+                    fInputs};
 }
 
 IRGenerator::IRBundle IRGenerator::convertProgram(

src/sksl/SkSLIRGenerator.h

@@ -257,6 +257,10 @@ private:
         return fContext.fCaps;
     }
 
+    ModifiersPool& modifiersPool() const {
+        return *fModifiers;
+    }
+
     Program::Inputs fInputs;
 
     std::unique_ptr<ASTFile> fFile;

src/sksl/SkSLInliner.cpp

@@ -589,7 +589,7 @@ Inliner::InlineVariable Inliner::makeInlineVariable(const String& baseName,
     // Create our new variable and add it to the symbol table.
     InlineVariable result;
     auto var = std::make_unique<Variable>(/*offset=*/-1,
-                                          fModifiers->addToPool(Modifiers()),
+                                          this->modifiersPool().add(Modifiers{}),
                                           name->c_str(),
                                           type,
                                           isBuiltinCode,

src/sksl/SkSLInliner.h

@@ -114,6 +114,8 @@ private:
     /** Checks whether inlining is viable for a FunctionCall, modulo recursion and function size. */
     bool isSafeToInline(const FunctionDefinition* functionDef);
 
+    ModifiersPool& modifiersPool() const { return *fModifiers; }
+
     const Context* fContext = nullptr;
     ModifiersPool* fModifiers = nullptr;
     Mangler fMangler;

src/sksl/SkSLModifiersPool.h

@@ -20,7 +20,7 @@ namespace SkSL {
  */
 class ModifiersPool {
 public:
-    const Modifiers* addToPool(const Modifiers& modifiers) {
+    const Modifiers* add(const Modifiers& modifiers) {
         auto [iter, wasInserted] = fModifiersSet.insert(modifiers);
         return &*iter;
     }

src/sksl/SkSLRehydrator.cpp

@@ -78,11 +78,10 @@ private:
 };
 
 Rehydrator::Rehydrator(const Context* context, ModifiersPool* modifiers,
-                       std::shared_ptr<SymbolTable> symbolTable, ErrorReporter* errorReporter,
+                       std::shared_ptr<SymbolTable> symbolTable,
                        const uint8_t* src, size_t length)
                 : fContext(*context)
                 , fModifiers(*modifiers)
-                , fErrors(errorReporter)
                 , fSymbolTable(std::move(symbolTable))
                 , fStart(src)
     SkDEBUGCODE(, fEnd(fStart + length)) {
@@ -184,8 +183,12 @@ const Symbol* Rehydrator::symbol() {
             const Type* returnType = this->type();
             const FunctionDeclaration* result =
                     fSymbolTable->takeOwnershipOfSymbol(std::make_unique<FunctionDeclaration>(
-                                              /*offset=*/-1, fModifiers.addToPool(modifiers), name,
+                            /*offset=*/-1,
-                                              std::move(parameters), returnType, /*builtin=*/true));
+                            this->modifiersPool().add(modifiers),
+                            name,
+                            std::move(parameters),
+                            returnType,
+                            /*builtin=*/true));
             this->addSymbol(id, result);
             return result;
         }
@@ -252,7 +255,7 @@ const Symbol* Rehydrator::symbol() {
         }
         case kVariable_Command: {
             uint16_t id = this->readU16();
-            const Modifiers* m = fModifiers.addToPool(this->modifiers());
+            const Modifiers* m = this->modifiersPool().add(this->modifiers());
             StringFragment name = this->readString();
             const Type* type = this->type();
             Variable::Storage storage = (Variable::Storage) this->readU8();
@@ -585,7 +588,7 @@ std::shared_ptr<SymbolTable> Rehydrator::symbolTable(bool inherit) {
     std::shared_ptr<SymbolTable> oldTable = fSymbolTable;
     std::shared_ptr<SymbolTable> result =
             inherit ? std::make_shared<SymbolTable>(fSymbolTable, /*builtin=*/true)
-                    : std::make_shared<SymbolTable>(fErrors, /*builtin=*/true);
+                    : std::make_shared<SymbolTable>(this->errorReporter(), /*builtin=*/true);
     fSymbolTable = result;
     std::vector<const Symbol*> ownedSymbols;
     ownedSymbols.reserve(ownedCount);

src/sksl/SkSLRehydrator.h

@@ -11,6 +11,7 @@
 #include "include/private/SkSLDefines.h"
 #include "include/private/SkSLModifiers.h"
 #include "include/private/SkSLSymbol.h"
+#include "src/sksl/SkSLContext.h"
 
 #include <vector>
 
@@ -151,7 +152,7 @@ public:
 
     // src must remain in memory as long as the objects created from it do
     Rehydrator(const Context* context, ModifiersPool* modifiers,
-               std::shared_ptr<SymbolTable> symbolTable, ErrorReporter* errorReporter,
+               std::shared_ptr<SymbolTable> symbolTable,
                const uint8_t* src, size_t length);
 
     std::vector<std::unique_ptr<ProgramElement>> elements();
@@ -227,9 +228,12 @@ private:
 
     const Type* type();
 
+    ErrorReporter* errorReporter() { return &fContext.fErrors; }
+
+    ModifiersPool& modifiersPool() const { return fModifiers; }
+
     const Context& fContext;
     ModifiersPool& fModifiers;
-    ErrorReporter* fErrors;
     std::shared_ptr<SymbolTable> fSymbolTable;
     std::vector<const Symbol*> fSymbols;
 

src/sksl/codegen/SkSLSPIRVCodeGenerator.cpp

@@ -2078,7 +2078,7 @@ SpvId SPIRVCodeGenerator::writeVariableReference(const VariableReference& ref, O
                         Modifiers::kUniform_Flag);
                 const Variable* intfVar = fSynthetics.takeOwnershipOfSymbol(
                         std::make_unique<Variable>(/*offset=*/-1,
-                                                   fProgram.fModifiers->addToPool(modifiers),
+                                                   fProgram.fModifiers->add(modifiers),
                                                    name,
                                                    intfStruct.get(),
                                                    /*builtin=*/false,
@@ -3494,7 +3494,7 @@ void SPIRVCodeGenerator::writeUniformBuffer(std::shared_ptr<SymbolTable> topLeve
     Modifiers modifiers{layout, Modifiers::kUniform_Flag};
 
     fUniformBuffer.fInnerVariable = std::make_unique<Variable>(
-            /*offset=*/-1, fProgram.fModifiers->addToPool(modifiers), kUniformBufferName,
+            /*offset=*/-1, fProgram.fModifiers->add(modifiers), kUniformBufferName,
             fUniformBuffer.fStruct.get(), /*builtin=*/false, Variable::Storage::kGlobal);
 
     // Create an interface block object for this global variable.

src/sksl/dsl/DSLVar.cpp

@@ -37,7 +37,7 @@ DSLVar::DSLVar(const char* name)
         fVar = DSLWriter::SymbolTable()->takeOwnershipOfIRNode(
                 std::make_unique<SkSL::Variable>(
                         /*offset=*/-1,
-                        DSLWriter::IRGenerator().fModifiers->addToPool(
+                        DSLWriter::IRGenerator().modifiersPool().add(
                                 SkSL::Modifiers(
                                         SkSL::Layout(/*flags=*/0, /*location=*/-1, /*offset=*/-1,
                                                      /*binding=*/-1, /*index=*/-1, /*set=*/-1,

src/sksl/dsl/priv/DSLWriter.cpp

@@ -79,8 +79,8 @@ void DSLWriter::Reset() {
     ProgramElements().clear();
 }
 
-const SkSL::Modifiers* DSLWriter::Modifiers(SkSL::Modifiers modifiers) {
+const SkSL::Modifiers* DSLWriter::Modifiers(const SkSL::Modifiers& modifiers) {
-    return IRGenerator().fModifiers->addToPool(modifiers);
+    return IRGenerator().modifiersPool().add(modifiers);
 }
 
 const char* DSLWriter::Name(const char* name) {

src/sksl/dsl/priv/DSLWriter.h

@@ -83,7 +83,7 @@ public:
      * Returns the final pointer to a pooled Modifiers object that should be used to represent the
      * given modifiers.
      */
-    static const SkSL::Modifiers* Modifiers(SkSL::Modifiers modifiers);
+    static const SkSL::Modifiers* Modifiers(const SkSL::Modifiers& modifiers);
 
     /**
      * Returns the SkSL variable corresponding to a DSLVar.