Refactor preprocessor API to avoid coupling (#1559)

Based on review feedback from #1556, this change updates the Slang preprocessor so that it is no longer coupled to policy details from higher levels of the software stack. In particular, the preprocessor used to:

* Deal with updating the list of file paths that a `Module` depends on.

* (As of #1556) detect NVAPI-related macro definitions and use them to construct an AST-level `Modifier` attached to the `ModuleDecl`.

This change introduces a callback interface where the `Preprocessor` calls out to a `PreprocessorHandler` at certain points during execution, allowing the handler to introduce custom logic that suits a particular high-level use case.

This change also removes the dependence of the preprocessor on the `Linkage`, because in practice only a small number of its sub-objects were needed. As a convenience, a wrapper function that takes a `Linkage` was left in place so that the existing call sites didn't have to change very much.

1 files changed, 122 insertions, 4 deletions

diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index 0add5acea..1086338f4 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -862,8 +862,7 @@ Expr* Linkage::parseTermString(String typeStr, RefPtr<Scope> scope)
         &sink,
         nullptr,
         Dictionary<String,String>(),
-        this,
-        nullptr);
+        this);
 
     return parseTermFromSourceFile(
         getASTBuilder(),
@@ -919,6 +918,119 @@ FrontEndCompileRequest::FrontEndCompileRequest(
 {
 }
 
+    /// Handlers for preprocessor callbacks to use when doing ordinary front-end compilation
+struct FrontEndPreprocessorHandler : PreprocessorHandler
+{
+public:
+    FrontEndPreprocessorHandler(
+        Module*         module,
+        ASTBuilder*     astBuilder,
+        DiagnosticSink* sink)
+        : m_module(module)
+        , m_astBuilder(astBuilder)
+        , m_sink(sink)
+    {
+    }
+
+protected:
+    Module*         m_module;
+    ASTBuilder*     m_astBuilder;
+    DiagnosticSink* m_sink;
+
+    // The first task that this handler tries to deal with is
+    // capturing all the files on which a module is dependent.
+    //
+    // That information is exposed through public APIs and used
+    // by applications to decide when they need to "hot reload"
+    // their shader code.
+    //
+    void handleFileDependency(String const& path) SLANG_OVERRIDE
+    {
+        m_module->addFilePathDependency(path);
+    }
+
+    // The second task that this handler deals with is detecting
+    // whether any macro values were set in a given source file
+    // that are semantically relevant to other stages of compilation.
+    //
+    void handleEndOfFile(Preprocessor* preprocessor) SLANG_OVERRIDE
+    {
+        // We look at the preprocessor state after reading the entire
+        // source file/string, in order to see if any macros have been
+        // set that should be considered semantically relevant for
+        // later stages of compilation.
+        //
+        // Note: Checking the macro environment *after* preprocessing is complete
+        // means that we can treat macros introduced via `-D` options or the API
+        // equivalently to macros introduced via `#define`s in user code.
+        //
+        // For now, the only case of semantically-relevant macros we need to worrry
+        // about are the NVAPI macros used to establish the register/space to use.
+        //
+        static const char* kNVAPIRegisterMacroName = "NV_SHADER_EXTN_SLOT";
+        static const char* kNVAPISpaceMacroName = "NV_SHADER_EXTN_REGISTER_SPACE";
+
+        // For NVAPI use, the `NV_SHADER_EXTN_SLOT` macro is required to be defined.
+        //
+        String nvapiRegister;
+        SourceLoc nvapiRegisterLoc;
+        if(!SLANG_FAILED(findMacroValue(preprocessor, kNVAPIRegisterMacroName, nvapiRegister, nvapiRegisterLoc)))
+        {
+            // In contrast, NVAPI can be used without defining `NV_SHADER_EXTN_REGISTER_SPACE`,
+            // which effectively defaults to `space0`.
+            //
+            String nvapiSpace = "space0";
+            SourceLoc nvapiSpaceLoc;
+            findMacroValue(preprocessor, kNVAPISpaceMacroName, nvapiSpace, nvapiSpaceLoc);
+
+            // We are going to store the values of these macros on the AST-level `ModuleDecl`
+            // so that they will be available to later processing stages.
+            //
+            auto moduleDecl = m_module->getModuleDecl();
+
+            if(auto existingModifier = moduleDecl->findModifier<NVAPISlotModifier>())
+            {
+                // If there is already a modifier attached to the module (perhaps
+                // because of preprocessing a different source file, or because
+                // of settings established via command-line options), then we
+                // need to validate that the values being set in this file
+                // match those already set (or else there is likely to be
+                // some kind of error in the user's code).
+                //
+                _validateNVAPIMacroMatch(kNVAPIRegisterMacroName, existingModifier->registerName, nvapiRegister,  nvapiRegisterLoc);
+                _validateNVAPIMacroMatch(kNVAPISpaceMacroName,    existingModifier->spaceName,    nvapiSpace,     nvapiSpaceLoc);
+            }
+            else
+            {
+                // If there is no existing modifier on the module, then we
+                // take responsibility for adding one, based on the macro
+                // values we saw.
+                //
+                auto modifier = m_astBuilder->create<NVAPISlotModifier>();
+                modifier->loc = nvapiRegisterLoc;
+                modifier->registerName = nvapiRegister;
+                modifier->spaceName = nvapiSpace;
+
+                addModifier(moduleDecl, modifier);
+            }
+        }
+    }
+
+        /// Validate that a re-defintion of an NVAPI-related macro matches any previous definition
+    void _validateNVAPIMacroMatch(
+        char const*     macroName,
+        String const&   existingValue,
+        String const&   newValue,
+        SourceLoc       loc)
+    {
+        if( existingValue != newValue )
+        {
+            m_sink->diagnose(loc, Diagnostics::nvapiMacroMismatch, macroName, existingValue, newValue);
+        }
+    }
+};
+
+
 void FrontEndCompileRequest::parseTranslationUnit(
     TranslationUnitRequest* translationUnit)
 {
@@ -983,6 +1095,13 @@ void FrontEndCompileRequest::parseTranslationUnit(
         translationUnitSyntax->modifiers.first = astBuilder->create<FromStdLibModifier>();
     }
 
+    // We use a custom handler for preprocessor callbacks, to
+    // ensure that relevant state that is only visible during
+    // preprocessoing can be communicated to later phases of
+    // compilation.
+    //
+    FrontEndPreprocessorHandler preprocessorHandler(module, astBuilder, getSink());
+
     for (auto sourceFile : translationUnit->getSourceFiles())
     {
         auto tokens = preprocessSource(
@@ -991,8 +1110,7 @@ void FrontEndCompileRequest::parseTranslationUnit(
             &includeSystem,
             combinedPreprocessorDefinitions,
             getLinkage(),
-            module,
-            astBuilder);
+            &preprocessorHandler);
 
         parseSourceFile(
             astBuilder,