Class inheritance (sign offs rebase) (#59)

* use an environment variable to locate program files.
+ verify early that pyyaml isn't missing. this causes 20 fails.
+ trim whitespace

* wip: class inheritance support. 1st broad source inspection pass.

* WIP: core of the change. activate deep lookup (resolve inherited names recursively).

* add miscelaneous utilities
+ add re-emission of interfaces
+ add emission of inheritance lists
+ change base holding from set to vector (to preserve order)
+ fixup base lookup scope (it was target class's scope, must be enclosing)
+ add a typeof case to exploit inheritance lookup

* Evolution of the lookup function to fixup seenats refering to inherited members, and also fully qualified idExpression that were bypassing lookup.

* Prepare some test cases to verify inheritance access understanding.
wip: advanced / verification of seenats not ready

* Fixup of forgotten function-parameter-semantic when that parameter is nameless.

* fixup of symbol dependency ordering: UDT internal variables that got declared prior to a nested type, ended up extracted ABOVE the definition of their own parent type.

* introduce class inheritance seenat advanced test

* add one OK test for the empty diamond case

* Add an error case test to make sure the multi concrete base inheritance correctly spawns a #17 error.

Signed-off-by: Vivien Oddou <vivien.oddou@siliconstudio.co.jp>

Platform/Windows/src/DirectX12PlatformEmitter.cpp

@@ -90,11 +90,11 @@ namespace AZ::ShaderCompiler
 
             if (!descriptorTable.empty())
             {
-                rootAttrList.push_back(Decorate("            \"DescriptorTable(", Join(descriptorTable.begin(), descriptorTable.end()       , ", \" \\\n                            \""), ", visibility=SHADER_VISIBILITY_ALL)"));
+                rootAttrList.push_back(Decorate("            \"DescriptorTable(", Join(descriptorTable, ", \" \\\n                            \""), ", visibility=SHADER_VISIBILITY_ALL)"));
             }
             if (!samplerTable.empty())
             {
-                rootAttrList.push_back(Decorate("            \"DescriptorTable(", Join(samplerTable.begin(), samplerTable.end(), ", \" \\\n                            \""), ", visibility=SHADER_VISIBILITY_ALL)"));
+                rootAttrList.push_back(Decorate("            \"DescriptorTable(", Join(samplerTable, ", \" \\\n                            \""), ", visibility=SHADER_VISIBILITY_ALL)"));
             }
         }
 
@@ -123,7 +123,7 @@ namespace AZ::ShaderCompiler
 
         rootAttrList.insert(rootAttrList.begin(), "\"RootFlags(0)");
 
-        return Decorate("#define sig ", Join(rootAttrList.begin(), rootAttrList.end(), ", \" \\\n"), "\"\n\n");
+        return Decorate("#define sig ", Join(rootAttrList, ", \" \\\n"), "\"\n\n");
     }
 
 }

src/AzslcEmitter.cpp

@@ -84,7 +84,7 @@ namespace AZ::ShaderCompiler
 
         if (!attr.m_argList.empty())
         {
-            out << "(" << Join(attr.m_argList.begin(), attr.m_argList.end(), ", ") << ")";
+            out << "(" << Join(attr.m_argList, ", ") << ")";
         }
         return out;
     }
@@ -122,6 +122,7 @@ namespace AZ::ShaderCompiler
             switch (iteratedSymbolKind)
             {
                 // top-level enums, structs and classes, as well as immediate-type-declaration enum/structs (`struct S{} s;`)
+            case Kind::Interface:
             case Kind::Struct:
             case Kind::Class:
             case Kind::Enum:
@@ -357,7 +358,7 @@ namespace AZ::ShaderCompiler
             }
         }
 
-        // DXC has made an error the definition of typedef in classes -> move them all to global
+        // DXC has made into an error the definition of typedef in classes -> move them all to global
         for (auto& [typeAliasUid, typeAliasInfo] : m_ir->GetOrderedSymbolsOfSubType_2<TypeAliasInfo>())
         {
             if (!IsGlobal(typeAliasUid.GetName()))
@@ -486,22 +487,48 @@ namespace AZ::ShaderCompiler
         EmitPreprocessorLineDirective(origSourceLine);
     }
 
-    void CodeEmitter::EmitStruct(const ClassInfo& classInfo, string_view structName, const Options& options)
+    string CodeEmitter::EmitInheritanceList(const ClassInfo& clInfo)
+    {
+        string hlsl = clInfo.HasAnyBases() ? " : " : "";
+        vector<string> mutatedBaseNames;
+        TransformCopy(clInfo.GetBases(), mutatedBaseNames,
+                      [&](const IdentifierUID& uid)
+                      {
+                          return GetTranslatedName(uid, UsageContext::ReferenceSite);
+                      });
+        hlsl += Join(mutatedBaseNames, ", ");
+        return hlsl;
+    }
+
+    void CodeEmitter::EmitStruct(const ClassInfo& classInfo, string_view structuredSymName, const Options& options)
     {
         EmitEmptyLinesToLineNumber(classInfo.GetOriginalLineNumber());
 
-        const bool hasName = (structName.length() > 0);
+        auto HlslStructuredDelcTagFromKind = [](Kind k)
+        {
+            switch (k)
+            {
+            case Kind::Struct: return "struct";
+            case Kind::Class: return "class";
+            case Kind::Interface: return "interface";
+            default: return " ";
+            }
+        };
+
+        const bool hasName = (structuredSymName.length() > 0);
         const auto tabs = "    ";
         if (hasName)
         {
-            EmitAllAttachedAttributes(IdentifierUID { QualifiedNameView{structName} });
-
-            m_out << (classInfo.m_kind == Kind::Struct ? "struct " : "class ") << GetTranslatedName(QualifiedNameView{structName}, UsageContext::DeclarationSite) << "\n{\n";
+            EmitAllAttachedAttributes(IdentifierUID { QualifiedNameView{structuredSymName} });
+            m_out << HlslStructuredDelcTagFromKind(classInfo.m_kind) << " "
+                  << GetTranslatedName(QualifiedNameView{structuredSymName}, UsageContext::DeclarationSite)
+                  << EmitInheritanceList(classInfo)
+                  << "\n{\n"; // conclusion of "class X : ::Stuff {"
         }
 
         for (const IdentifierUID& memberUid : classInfo.GetOrderedMembers())
         {
-            if (structName.empty() || m_translations.GetLandingScope(memberUid.GetName()) == structName)
+            if (structuredSymName.empty() || m_translations.GetLandingScope(memberUid.GetName()) == structuredSymName)
             {
                 auto& [uid, info] = *m_ir->GetIdAndKindInfo(memberUid.GetName());
                 if (info.IsKindOneOf(Kind::Class, Kind::Struct, Kind::Interface))
@@ -712,7 +739,6 @@ namespace AZ::ShaderCompiler
                                || emitAsDefinition && AlreadyEmittedFunctionDefinition(uid);
         bool undefinedFunction = funcSub.IsUndefinedFunction();
         if (riskDoubleEmission
-            || funcSub.m_isVirtual   // interface's methods (isVirtual) are a declarative construct of AZSL and don't appear in HLSL.
             || undefinedFunction && emitAsDefinition)
         {
             return;

src/AzslcEmitter.h

@@ -142,6 +142,11 @@ namespace AZ::ShaderCompiler
                         }
                     }
 
+                    if (auto* semantic = param.m_semanticCtx)
+                    {
+                        m_out << " " + semantic->getText();
+                    }
+
                     if (param.m_defaultValueExpression)
                     {
                         EmitText(param.m_defaultValueExpression->getSourceInterval());
@@ -222,11 +227,14 @@ namespace AZ::ShaderCompiler
         mutable NewLineCounterStream m_out;
         void EmitEmptyLinesToLineNumber(size_t originalLineNumber) const;
 
-        // This template takes over the previous implementation of
-        // void GetTextInStream(misc::Interval interval, std::ostream& output) const override;
-        // The idea is that by using the template We only have to write the same code once
-        // whether We are using a regular std::ostream or an instance of NewLineCounterStream.
+        //! This template takes over the previous implementation of
+        //! void GetTextInStream(misc::Interval interval, std::ostream& output) const override;
+        //! The idea is that by using the template We only have to write the same code once
+        //! whether We are using a regular std::ostream or an instance of NewLineCounterStream.
         template <class StreamLike>
         void GetTextInStreamInternal(misc::Interval interval, StreamLike& output, bool emitNewLines) const;
+
+        //! This is a readability function for class emission code. Serves for HLSL declarator of classes
+        string EmitInheritanceList(const ClassInfo& clInfo);
     };
 }

src/AzslcException.h

@@ -72,6 +72,8 @@ namespace AZ::ShaderCompiler
         ORCHESTRATOR_SRG_EXTENSION_HAS_DIFFERENT_SEMANTIC = 47u,
         ORCHESTRATOR_UNBOUNDED_RESOURCE_ISSUE = 48u,
         ORCHESTRATOR_UNKNOWN_OPTION_TYPE = 49u,
+        ORCHESTRATOR_CONSTANT_FOLDING_FAULT = 50u,
+        ORCHESTRATOR_TYPE_LOOKUP_FAULT = 51u,
 
 
         // Treat all compiler warnings as errors

src/AzslcKindInfo.h

@@ -105,6 +105,29 @@ namespace AZ::ShaderCompiler
             m_ordered.insert(itor, newUid);
         }
 
+        bool HasBase(const IdentifierUID& query) const
+        {
+            return std::find(m_bases.begin(), m_bases.end(), query) != m_bases.end();
+        }
+
+        bool HasAnyBases() const
+        {
+            return !m_bases.empty();
+        }
+
+        void PushBase(const IdentifierUID& newBase)
+        {
+            if (!HasBase(newBase))
+            {
+                m_bases.push_back(newBase);
+            }
+        }
+
+        const vector<IdentifierUID>& GetBases() const
+        {
+            return m_bases;
+        }
+
         const vector<IdentifierUID>& GetMemberFields() const
         {
             return m_memberFields;
@@ -185,7 +208,6 @@ namespace AZ::ShaderCompiler
         }
 
         Kind                           m_kind;   // which of class/struct/interface/srgsemantic ? (repetition of data in the upper KindInfo)
-        unordered_set< IdentifierUID > m_bases;
 
         using DeclNode = variant< AstClassDeclNode*, AstStructDeclNode*, AstEnumDeclNode*, AstInterfaceDeclNode*, AstSRGSemanticDeclNode* >;
         DeclNode                       m_declNodeVt;
@@ -197,6 +219,7 @@ namespace AZ::ShaderCompiler
         unordered_set< IdentifierUID > m_members;      //!< Fast lookup
         vector< IdentifierUID >        m_memberFields; //!< Only the member fields, in order of declaration. All member fields are members.
         vector< IdentifierUID >        m_ordered;      //!< Ordered. all contained symbols
+        vector< IdentifierUID >        m_bases;
     };
 
     //! an extended type information gathers:
@@ -346,7 +369,7 @@ namespace AZ::ShaderCompiler
         // returns an ArrayDimensions struct const ref.
         inline const auto&         GetArrayDimensions() const;
         // Returns the line number, in the AZSL file, where this symbol is declared. 
-        inline size_t GetOriginalLineNumber () const;
+        inline size_t              GetOriginalLineNumber () const;
 
         AstUnnamedVarDecl*         m_declNode = nullptr;
         UnqualifiedName            m_identifier;
@@ -657,14 +680,15 @@ namespace AZ::ShaderCompiler
         }
 
         //! add a parameter
-        void PushParameter(IdentifierUID varName, const ExtendedTypeInfo& typeInfo, TypeQualifier typeQualifier, const std::vector<azslParser::ArrayRankSpecifierContext*>& arrayRankSpecifiers, AstVarInitializer* initCtx)
+        void PushParameter(IdentifierUID varName, const ExtendedTypeInfo& typeInfo, TypeQualifier typeQualifier, AstUnnamedVarDecl* unnamedCtx)
         {
             Parameter param;
             param.m_varId = varName;
             param.m_typeInfo = typeInfo;
             param.m_typeQualifier = typeQualifier;
-            param.m_arrayRankSpecifiers = arrayRankSpecifiers;
-            param.m_defaultValueExpression = initCtx;
+            param.m_semanticCtx = unnamedCtx->SemanticOpt;
+            param.m_arrayRankSpecifiers = unnamedCtx->ArrayRankSpecifiers;
+            param.m_defaultValueExpression = unnamedCtx->variableInitializer();
             m_parameters[m_currentList].push_back(param);
         }
 
@@ -756,6 +780,7 @@ namespace AZ::ShaderCompiler
             IdentifierUID m_varId;
             ExtendedTypeInfo m_typeInfo;
             TypeQualifier m_typeQualifier;
+            azslParser::HlslSemanticContext* m_semanticCtx = nullptr;
             std::vector<azslParser::ArrayRankSpecifierContext*> m_arrayRankSpecifiers;
             AstVarInitializer* m_defaultValueExpression = nullptr;
         };

src/AzslcListener.cpp

@@ -64,7 +64,10 @@ namespace AZ::ShaderCompiler
     void SemaCheckListener::enterClassDefinition(azslParser::ClassDefinitionContext* ctx)
     {
         m_ir->m_sema.RegisterClass(ctx);
-        m_ir->m_scope.EnterScope(ctx->Name->getText(), ctx->LeftBrace()->getSourceInterval().a);
+        if (!ctx->baseList())  // if there is a base list, we can't enter the scope that early. it has to be at base list exit.
+        {
+            m_ir->m_scope.EnterScope(ctx->Name->getText(), ctx->LeftBrace()->getSourceInterval().a);
+        }
     }
 
     void SemaCheckListener::enterEnumDefinition(azslParser::EnumDefinitionContext* ctx)
@@ -278,9 +281,18 @@ namespace AZ::ShaderCompiler
 
     void SemaCheckListener::enterBaseList(azslParser::BaseListContext* ctx)
     {
+
         m_ir->m_sema.RegisterBases(ctx);
     }
 
+    void SemaCheckListener::exitBaseList(azslParser::BaseListContext* ctx)
+    {
+        // if there was a base list, we deferred entering in scope
+        auto* classDefCtx = polymorphic_downcast<AstClassDeclNode*>(ctx->parent);  // baseList is only used in that context
+        m_ir->m_scope.EnterScope(classDefCtx->Name->getText(),
+                                 classDefCtx->LeftBrace()->getSourceInterval().a);
+    }
+
     void SemaCheckListener::enterCompilerExtensionStatement(azslParser::CompilerExtensionStatementContext* ctx)
     {
         if (m_silentPrintExtensions)

src/AzslcListener.h

@@ -46,6 +46,7 @@ namespace AZ::ShaderCompiler
         void exitFunctionParam(azslParser::FunctionParamContext* ctx) override;
         void enterNamedVariableDeclarator(azslParser::NamedVariableDeclaratorContext* ctx) override;
         void enterBaseList(azslParser::BaseListContext* ctx) override;
+        void exitBaseList(azslParser::BaseListContext* ctx) override;
         void enterCompilerExtensionStatement(azslParser::CompilerExtensionStatementContext* ctx) override;
         void enterGlobalAttribute(azslParser::GlobalAttributeContext* ctx) override;
         void enterAttachedAttribute(azslParser::AttachedAttributeContext* ctx) override;

src/AzslcMain.cpp

@@ -21,9 +21,9 @@ namespace StdFs = std::filesystem;
 // Correspond to the supported version of the AZSL language.
 #define AZSLC_MAJOR "1"
 // For large features or milestones. Minor version allows for breaking changes. Existing tests can change.
-#define AZSLC_MINOR "7"
+#define AZSLC_MINOR "8"   // introduction of class inheritance
 // For small features or bug fixes. They cannot introduce breaking changes. Existing tests shouldn't change.
-#define AZSLC_REVISION "35" // Upgrade from Antlr 4.7.1 to Antlr 4.9.3
+#define AZSLC_REVISION "4"
 namespace AZ::ShaderCompiler
 {
     DiagnosticStream verboseCout;

src/AzslcMangling.h

@@ -18,7 +18,7 @@ namespace AZ::ShaderCompiler
     //! Picture a sort of BOOST_STRONG_TYPEDEF. But that keeps compatibility with a 'view' type
     struct QualifiedName : string
     {
-        // I need to declare explicitly a default constructor, since the declaration of a copy constructor hereunder would otherwise delete it.
+        // Need to declare explicitly a default constructor, since the declaration of a copy constructor hereunder would otherwise delete it.
         QualifiedName() = default;
 
         // Provide implicit compatibility from string_view for sheer convenience
@@ -354,6 +354,12 @@ namespace AZ::ShaderCompiler
         return QualifiedNameView{GetParentName(string_view{path})};
     }
 
+    //! Overload for when you work with QualifiedName type
+    inline QualifiedNameView GetParentName(const QualifiedName& path)
+    {
+        return QualifiedNameView{GetParentName(QualifiedNameView{path})};
+    }
+
     struct PathPart
     {
         string_view m_slice;

src/AzslcReflection.cpp

@@ -136,7 +136,7 @@ namespace AZ::ShaderCompiler
                 {
                     return false;
                 }
-                auto[semanticName, semanticIndex, isSystemValue] = ExtractHlslSemantic(hlslSemantic);
+                auto [semanticName, semanticIndex, isSystemValue] = ExtractHlslSemantic(hlslSemantic);
                 if (!BuildOMElement(jsonVal, attrVarInfo.m_typeInfoExt, semanticName.c_str(), semanticIndex, isSystemValue))
                 {
                     return false;

src/AzslcScopeTracker.cpp

@@ -50,7 +50,7 @@ namespace AZ::ShaderCompiler
 
     void ScopeTracker::UpdateCurScopeUID()
     {
-        auto& [uid, Kind] = *m_symFinder(m_currentScopePath);
+        auto& [uid, _] = *m_symFinder(m_currentScopePath);
         // ↓ in this case please make sure you register the scope identifier before calling EnterScope. yes it is a sequential coupling antipattern, sorry for now
         assert(IsRooted(m_currentScopePath));
         m_currentScopeUID = uid;