Clean up variable declaration class hierarchy (#787)

The AST class hierarchy for variable declarations had a few messy bits.
First, there are more subclasses of `VarDeclBase` than seem strictly necessary; especially for stuff like `struct` member variable which use `StructField` even for `static` fields (which are effectively globals).
Second, the AST node type for the "cases" within an `enum` was made a subclass of `VarDeclBase` for expediency, but this isn't really semantically accurate (and doesn't seem to be paying off much in deduplication of code).

This change tries to address both of those problems.

First, we replace the existing `Variable` and `StructField` cases with a single `VarDecl` case that covers globals, locals, and member variables.
I haven't gone so far as to replace function parameters or generic value parameters, but that might be worth considering as a further clean-up.

Second, we change `EnumCaseDecl` to inherit directly from  `Decl` instead of `VarDeclBase` and add an explicit case for handling them where they were previously handled as if they were variable declarations (this was done by manually surveying all locations in the code that referenced `VarDeclBase`).

8 files changed, 130 insertions, 64 deletions

diff --git a/source/slang/check.cpp b/source/slang/check.cpp
index 9aa5eb689..82936b65d 100644
--- a/source/slang/check.cpp
+++ b/source/slang/check.cpp
@@ -593,7 +593,7 @@ namespace Slang
 
                         auto interfaceDecl = interfaceDeclRef.getDecl();
 
-                        RefPtr<Variable> varDecl = new Variable();
+                        RefPtr<VarDecl> varDecl = new VarDecl();
                         varDecl->ParentDecl = nullptr; // TODO: need to fill this in somehow!
                         varDecl->checkState = DeclCheckState::Checked;
                         varDecl->nameAndLoc.loc = expr->loc;
@@ -1039,7 +1039,8 @@ namespace Slang
             // the scope that was in place *before* the variable was declared, but
             // this is a quick fix that at least alerts the user to how we are
             // interpreting their code.
-            if (auto varDecl = decl.As<Variable>())
+            //
+            if (auto varDecl = decl.As<VarDecl>())
             {
                 if (auto parenScope = varDecl->ParentDecl->As<ScopeDecl>())
                 {
@@ -1678,7 +1679,7 @@ namespace Slang
                     // We will go through the fields in order and try to match them
                     // up with initializer arguments.
                     //
-                    for(auto fieldDeclRef : getMembersOfType<StructField>(toStructDeclRef))
+                    for(auto fieldDeclRef : getMembersOfType<VarDecl>(toStructDeclRef))
                     {
                         RefPtr<Expr> coercedArg;
                         bool argResult = tryReadArgFromInitializerList(
@@ -2806,11 +2807,6 @@ namespace Slang
             }
         }
 
-        void visitStructField(StructField* field)
-        {
-            CheckVarDeclCommon(field);
-        }
-
         bool doesSignatureMatchRequirement(
             DeclRef<CallableDecl>   satisfyingMemberDeclRef,
             DeclRef<CallableDecl>   requiredMemberDeclRef,
@@ -3649,7 +3645,7 @@ namespace Slang
                 IntegerLiteralValue defaultTag = 0;
                 for(auto caseDecl : decl->getMembersOfType<EnumCaseDecl>())
                 {
-                    if(auto explicitTagValExpr = caseDecl->initExpr)
+                    if(auto explicitTagValExpr = caseDecl->tagExpr)
                     {
                         // This tag has an initializer, so it should establish
                         // the tag value for a successor case that doesn't
@@ -3684,7 +3680,7 @@ namespace Slang
                         tagValExpr->type = QualType(tagType);
                         tagValExpr->value = defaultTag;
 
-                        caseDecl->initExpr = tagValExpr;
+                        caseDecl->tagExpr = tagValExpr;
                     }
 
                     // Default tag for the next case will be one more than
@@ -3737,7 +3733,7 @@ namespace Slang
 
                 // Need to check the init expression, if present, since
                 // that represents the explicit tag for this case.
-                if(auto initExpr = decl->initExpr)
+                if(auto initExpr = decl->tagExpr)
                 {
                     initExpr = CheckExpr(initExpr);
                     initExpr = Coerce(tagType, initExpr);
@@ -3747,7 +3743,7 @@ namespace Slang
                     // the value.
                     CheckIntegerConstantExpression(initExpr);
 
-                    decl->initExpr = initExpr;
+                    decl->tagExpr = initExpr;
                 }
             }
 
@@ -4620,7 +4616,7 @@ namespace Slang
             }
         }
 
-        void visitVariable(Variable* varDecl)
+        void visitVarDecl(VarDecl* varDecl)
         {
             CheckVarDeclCommon(varDecl);
         }
@@ -4895,6 +4891,14 @@ namespace Slang
                     }
 
                 }
+                else if(auto enumRef = declRef.As<EnumCaseDecl>())
+                {
+                    // The cases in an `enum` declaration can also be used as constant expressions,
+                    if(auto tagExpr = getTagExpr(enumRef))
+                    {
+                        return TryConstantFoldExpr(tagExpr.Ptr());
+                    }
+                }
             }
 
             if(auto castExpr = dynamic_cast<TypeCastExpr*>(expr))
@@ -9534,6 +9538,13 @@ namespace Slang
             qualType.IsLeftValue = isLValue;
             return qualType;
         }
+        else if( auto enumCaseDeclRef = declRef.As<EnumCaseDecl>() )
+        {
+            QualType qualType;
+            qualType.type = getType(enumCaseDeclRef);
+            qualType.IsLeftValue = false;
+            return qualType;
+        }
         else if (auto typeAliasDeclRef = declRef.As<TypeDefDecl>())
         {
             auto type = getNamedType(session, typeAliasDeclRef);
diff --git a/source/slang/decl-defs.h b/source/slang/decl-defs.h
index 502412b4b..076db1188 100644
--- a/source/slang/decl-defs.h
+++ b/source/slang/decl-defs.h
@@ -33,7 +33,7 @@ ABSTRACT_SYNTAX_CLASS(ContainerDecl, Decl)
     )
 END_SYNTAX_CLASS()
 
-// Base class for all variable-like declarations
+// Base class for all variable declarations
 ABSTRACT_SYNTAX_CLASS(VarDeclBase, Decl)
 
     // type of the variable
@@ -47,9 +47,9 @@ ABSTRACT_SYNTAX_CLASS(VarDeclBase, Decl)
     SYNTAX_FIELD(RefPtr<Expr>, initExpr)
 END_SYNTAX_CLASS()
 
-
-// A field of a `struct` type
-SIMPLE_SYNTAX_CLASS(StructField, VarDeclBase)
+// Ordinary potentially-mutable variables (locals, globals, and member variables)
+SYNTAX_CLASS(VarDecl, VarDeclBase)
+END_SYNTAX_CLASS()
 
 // An `AggTypeDeclBase` captures the shared functionality
 // between true aggregate type declarations and extension
@@ -76,9 +76,9 @@ ABSTRACT_SYNTAX_CLASS(AggTypeDecl, AggTypeDeclBase)
 RAW(
     // extensions that might apply to this declaration
     ExtensionDecl* candidateExtensions = nullptr;
-    FilteredMemberList<StructField> GetFields()
+    FilteredMemberList<VarDecl> GetFields()
     {
-        return getMembersOfType<StructField>();
+        return getMembersOfType<VarDecl>();
     }
     )
 END_SYNTAX_CLASS()
@@ -98,7 +98,28 @@ RAW(
 )
 END_SYNTAX_CLASS()
 
-SIMPLE_SYNTAX_CLASS(EnumCaseDecl, VarDeclBase)
+// A single case in an enum.
+//
+// E.g., in a declaration like:
+//
+//      enum Color { Red = 0, Green, Blue };
+//
+// The `Red = 0` is the declaration of the `Red`
+// case, with `0` as an explicit expression for its
+// _tag value_.
+//
+SYNTAX_CLASS(EnumCaseDecl, Decl)
+
+    // type of the parent `enum`
+    SYNTAX_FIELD(TypeExp, type)
+
+    RAW(
+    Type* getType() { return type.type.Ptr(); }
+    )
+
+    // Tag value
+    SYNTAX_FIELD(RefPtr<Expr>, tagExpr)
+END_SYNTAX_CLASS()
 
 // An interface which other types can conform to
 SIMPLE_SYNTAX_CLASS(InterfaceDecl, AggTypeDecl)
@@ -156,6 +177,7 @@ END_SYNTAX_CLASS()
 // A scope for local declarations (e.g., as part of a statement)
 SIMPLE_SYNTAX_CLASS(ScopeDecl, ContainerDecl)
 
+// A function/initializer/subscript parameter (potentially mutable)
 SIMPLE_SYNTAX_CLASS(ParamDecl, VarDeclBase)
 
 // Base class for things that have parameter lists and can thus be applied to arguments ("called")
@@ -205,8 +227,6 @@ SIMPLE_SYNTAX_CLASS(RefAccessorDecl, AccessorDecl)
 
 SIMPLE_SYNTAX_CLASS(FuncDecl, FunctionDeclBase)
 
-SIMPLE_SYNTAX_CLASS(Variable, VarDeclBase);
-
 // A "module" of code (essentiately, a single translation unit)
 // that provides a scope for some number of declarations.
 SYNTAX_CLASS(ModuleDecl, ContainerDecl)
diff --git a/source/slang/expr-defs.h b/source/slang/expr-defs.h
index bd4ba5038..8afa93fbd 100644
--- a/source/slang/expr-defs.h
+++ b/source/slang/expr-defs.h
@@ -183,7 +183,7 @@ END_SYNTAX_CLASS()
 // An expression that binds a temporary variable in a local expression context
 SYNTAX_CLASS(LetExpr, Expr)
 RAW(
-    RefPtr<VarDeclBase> decl;
+    RefPtr<VarDecl> decl;
     RefPtr<Expr> body;
 )
 END_SYNTAX_CLASS()
diff --git a/source/slang/lower-to-ir.cpp b/source/slang/lower-to-ir.cpp
index 3e981dbc5..082f16b22 100644
--- a/source/slang/lower-to-ir.cpp
+++ b/source/slang/lower-to-ir.cpp
@@ -768,17 +768,17 @@ LoweredValInfo emitCallToDeclRef(
 }
 
 IRInst* getFieldKey(
-    IRGenContext*           context,
-    DeclRef<StructField>    field)
+    IRGenContext*       context,
+    DeclRef<VarDecl>    field)
 {
     return getSimpleVal(context, emitDeclRef(context, field, context->irBuilder->getKeyType()));
 }
 
 LoweredValInfo extractField(
-    IRGenContext*           context,
-    IRType*                 fieldType,
-    LoweredValInfo          base,
-    DeclRef<StructField>    field)
+    IRGenContext*       context,
+    IRType*             fieldType,
+    LoweredValInfo      base,
+    DeclRef<VarDecl>    field)
 {
     IRBuilder* builder = context->irBuilder;
 
@@ -900,7 +900,7 @@ top:
             auto base = materialize(context, boundMemberInfo->base);
 
             auto declRef = boundMemberInfo->declRef;
-            if( auto fieldDeclRef = declRef.As<StructField>() )
+            if( auto fieldDeclRef = declRef.As<VarDecl>() )
             {
                 lowered = extractField(context, boundMemberInfo->type, base, fieldDeclRef);
                 goto top;
@@ -1871,7 +1871,7 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
         auto loweredBase = lowerRValueExpr(context, expr->BaseExpression);
 
         auto declRef = expr->declRef;
-        if (auto fieldDeclRef = declRef.As<StructField>())
+        if (auto fieldDeclRef = declRef.As<VarDecl>())
         {
             // Okay, easy enough: we have a reference to a field of a struct type...
             return extractField(loweredType, loweredBase, fieldDeclRef);
@@ -2032,7 +2032,7 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
             if (auto aggTypeDeclRef = declRef.As<AggTypeDecl>())
             {
                 List<IRInst*> args;
-                for (auto ff : getMembersOfType<StructField>(aggTypeDeclRef))
+                for (auto ff : getMembersOfType<VarDecl>(aggTypeDeclRef))
                 {
                     if (ff.getDecl()->HasModifier<HLSLStaticModifier>())
                         continue;
@@ -2050,7 +2050,7 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
         UNREACHABLE_RETURN(LoweredValInfo());
     }
 
-    LoweredValInfo getDefaultVal(StructField* decl)
+    LoweredValInfo getDefaultVal(VarDeclBase* decl)
     {
         if(auto initExpr = decl->initExpr)
         {
@@ -2156,7 +2156,7 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
             if (auto aggTypeDeclRef = declRef.As<AggTypeDecl>())
             {
                 UInt argCounter = 0;
-                for (auto ff : getMembersOfType<StructField>(aggTypeDeclRef))
+                for (auto ff : getMembersOfType<VarDecl>(aggTypeDeclRef))
                 {
                     if (ff.getDecl()->HasModifier<HLSLStaticModifier>())
                         continue;
@@ -2596,9 +2596,9 @@ struct ExprLoweringVisitorBase : ExprVisitor<Derived, LoweredValInfo>
     }
 
     LoweredValInfo extractField(
-        IRType*                 fieldType,
-        LoweredValInfo          base,
-        DeclRef<StructField>    field)
+        IRType*             fieldType,
+        LoweredValInfo      base,
+        DeclRef<VarDecl>    field)
     {
         return Slang::extractField(context, fieldType, base, field);
     }
@@ -3713,7 +3713,7 @@ LoweredValInfo tryGetAddress(
             // we care about, and then write it back.
 
             auto declRef = boundMemberInfo->declRef;
-            if( auto fieldDeclRef = declRef.As<StructField>() )
+            if( auto fieldDeclRef = declRef.As<VarDecl>() )
             {
                 auto baseVal = boundMemberInfo->base;
                 auto basePtr = tryGetAddress(context, baseVal, TryGetAddressMode::Aggressive);
@@ -3955,7 +3955,7 @@ top:
             // we care about, and then write it back.
 
             auto declRef = boundMemberInfo->declRef;
-            if( auto fieldDeclRef = declRef.As<StructField>() )
+            if( auto fieldDeclRef = declRef.As<VarDecl>() )
             {
                 // materialize the base value and move it into
                 // a mutable temporary if needed
@@ -4321,7 +4321,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         return LoweredValInfo();
     }
 
-    bool isGlobalVarDecl(VarDeclBase* decl)
+    bool isGlobalVarDecl(VarDecl* decl)
     {
         auto parent = decl->ParentDecl;
         if (dynamic_cast<ModuleDecl*>(parent))
@@ -4329,11 +4329,31 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
             // Variable declared at global scope? -> Global.
             return true;
         }
+        else if(dynamic_cast<AggTypeDeclBase*>(parent))
+        {
+            if(decl->HasModifier<HLSLStaticModifier>())
+            {
+                // A `static` member variable is effectively global.
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    bool isMemberVarDecl(VarDecl* decl)
+    {
+        auto parent = decl->ParentDecl;
+        if (dynamic_cast<AggTypeDecl*>(parent))
+        {
+            // A variable declared inside of an aggregate type declaration is a member.
+            return true;
+        }
 
         return false;
     }
 
-    LoweredValInfo lowerGlobalShaderParam(VarDeclBase* decl)
+    LoweredValInfo lowerGlobalShaderParam(VarDecl* decl)
     {
         IRType* paramType = lowerType(context, decl->getType());
 
@@ -4361,7 +4381,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         return paramVal;
     }
 
-    LoweredValInfo lowerGlobalVarDecl(VarDeclBase* decl)
+    LoweredValInfo lowerGlobalVarDecl(VarDecl* decl)
     {
         if(isGlobalShaderParameter(decl))
         {
@@ -4716,7 +4736,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
             lowerType(context, decl->type));
     }
 
-    LoweredValInfo visitVarDeclBase(VarDeclBase* decl)
+    LoweredValInfo visitVarDecl(VarDecl* decl)
     {
         // Detect global (or effectively global) variables
         // and handle them differently.
@@ -4730,6 +4750,11 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
             return lowerFunctionStaticVarDecl(decl);
         }
 
+        if(isMemberVarDecl(decl))
+        {
+            return lowerMemberVarDecl(decl);
+        }
+
         // A user-defined variable declaration will usually turn into
         // an `alloca` operation for the variable's storage,
         // plus some code to initialize it and then store to the variable.
@@ -4838,7 +4863,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         // Emit any generics that should wrap the actual type.
         emitOuterGenerics(subContext, decl, decl);
 
-        return lowerRValueExpr(subContext, decl->initExpr);
+        return lowerRValueExpr(subContext, decl->tagExpr);
     }
 
     LoweredValInfo visitEnumDecl(EnumDecl* decl)
@@ -4902,7 +4927,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
 
         subBuilder->setInsertInto(irStruct);
 
-        for (auto fieldDecl : decl->getMembersOfType<StructField>())
+        for (auto fieldDecl : decl->getMembersOfType<VarDeclBase>())
         {
             if (fieldDecl->HasModifier<HLSLStaticModifier>())
             {
@@ -4946,7 +4971,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         return LoweredValInfo::simple(finishOuterGenerics(subBuilder, irStruct));
     }
 
-    LoweredValInfo visitStructField(StructField* fieldDecl)
+    LoweredValInfo lowerMemberVarDecl(VarDecl* fieldDecl)
     {
         // Each field declaration in the AST translates into
         // a "key" that can be used to extract field values
diff --git a/source/slang/parameter-binding.cpp b/source/slang/parameter-binding.cpp
index 3f5c01d09..6bb8749dd 100644
--- a/source/slang/parameter-binding.cpp
+++ b/source/slang/parameter-binding.cpp
@@ -658,10 +658,10 @@ static void diagnoseTypeFieldsMismatch(
 }
 
 static void collectFields(
-    DeclRef<AggTypeDecl>        declRef,
-    List<DeclRef<StructField>>& outFields)
+    DeclRef<AggTypeDecl>    declRef,
+    List<DeclRef<VarDecl>>& outFields)
 {
-    for( auto fieldDeclRef : getMembersOfType<StructField>(declRef) )
+    for( auto fieldDeclRef : getMembersOfType<VarDecl>(declRef) )
     {
         if(fieldDeclRef.getDecl()->HasModifier<HLSLStaticModifier>())
             continue;
@@ -883,8 +883,8 @@ static bool validateTypesMatch(
                 {
                     if( auto rightStructDeclRef = rightDeclRef.As<AggTypeDecl>() )
                     {
-                        List<DeclRef<StructField>> leftFields;
-                        List<DeclRef<StructField>> rightFields;
+                        List<DeclRef<VarDecl>> leftFields;
+                        List<DeclRef<VarDecl>> rightFields;
 
                         collectFields(leftStructDeclRef, leftFields);
                         collectFields(rightStructDeclRef, rightFields);
diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp
index 1afc8ebcf..a21d57b05 100644
--- a/source/slang/parser.cpp
+++ b/source/slang/parser.cpp
@@ -1322,17 +1322,17 @@ namespace Slang
     static RefPtr<VarDeclBase> CreateVarDeclForContext(
         ContainerDecl*  containerDecl )
     {
-        if (dynamic_cast<StructDecl*>(containerDecl) || dynamic_cast<ClassDecl*>(containerDecl))
-        {
-            return new StructField();
-        }
-        else if (dynamic_cast<CallableDecl*>(containerDecl))
+        if (dynamic_cast<CallableDecl*>(containerDecl))
         {
+            // Function parameters always use their dedicated syntax class.
+            //
             return new ParamDecl();
         }
         else
         {
-            return new Variable();
+            // Globals, locals, and member variables all use the same syntax class.
+            //
+            return new VarDecl();
         }
     }
 
@@ -2227,7 +2227,7 @@ namespace Slang
         // The first is a type declaration that holds all the members, while
         // the second is a variable declaration that uses the buffer type.
         RefPtr<StructDecl> bufferDataTypeDecl = new StructDecl();
-        RefPtr<Variable> bufferVarDecl = new Variable();
+        RefPtr<VarDecl> bufferVarDecl = new VarDecl();
 
         // Both declarations will have a location that points to the name
         parser->FillPosition(bufferDataTypeDecl.Ptr());
@@ -2395,7 +2395,7 @@ namespace Slang
         // The first is a type declaration that holds all the members, while
         // the second is a variable declaration that uses the buffer type.
         RefPtr<StructDecl> blockDataTypeDecl = new StructDecl();
-        RefPtr<Variable> blockVarDecl = new Variable();
+        RefPtr<VarDecl> blockVarDecl = new VarDecl();
 
         addModifier(blockDataTypeDecl, new ImplicitParameterGroupElementTypeModifier());
         addModifier(blockVarDecl, new ImplicitParameterGroupVariableModifier());
@@ -3135,7 +3135,7 @@ namespace Slang
 
         if(AdvanceIf(parser, TokenType::OpAssign))
         {
-            decl->initExpr = parser->ParseArgExpr();
+            decl->tagExpr = parser->ParseArgExpr();
         }
 
         return decl;
@@ -3270,7 +3270,7 @@ namespace Slang
         parser->ReadToken(TokenType::LParent);
 
         NameLoc varNameAndLoc = expectIdentifier(parser);
-        RefPtr<Variable> varDecl = new Variable();
+        RefPtr<VarDecl> varDecl = new VarDecl();
         varDecl->nameAndLoc = varNameAndLoc;
         varDecl->loc = varNameAndLoc.loc;
 
diff --git a/source/slang/stmt-defs.h b/source/slang/stmt-defs.h
index a9877c281..01dbcc4ca 100644
--- a/source/slang/stmt-defs.h
+++ b/source/slang/stmt-defs.h
@@ -98,7 +98,7 @@ END_SYNTAX_CLASS()
 
 // A compile-time, range-based `for` loop, which will not appear in the output code
 SYNTAX_CLASS(CompileTimeForStmt, ScopeStmt)
-    SYNTAX_FIELD(RefPtr<Variable>, varDecl)
+    SYNTAX_FIELD(RefPtr<VarDecl>, varDecl)
     SYNTAX_FIELD(RefPtr<Expr>, rangeBeginExpr)
     SYNTAX_FIELD(RefPtr<Expr>, rangeEndExpr)
     SYNTAX_FIELD(RefPtr<Stmt>, body)
diff --git a/source/slang/syntax.h b/source/slang/syntax.h
index bd7de74ad..9b0277b56 100644
--- a/source/slang/syntax.h
+++ b/source/slang/syntax.h
@@ -1223,14 +1223,24 @@ namespace Slang
         return declRef.Substitute(declRef.getDecl()->initExpr);
     }
 
+    inline RefPtr<Type> getType(DeclRef<EnumCaseDecl> const& declRef)
+    {
+        return declRef.Substitute(declRef.getDecl()->type.Ptr());
+    }
+
+    inline RefPtr<Expr> getTagExpr(DeclRef<EnumCaseDecl> const& declRef)
+    {
+        return declRef.Substitute(declRef.getDecl()->tagExpr);
+    }
+
     inline RefPtr<Type> GetTargetType(DeclRef<ExtensionDecl> const& declRef)
     {
         return declRef.Substitute(declRef.getDecl()->targetType.Ptr());
     }
     
-    inline FilteredMemberRefList<StructField> GetFields(DeclRef<StructDecl> const& declRef)
+    inline FilteredMemberRefList<VarDecl> GetFields(DeclRef<StructDecl> const& declRef)
     {
-        return getMembersOfType<StructField>(declRef);
+        return getMembersOfType<VarDecl>(declRef);
     }
 
     inline RefPtr<Type> getBaseType(DeclRef<InheritanceDecl> const& declRef)