Add as<SymbolSubclass> to downcast Symbols more safely.

johnstiles-google · Skia Commit-Bot · commit 17c5b7024297 · 2020-08-19T14:21:40.000Z
The as<T>() function asserts that the Symbol is of the correct kind before performing the downcast, and is also generally easier to read as function calls flow naturally from left-to-right, and C-style casts don't. This CL updates several Symbol downcasts in SkSL to the as<T> syntax, but is not intended to exhaustively replace them all (although that would be ideal). In places where we SkASSERTed the expression's fKind immediately before a cast, the assert has been removed because it would be redundant with the behavior of as<T>(). Change-Id: I33cb2b9657dc410241dbc96be85c4ce182a1b391 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311436 Commit-Queue: John Stiles <johnstiles@google.com> Commit-Queue: Brian Osman <brianosman@google.com> Auto-Submit: John Stiles <johnstiles@google.com> Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/SkSLByteCodeGenerator.cpp b/src/sksl/SkSLByteCodeGenerator.cpp
@@ -1602,7 +1602,7 @@ class ByteCodeExpressionLValue : public ByteCodeGenerator::LValue {
 std::unique_ptr<ByteCodeGenerator::LValue> ByteCodeGenerator::getLValue(const Expression& e) {
     switch (e.fKind) {
         case Expression::kExternalValue_Kind: {
-            const ExternalValue* value = ((ExternalValueReference&) e).fValue;
+            const ExternalValue* value = e.as<ExternalValueReference>().fValue;
             int index = fOutput->fExternalValues.size();
             fOutput->fExternalValues.push_back(value);
             SkASSERT(index <= 255);
@@ -1613,7 +1613,7 @@ std::unique_ptr<ByteCodeGenerator::LValue> ByteCodeGenerator::getLValue(const Ex
         case Expression::kVariableReference_Kind:
             return std::unique_ptr<LValue>(new ByteCodeExpressionLValue(this, e));
         case Expression::kSwizzle_Kind: {
-            const Swizzle& s = (const Swizzle&) e;
+            const Swizzle& s = e.as<Swizzle>();
             return swizzle_is_simple(s)
                     ? std::unique_ptr<LValue>(new ByteCodeExpressionLValue(this, e))
                     : std::unique_ptr<LValue>(new ByteCodeSwizzleLValue(this, s));
diff --git a/src/sksl/SkSLCPPCodeGenerator.cpp b/src/sksl/SkSLCPPCodeGenerator.cpp
@@ -86,11 +86,9 @@ void CPPCodeGenerator::writeBinaryExpression(const BinaryExpression& b,
                b.fRight->fKind == Expression::kNullLiteral_Kind) {
         const Variable* var;
         if (b.fLeft->fKind != Expression::kNullLiteral_Kind) {
-            SkASSERT(b.fLeft->fKind == Expression::kVariableReference_Kind);
-            var = &((VariableReference&) *b.fLeft).fVariable;
+            var = &b.fLeft->as<VariableReference>().fVariable;
         } else {
-            SkASSERT(b.fRight->fKind == Expression::kVariableReference_Kind);
-            var = &((VariableReference&) *b.fRight).fVariable;
+            var = &b.fRight->as<VariableReference>().fVariable;
         }
         SkASSERT(var->fType.kind() == Type::kNullable_Kind &&
                  var->fType.componentType() == *fContext.fFragmentProcessor_Type);
diff --git a/src/sksl/SkSLDehydrator.cpp b/src/sksl/SkSLDehydrator.cpp
@@ -142,7 +142,7 @@ void Dehydrator::write(const Symbol& s) {
     }
     switch (s.fKind) {
         case Symbol::kFunctionDeclaration_Kind: {
-            const FunctionDeclaration& f = (const FunctionDeclaration&) s;
+            const FunctionDeclaration& f = s.as<FunctionDeclaration>();
             this->writeU8(Rehydrator::kFunctionDeclaration_Command);
             this->writeId(&f);
             this->write(f.fModifiers);
@@ -155,7 +155,7 @@ void Dehydrator::write(const Symbol& s) {
             break;
         }
         case Symbol::kUnresolvedFunction_Kind: {
-            const UnresolvedFunction& f = (const UnresolvedFunction&) s;
+            const UnresolvedFunction& f = s.as<UnresolvedFunction>();
             this->writeU8(Rehydrator::kUnresolvedFunction_Command);
             this->writeId(&f);
             this->writeU8(f.fFunctions.size());
@@ -165,7 +165,7 @@ void Dehydrator::write(const Symbol& s) {
             break;
         }
         case Symbol::kType_Kind: {
-            const Type& t = (const Type&) s;
+            const Type& t = s.as<Type>();
             switch (t.kind()) {
                 case Type::kArray_Kind:
                     this->writeU8(Rehydrator::kArrayType_Command);
@@ -202,7 +202,7 @@ void Dehydrator::write(const Symbol& s) {
             break;
         }
         case Symbol::kVariable_Kind: {
-            Variable& v = (Variable&) s;
+            const Variable& v = s.as<Variable>();
             this->writeU8(Rehydrator::kVariable_Command);
             this->writeId(&v);
             this->write(v.fModifiers);
@@ -212,7 +212,7 @@ void Dehydrator::write(const Symbol& s) {
             break;
         }
         case Symbol::kField_Kind: {
-            Field& f = (Field&) s;
+            const Field& f = s.as<Field>();
             this->writeU8(Rehydrator::kField_Command);
             this->writeU16(this->symbolId(&f.fOwner));
             this->writeU8(f.fFieldIndex);
diff --git a/src/sksl/SkSLExternalValue.h b/src/sksl/SkSLExternalValue.h
@@ -17,8 +17,10 @@ class Type;
 
 class ExternalValue : public Symbol {
 public:
+    static constexpr Kind kSymbolKind = kExternal_Kind;
+
     ExternalValue(const char* name, const Type& type)
-        : INHERITED(-1, kExternal_Kind, name)
+        : INHERITED(-1, kSymbolKind, name)
         , fType(type) {}
 
     virtual bool canRead() const {
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
@@ -988,10 +988,10 @@ void IRGenerator::convertFunction(const ASTNode& f) {
         std::vector<const FunctionDeclaration*> functions;
         switch (entry->fKind) {
             case Symbol::kUnresolvedFunction_Kind:
-                functions = ((UnresolvedFunction*) entry)->fFunctions;
+                functions = entry->as<UnresolvedFunction>().fFunctions;
                 break;
             case Symbol::kFunctionDeclaration_Kind:
-                functions.push_back((FunctionDeclaration*) entry);
+                functions.push_back(&entry->as<FunctionDeclaration>());
                 break;
             default:
                 fErrors.error(f.fOffset, "symbol '" + fd.fName + "' was already defined");
@@ -1243,13 +1243,13 @@ const Type* IRGenerator::convertType(const ASTNode& type) {
     const Symbol* result = (*fSymbolTable)[td.fName];
     if (result && result->fKind == Symbol::kType_Kind) {
         if (td.fIsNullable) {
-            if (((Type&) *result) == *fContext.fFragmentProcessor_Type) {
+            if (result->as<Type>() == *fContext.fFragmentProcessor_Type) {
                 if (type.begin() != type.end()) {
                     fErrors.error(type.fOffset, "type '" + td.fName + "' may not be used in "
                                                 "an array");
                 }
                 result = fSymbolTable->takeOwnershipOfSymbol(std::make_unique<Type>(
-                        String(result->fName) + "?", Type::kNullable_Kind, (const Type&)*result));
+                        String(result->fName) + "?", Type::kNullable_Kind, result->as<Type>()));
             } else {
                 fErrors.error(type.fOffset, "type '" + td.fName + "' may not be nullable");
             }
@@ -1262,9 +1262,9 @@ const Type* IRGenerator::convertType(const ASTNode& type) {
             }
             name += "]";
             result = fSymbolTable->takeOwnershipOfSymbol(std::make_unique<Type>(
-                    name, Type::kArray_Kind, (const Type&)*result, size ? size.getInt() : 0));
+                    name, Type::kArray_Kind, result->as<Type>(), size ? size.getInt() : 0));
         }
-        return (const Type*) result;
+        return &result->as<Type>();
     }
     fErrors.error(type.fOffset, "unknown type '" + td.fName + "'");
     return nullptr;
@@ -1317,16 +1317,16 @@ std::unique_ptr<Expression> IRGenerator::convertIdentifier(const ASTNode& identi
     switch (result->fKind) {
         case Symbol::kFunctionDeclaration_Kind: {
             std::vector<const FunctionDeclaration*> f = {
-                (const FunctionDeclaration*) result
+                &result->as<FunctionDeclaration>()
             };
             return std::make_unique<FunctionReference>(fContext, identifier.fOffset, f);
         }
         case Symbol::kUnresolvedFunction_Kind: {
-            const UnresolvedFunction* f = (const UnresolvedFunction*) result;
+            const UnresolvedFunction* f = &result->as<UnresolvedFunction>();
             return std::make_unique<FunctionReference>(fContext, identifier.fOffset, f->fFunctions);
         }
         case Symbol::kVariable_Kind: {
-            const Variable* var = (const Variable*) result;
+            const Variable* var = &result->as<Variable>();
             switch (var->fModifiers.fLayout.fBuiltin) {
                 case SK_WIDTH_BUILTIN:
                     fInputs.fRTWidth = true;
@@ -1373,7 +1373,7 @@ std::unique_ptr<Expression> IRGenerator::convertIdentifier(const ASTNode& identi
                                                        VariableReference::kRead_RefKind);
         }
         case Symbol::kField_Kind: {
-            const Field* field = (const Field*) result;
+            const Field* field = &result->as<Field>();
             VariableReference* base = new VariableReference(identifier.fOffset, field->fOwner,
                                                             VariableReference::kRead_RefKind);
             return std::unique_ptr<Expression>(new FieldAccess(
@@ -1382,11 +1382,11 @@ std::unique_ptr<Expression> IRGenerator::convertIdentifier(const ASTNode& identi
                                                   FieldAccess::kAnonymousInterfaceBlock_OwnerKind));
         }
         case Symbol::kType_Kind: {
-            const Type* t = (const Type*) result;
+            const Type* t = &result->as<Type>();
             return std::make_unique<TypeReference>(fContext, identifier.fOffset, *t);
         }
         case Symbol::kExternal_Kind: {
-            const ExternalValue* r = (const ExternalValue*) result;
+            const ExternalValue* r = &result->as<ExternalValue>();
             return std::make_unique<ExternalValueReference>(identifier.fOffset, r);
         }
         default:
@@ -2500,7 +2500,7 @@ std::unique_ptr<Expression> IRGenerator::call(int offset,
                                             ((TypeReference&) *functionValue).fValue,
                                             std::move(arguments));
         case Expression::kExternalValue_Kind: {
-            const ExternalValue* v = ((ExternalValueReference&) *functionValue).fValue;
+            const ExternalValue* v = functionValue->as<ExternalValueReference>().fValue;
             if (!v->canCall()) {
                 fErrors.error(offset, "this external value is not a function");
                 return nullptr;
@@ -2809,7 +2809,7 @@ std::unique_ptr<Expression> IRGenerator::convertIndex(std::unique_ptr<Expression
 std::unique_ptr<Expression> IRGenerator::convertField(std::unique_ptr<Expression> base,
                                                       StringFragment field) {
     if (base->fKind == Expression::kExternalValue_Kind) {
-        const ExternalValue& ev = *((ExternalValueReference&) *base).fValue;
+        const ExternalValue& ev = *base->as<ExternalValueReference>().fValue;
         ExternalValue* result = ev.getChild(String(field).c_str());
         if (!result) {
             fErrors.error(base->fOffset, "external value does not have a child named '" + field +
diff --git a/src/sksl/ir/SkSLEnum.h b/src/sksl/ir/SkSLEnum.h
@@ -42,7 +42,7 @@ struct Enum : public ProgramElement {
         std::sort(sortedSymbols.begin(), sortedSymbols.end(),
                   [](const Symbol* a, const Symbol* b) { return a->fName < b->fName; });
         for (const auto& s : sortedSymbols) {
-            const Expression& initialValue = *((Variable*) s)->fInitialValue;
+            const Expression& initialValue = *s->as<Variable>().fInitialValue;
             result += separator + "    " + s->fName + " = " +
                       to_string(initialValue.as<IntLiteral>().fValue);
             separator = ",\n";
diff --git a/src/sksl/ir/SkSLField.h b/src/sksl/ir/SkSLField.h
@@ -22,8 +22,10 @@ namespace SkSL {
  * result of declaring anonymous interface blocks.
  */
 struct Field : public Symbol {
+    static constexpr Kind kSymbolKind = kField_Kind;
+
     Field(int offset, const Variable& owner, int fieldIndex)
-    : INHERITED(offset, kField_Kind, owner.fType.fields()[fieldIndex].fName)
+    : INHERITED(offset, kSymbolKind, owner.fType.fields()[fieldIndex].fName)
     , fOwner(owner)
     , fFieldIndex(fieldIndex) {}
 
diff --git a/src/sksl/ir/SkSLFunctionDeclaration.h b/src/sksl/ir/SkSLFunctionDeclaration.h
@@ -25,10 +25,12 @@ struct FunctionDefinition;
  * A function declaration (not a definition -- does not contain a body).
  */
 struct FunctionDeclaration : public Symbol {
+    static constexpr Kind kSymbolKind = kFunctionDeclaration_Kind;
+
     FunctionDeclaration(int offset, Modifiers modifiers, StringFragment name,
                         std::vector<const Variable*> parameters, const Type& returnType,
                         bool builtin)
-    : INHERITED(offset, kFunctionDeclaration_Kind, std::move(name))
+    : INHERITED(offset, kSymbolKind, std::move(name))
     , fDefinition(nullptr)
     , fBuiltin(builtin)
     , fModifiers(modifiers)
diff --git a/src/sksl/ir/SkSLSymbol.h b/src/sksl/ir/SkSLSymbol.h
@@ -32,6 +32,21 @@ struct Symbol : public IRNode {
 
     ~Symbol() override {}
 
+    /**
+     *  Use as<T> to downcast symbols. e.g. replace `(Variable&) sym` with `sym.as<Variable>()`.
+     */
+    template <typename T>
+    const T& as() const {
+        SkASSERT(this->fKind == T::kSymbolKind);
+        return static_cast<const T&>(*this);
+    }
+
+    template <typename T>
+    T& as() {
+        SkASSERT(this->fKind == T::kSymbolKind);
+        return static_cast<T&>(*this);
+    }
+
     Kind fKind;
     StringFragment fName;
 
diff --git a/src/sksl/ir/SkSLSymbolTable.cpp b/src/sksl/ir/SkSLSymbolTable.cpp
@@ -13,9 +13,9 @@ namespace SkSL {
 std::vector<const FunctionDeclaration*> SymbolTable::GetFunctions(const Symbol& s) {
     switch (s.fKind) {
         case Symbol::kFunctionDeclaration_Kind:
-            return { &((FunctionDeclaration&) s) };
+            return { &s.as<FunctionDeclaration>() };
         case Symbol::kUnresolvedFunction_Kind:
-            return ((UnresolvedFunction&) s).fFunctions;
+            return s.as<UnresolvedFunction>().fFunctions;
         default:
             return std::vector<const FunctionDeclaration*>();
     }
@@ -74,17 +74,17 @@ void SymbolTable::addWithoutOwnership(StringFragment name, const Symbol* symbol)
         const Symbol* oldSymbol = existing->second;
         if (oldSymbol->fKind == Symbol::kFunctionDeclaration_Kind) {
             std::vector<const FunctionDeclaration*> functions;
-            functions.push_back((const FunctionDeclaration*) oldSymbol);
-            functions.push_back((const FunctionDeclaration*) symbol);
+            functions.push_back(&oldSymbol->as<FunctionDeclaration>());
+            functions.push_back(&symbol->as<FunctionDeclaration>());
             std::unique_ptr<const Symbol> u = std::unique_ptr<const Symbol>(
                                                       new UnresolvedFunction(std::move(functions)));
             fSymbols[name] = this->takeOwnershipOfSymbol(std::move(u));
         } else if (oldSymbol->fKind == Symbol::kUnresolvedFunction_Kind) {
             std::vector<const FunctionDeclaration*> functions;
-            for (const auto* f : ((UnresolvedFunction&) *oldSymbol).fFunctions) {
+            for (const auto* f : oldSymbol->as<UnresolvedFunction>().fFunctions) {
                 functions.push_back(f);
             }
-            functions.push_back((const FunctionDeclaration*) symbol);
+            functions.push_back(&symbol->as<FunctionDeclaration>());
             std::unique_ptr<const Symbol> u = std::unique_ptr<const Symbol>(
                                                       new UnresolvedFunction(std::move(functions)));
             fSymbols[name] = this->takeOwnershipOfSymbol(std::move(u));
diff --git a/src/sksl/ir/SkSLType.h b/src/sksl/ir/SkSLType.h
@@ -26,6 +26,8 @@ class Context;
  */
 class Type : public Symbol {
 public:
+    static constexpr Kind kSymbolKind = kType_Kind;
+
     struct Field {
         Field(Modifiers modifiers, StringFragment name, const Type* type)
         : fModifiers(modifiers)
@@ -66,7 +68,7 @@ class Type : public Symbol {
     // Create an "other" (special) type with the given name. These types cannot be directly
     // referenced from user code.
     Type(const char* name)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kOther_Kind)
     , fNumberKind(kNonnumeric_NumberKind) {
@@ -76,7 +78,7 @@ class Type : public Symbol {
 
     // Create an "other" (special) type that supports field access.
     Type(const char* name, std::vector<Field> fields)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kOther_Kind)
     , fNumberKind(kNonnumeric_NumberKind)
@@ -87,7 +89,7 @@ class Type : public Symbol {
 
     // Create a simple type.
     Type(String name, Kind kind)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(std::move(name))
     , fTypeKind(kind)
     , fNumberKind(kNonnumeric_NumberKind) {
@@ -97,7 +99,7 @@ class Type : public Symbol {
 
     // Create a generic type which maps to the listed types.
     Type(const char* name, std::vector<const Type*> types)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kGeneric_Kind)
     , fNumberKind(kNonnumeric_NumberKind)
@@ -108,7 +110,7 @@ class Type : public Symbol {
 
     // Create a struct type with the given fields.
     Type(int offset, String name, std::vector<Field> fields)
-    : INHERITED(offset, kType_Kind, StringFragment())
+    : INHERITED(offset, kSymbolKind, StringFragment())
     , fNameString(std::move(name))
     , fTypeKind(kStruct_Kind)
     , fNumberKind(kNonnumeric_NumberKind)
@@ -119,7 +121,7 @@ class Type : public Symbol {
 
     // Create a scalar type.
     Type(const char* name, NumberKind numberKind, int priority, bool highPrecision = false)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kScalar_Kind)
     , fNumberKind(numberKind)
@@ -136,7 +138,7 @@ class Type : public Symbol {
          NumberKind numberKind,
          int priority,
          std::vector<const Type*> coercibleTypes)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kScalar_Kind)
     , fNumberKind(numberKind)
@@ -150,7 +152,7 @@ class Type : public Symbol {
 
     // Create a nullable type.
     Type(String name, Kind kind, const Type& componentType)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(std::move(name))
     , fTypeKind(kind)
     , fNumberKind(kNonnumeric_NumberKind)
@@ -168,7 +170,7 @@ class Type : public Symbol {
 
     // Create a vector or array type.
     Type(String name, Kind kind, const Type& componentType, int columns)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(std::move(name))
     , fTypeKind(kind)
     , fNumberKind(kNonnumeric_NumberKind)
@@ -182,7 +184,7 @@ class Type : public Symbol {
 
     // Create a matrix type.
     Type(const char* name, const Type& componentType, int columns, int rows)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kMatrix_Kind)
     , fNumberKind(kNonnumeric_NumberKind)
@@ -197,7 +199,7 @@ class Type : public Symbol {
     // Create a texture type.
     Type(const char* name, SpvDim_ dimensions, bool isDepth, bool isArrayed, bool isMultisampled,
          bool isSampled)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kTexture_Kind)
     , fNumberKind(kNonnumeric_NumberKind)
@@ -213,7 +215,7 @@ class Type : public Symbol {
 
     // Create a sampler type.
     Type(const char* name, const Type& textureType)
-    : INHERITED(-1, kType_Kind, StringFragment())
+    : INHERITED(-1, kSymbolKind, StringFragment())
     , fNameString(name)
     , fTypeKind(kSampler_Kind)
     , fNumberKind(kNonnumeric_NumberKind)
diff --git a/src/sksl/ir/SkSLUnresolvedFunction.h b/src/sksl/ir/SkSLUnresolvedFunction.h
diff --git a/src/sksl/ir/SkSLVariable.h b/src/sksl/ir/SkSLVariable.h
