From a08a3140716b89146bf0a22dc014c5470e90e910 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Tue, 22 Jan 2019 14:57:25 -0800 Subject: 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`). --- source/slang/check.cpp | 37 +++++++++++++------- source/slang/decl-defs.h | 38 +++++++++++++++----- source/slang/expr-defs.h | 2 +- source/slang/lower-to-ir.cpp | 71 ++++++++++++++++++++++++++------------ source/slang/parameter-binding.cpp | 10 +++--- source/slang/parser.cpp | 20 +++++------ source/slang/stmt-defs.h | 2 +- source/slang/syntax.h | 14 ++++++-- 8 files changed, 130 insertions(+), 64 deletions(-) (limited to 'source') 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 varDecl = new Variable(); + RefPtr 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()) + // + if (auto varDecl = decl.As()) { if (auto parenScope = varDecl->ParentDecl->As()) { @@ -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(toStructDeclRef)) + for(auto fieldDeclRef : getMembersOfType(toStructDeclRef)) { RefPtr coercedArg; bool argResult = tryReadArgFromInitializerList( @@ -2806,11 +2807,6 @@ namespace Slang } } - void visitStructField(StructField* field) - { - CheckVarDeclCommon(field); - } - bool doesSignatureMatchRequirement( DeclRef satisfyingMemberDeclRef, DeclRef requiredMemberDeclRef, @@ -3649,7 +3645,7 @@ namespace Slang IntegerLiteralValue defaultTag = 0; for(auto caseDecl : decl->getMembersOfType()) { - 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()) + { + // 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(expr)) @@ -9534,6 +9538,13 @@ namespace Slang qualType.IsLeftValue = isLValue; return qualType; } + else if( auto enumCaseDeclRef = declRef.As() ) + { + QualType qualType; + qualType.type = getType(enumCaseDeclRef); + qualType.IsLeftValue = false; + return qualType; + } else if (auto typeAliasDeclRef = declRef.As()) { 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, 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 GetFields() + FilteredMemberList GetFields() { - return getMembersOfType(); + return getMembersOfType(); } ) 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, 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 decl; + RefPtr decl; RefPtr 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 field) + IRGenContext* context, + DeclRef field) { return getSimpleVal(context, emitDeclRef(context, field, context->irBuilder->getKeyType())); } LoweredValInfo extractField( - IRGenContext* context, - IRType* fieldType, - LoweredValInfo base, - DeclRef field) + IRGenContext* context, + IRType* fieldType, + LoweredValInfo base, + DeclRef 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() ) + if( auto fieldDeclRef = declRef.As() ) { lowered = extractField(context, boundMemberInfo->type, base, fieldDeclRef); goto top; @@ -1871,7 +1871,7 @@ struct ExprLoweringVisitorBase : ExprVisitor auto loweredBase = lowerRValueExpr(context, expr->BaseExpression); auto declRef = expr->declRef; - if (auto fieldDeclRef = declRef.As()) + if (auto fieldDeclRef = declRef.As()) { // 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 if (auto aggTypeDeclRef = declRef.As()) { List args; - for (auto ff : getMembersOfType(aggTypeDeclRef)) + for (auto ff : getMembersOfType(aggTypeDeclRef)) { if (ff.getDecl()->HasModifier()) continue; @@ -2050,7 +2050,7 @@ struct ExprLoweringVisitorBase : ExprVisitor UNREACHABLE_RETURN(LoweredValInfo()); } - LoweredValInfo getDefaultVal(StructField* decl) + LoweredValInfo getDefaultVal(VarDeclBase* decl) { if(auto initExpr = decl->initExpr) { @@ -2156,7 +2156,7 @@ struct ExprLoweringVisitorBase : ExprVisitor if (auto aggTypeDeclRef = declRef.As()) { UInt argCounter = 0; - for (auto ff : getMembersOfType(aggTypeDeclRef)) + for (auto ff : getMembersOfType(aggTypeDeclRef)) { if (ff.getDecl()->HasModifier()) continue; @@ -2596,9 +2596,9 @@ struct ExprLoweringVisitorBase : ExprVisitor } LoweredValInfo extractField( - IRType* fieldType, - LoweredValInfo base, - DeclRef field) + IRType* fieldType, + LoweredValInfo base, + DeclRef 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() ) + if( auto fieldDeclRef = declRef.As() ) { 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() ) + if( auto fieldDeclRef = declRef.As() ) { // materialize the base value and move it into // a mutable temporary if needed @@ -4321,7 +4321,7 @@ struct DeclLoweringVisitor : DeclVisitor return LoweredValInfo(); } - bool isGlobalVarDecl(VarDeclBase* decl) + bool isGlobalVarDecl(VarDecl* decl) { auto parent = decl->ParentDecl; if (dynamic_cast(parent)) @@ -4329,11 +4329,31 @@ struct DeclLoweringVisitor : DeclVisitor // Variable declared at global scope? -> Global. return true; } + else if(dynamic_cast(parent)) + { + if(decl->HasModifier()) + { + // A `static` member variable is effectively global. + return true; + } + } + + return false; + } + + bool isMemberVarDecl(VarDecl* decl) + { + auto parent = decl->ParentDecl; + if (dynamic_cast(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 return paramVal; } - LoweredValInfo lowerGlobalVarDecl(VarDeclBase* decl) + LoweredValInfo lowerGlobalVarDecl(VarDecl* decl) { if(isGlobalShaderParameter(decl)) { @@ -4716,7 +4736,7 @@ struct DeclLoweringVisitor : DeclVisitor 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 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 // 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 subBuilder->setInsertInto(irStruct); - for (auto fieldDecl : decl->getMembersOfType()) + for (auto fieldDecl : decl->getMembersOfType()) { if (fieldDecl->HasModifier()) { @@ -4946,7 +4971,7 @@ struct DeclLoweringVisitor : DeclVisitor 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 declRef, - List>& outFields) + DeclRef declRef, + List>& outFields) { - for( auto fieldDeclRef : getMembersOfType(declRef) ) + for( auto fieldDeclRef : getMembersOfType(declRef) ) { if(fieldDeclRef.getDecl()->HasModifier()) continue; @@ -883,8 +883,8 @@ static bool validateTypesMatch( { if( auto rightStructDeclRef = rightDeclRef.As() ) { - List> leftFields; - List> rightFields; + List> leftFields; + List> 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 CreateVarDeclForContext( ContainerDecl* containerDecl ) { - if (dynamic_cast(containerDecl) || dynamic_cast(containerDecl)) - { - return new StructField(); - } - else if (dynamic_cast(containerDecl)) + if (dynamic_cast(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 bufferDataTypeDecl = new StructDecl(); - RefPtr bufferVarDecl = new Variable(); + RefPtr 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 blockDataTypeDecl = new StructDecl(); - RefPtr blockVarDecl = new Variable(); + RefPtr 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 varDecl = new Variable(); + RefPtr 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, varDecl) + SYNTAX_FIELD(RefPtr, varDecl) SYNTAX_FIELD(RefPtr, rangeBeginExpr) SYNTAX_FIELD(RefPtr, rangeEndExpr) SYNTAX_FIELD(RefPtr, 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 getType(DeclRef const& declRef) + { + return declRef.Substitute(declRef.getDecl()->type.Ptr()); + } + + inline RefPtr getTagExpr(DeclRef const& declRef) + { + return declRef.Substitute(declRef.getDecl()->tagExpr); + } + inline RefPtr GetTargetType(DeclRef const& declRef) { return declRef.Substitute(declRef.getDecl()->targetType.Ptr()); } - inline FilteredMemberRefList GetFields(DeclRef const& declRef) + inline FilteredMemberRefList GetFields(DeclRef const& declRef) { - return getMembersOfType(declRef); + return getMembersOfType(declRef); } inline RefPtr getBaseType(DeclRef const& declRef) -- cgit v1.2.3