From 150218bec9e992d32833dd9a0c1396a4d7c12b7e Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Thu, 24 Sep 2020 13:09:40 -0700 Subject: 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. --- source/slang/slang.cpp | 126 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 122 insertions(+), 4 deletions(-) (limited to 'source/slang/slang.cpp') 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) &sink, nullptr, Dictionary(), - 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()) + { + // 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(); + 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(); } + // 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, -- cgit v1.2.3