Enable warning for non-virtual destructors

BUILD.gn

@@ -104,6 +104,7 @@ config("extra_warnings") {
       # Remove when crbug.com/428099 is resolved.
       "-Winconsistent-missing-override",
       "-Wunneeded-internal-declaration",
+      "-Wnon-virtual-dtor",
     ]
   }
 }

src/compiler/translator/Symbol.cpp

@@ -33,11 +33,13 @@ static const char kFunctionMangledNameSeparator = '(';
 TSymbol::TSymbol(TSymbolTable *symbolTable,
                  const ImmutableString &name,
                  SymbolType symbolType,
+                 SymbolClass symbolClass,
                  TExtension extension)
     : mName(name),
       mUniqueId(symbolTable->nextUniqueId()),
       mSymbolType(symbolType),
-      mExtension(extension)
+      mExtension(extension),
+      mSymbolClass(symbolClass)
 {
     ASSERT(mSymbolType == SymbolType::BuiltIn || mExtension == TExtension::UNDEFINED);
     ASSERT(mName != "" || mSymbolType == SymbolType::AngleInternal ||
@@ -62,6 +64,12 @@ ImmutableString TSymbol::name() const
 
 ImmutableString TSymbol::getMangledName() const
 {
+    if (mSymbolClass == SymbolClass::Function)
+    {
+        // We do this instead of using proper virtual functions so that we can better support
+        // constexpr symbols.
+        return static_cast<const TFunction *>(this)->getFunctionMangledName();
+    }
     ASSERT(mSymbolType != SymbolType::Empty);
     return name();
 }
@@ -71,7 +79,9 @@ TVariable::TVariable(TSymbolTable *symbolTable,
                      const TType *type,
                      SymbolType symbolType,
                      TExtension extension)
-    : TSymbol(symbolTable, name, symbolType, extension), mType(type), unionArray(nullptr)
+    : TSymbol(symbolTable, name, symbolType, SymbolClass::Variable, extension),
+      mType(type),
+      unionArray(nullptr)
 {
     ASSERT(mType);
 }
@@ -80,7 +90,7 @@ TStructure::TStructure(TSymbolTable *symbolTable,
                        const ImmutableString &name,
                        const TFieldList *fields,
                        SymbolType symbolType)
-    : TSymbol(symbolTable, name, symbolType), TFieldListCollection(fields)
+    : TSymbol(symbolTable, name, symbolType, SymbolClass::Struct), TFieldListCollection(fields)
 {
 }
 
@@ -88,7 +98,8 @@ TStructure::TStructure(const TSymbolUniqueId &id,
                        const ImmutableString &name,
                        TExtension extension,
                        const TFieldList *fields)
-    : TSymbol(id, name, SymbolType::BuiltIn, extension), TFieldListCollection(fields)
+    : TSymbol(id, name, SymbolType::BuiltIn, extension, SymbolClass::Struct),
+      TFieldListCollection(fields)
 {
 }
 
@@ -126,7 +137,7 @@ TInterfaceBlock::TInterfaceBlock(TSymbolTable *symbolTable,
                                  const TLayoutQualifier &layoutQualifier,
                                  SymbolType symbolType,
                                  TExtension extension)
-    : TSymbol(symbolTable, name, symbolType, extension),
+    : TSymbol(symbolTable, name, symbolType, SymbolClass::InterfaceBlock, extension),
       TFieldListCollection(fields),
       mBlockStorage(layoutQualifier.blockStorage),
       mBinding(layoutQualifier.binding)
@@ -138,7 +149,7 @@ TInterfaceBlock::TInterfaceBlock(const TSymbolUniqueId &id,
                                  const ImmutableString &name,
                                  TExtension extension,
                                  const TFieldList *fields)
-    : TSymbol(id, name, SymbolType::BuiltIn, extension),
+    : TSymbol(id, name, SymbolType::BuiltIn, extension, SymbolClass::InterfaceBlock),
       TFieldListCollection(fields),
       mBlockStorage(EbsUnspecified),
       mBinding(0)
@@ -150,7 +161,7 @@ TFunction::TFunction(TSymbolTable *symbolTable,
                      SymbolType symbolType,
                      const TType *retType,
                      bool knownToNotHaveSideEffects)
-    : TSymbol(symbolTable, name, symbolType, TExtension::UNDEFINED),
+    : TSymbol(symbolTable, name, symbolType, SymbolClass::Function, TExtension::UNDEFINED),
       mParametersVector(new TParamVector()),
       mParameters(nullptr),
       mParamCount(0u),

src/compiler/translator/Symbol.h

@@ -28,6 +28,14 @@ enum class SymbolType
     Empty  // Meaning symbol without a name.
 };
 
+enum class SymbolClass
+{
+    Function,
+    Variable,
+    Struct,
+    InterfaceBlock
+};
+
 // Symbol base class. (Can build functions or variables out of these...)
 class TSymbol : angle::NonCopyable
 {
@@ -36,9 +44,10 @@ class TSymbol : angle::NonCopyable
     TSymbol(TSymbolTable *symbolTable,
             const ImmutableString &name,
             SymbolType symbolType,
+            SymbolClass symbolClass,
             TExtension extension = TExtension::UNDEFINED);
 
-    // Note that we don't have a virtual destructor in order to support constexpr symbols. Data is
+    // Note that we can't have a virtual destructor in order to support constexpr symbols. Data is
     // either statically allocated or pool allocated.
     ~TSymbol() = default;
 
@@ -46,11 +55,11 @@ class TSymbol : angle::NonCopyable
     // as for internal variables.
     ImmutableString name() const;
     // Don't call getMangledName() for empty symbols (symbolType == SymbolType::Empty).
-    virtual ImmutableString getMangledName() const;
+    ImmutableString getMangledName() const;
 
-    virtual bool isFunction() const { return false; }
-    virtual bool isVariable() const { return false; }
-    virtual bool isStruct() const { return false; }
+    bool isFunction() const { return mSymbolClass == SymbolClass::Function; }
+    bool isVariable() const { return mSymbolClass == SymbolClass::Variable; }
+    bool isStruct() const { return mSymbolClass == SymbolClass::Struct; }
 
     const TSymbolUniqueId &uniqueId() const { return mUniqueId; }
     SymbolType symbolType() const { return mSymbolType; }
@@ -60,8 +69,13 @@ class TSymbol : angle::NonCopyable
     constexpr TSymbol(const TSymbolUniqueId &id,
                       const ImmutableString &name,
                       SymbolType symbolType,
-                      TExtension extension)
-        : mName(name), mUniqueId(id), mSymbolType(symbolType), mExtension(extension)
+                      TExtension extension,
+                      SymbolClass symbolClass)
+        : mName(name),
+          mUniqueId(id),
+          mSymbolType(symbolType),
+          mExtension(extension),
+          mSymbolClass(symbolClass)
     {
     }
 
@@ -71,6 +85,10 @@ class TSymbol : angle::NonCopyable
     const TSymbolUniqueId mUniqueId;
     const SymbolType mSymbolType;
     const TExtension mExtension;
+
+    // We use this instead of having virtual functions for querying the class in order to support
+    // constexpr symbols.
+    const SymbolClass mSymbolClass;
 };
 
 // Variable.
@@ -84,7 +102,6 @@ class TVariable : public TSymbol
               SymbolType symbolType,
               TExtension ext = TExtension::UNDEFINED);
 
-    bool isVariable() const override { return true; }
     const TType &getType() const { return *mType; }
 
     const TConstantUnion *getConstPointer() const { return unionArray; }
@@ -97,7 +114,9 @@ class TVariable : public TSymbol
                         SymbolType symbolType,
                         TExtension extension,
                         const TType *type)
-        : TSymbol(id, name, symbolType, extension), mType(type), unionArray(nullptr)
+        : TSymbol(id, name, symbolType, extension, SymbolClass::Variable),
+          mType(type),
+          unionArray(nullptr)
     {
     }
 
@@ -115,8 +134,6 @@ class TStructure : public TSymbol, public TFieldListCollection
                const TFieldList *fields,
                SymbolType symbolType);
 
-    bool isStruct() const override { return true; }
-
     // The char arrays passed in must be pool allocated or static.
     void createSamplerSymbols(const char *namePrefix,
                               const TString &apiNamePrefix,
@@ -204,12 +221,10 @@ class TFunction : public TSymbol
               const TType *retType,
               bool knownToNotHaveSideEffects);
 
-    bool isFunction() const override { return true; }
-
     void addParameter(const TVariable *p);
     void shareParameters(const TFunction &parametersSource);
 
-    ImmutableString getMangledName() const override
+    ImmutableString getFunctionMangledName() const
     {
         ASSERT(symbolType() != SymbolType::BuiltIn);
         if (mMangledName.empty())
@@ -245,7 +260,7 @@ class TFunction : public TSymbol
                         const TType *retType,
                         TOperator op,
                         bool knownToNotHaveSideEffects)
-        : TSymbol(id, name, SymbolType::BuiltIn, extension),
+        : TSymbol(id, name, SymbolType::BuiltIn, extension, SymbolClass::Function),
           mParametersVector(nullptr),
           mParameters(parameters),
           mParamCount(paramCount),