Support defines and includes with compilation unit listings

MikePopoloski · Feb 4, 2024 · b3c5284 · b3c5284
1 parent 30d2d67
commit b3c5284
Show file tree

Hide file tree

Showing 12 changed files with 71 additions and 24 deletions.
diff --git a/bindings/python/UtilBindings.cpp b/bindings/python/UtilBindings.cpp
@@ -163,7 +163,7 @@ void registerUtil(py::module_& m) {
             "readHeader",
             [](SourceManager& self, std::string_view path, SourceLocation includedFrom,
                const SourceLibrary* library, bool isSystemPath) {
-                auto result = self.readHeader(path, includedFrom, library, isSystemPath);
+                auto result = self.readHeader(path, includedFrom, library, isSystemPath, {});
                 if (!result)
                     throw fs::filesystem_error("", path, result.error());
                 return *result;

diff --git a/include/slang/driver/SourceLoader.h b/include/slang/driver/SourceLoader.h
@@ -145,7 +145,7 @@ class SLANG_EXPORT SourceLoader {
     // One entry per unit of files + options to compile them.
     // Only used for addSeparateUnit.
     struct UnitEntry {
-        std::vector<std::string> includePaths;
+        std::vector<std::filesystem::path> includePaths;
         std::vector<std::string> defines;
         const SourceLibrary* library = nullptr;
     };

diff --git a/include/slang/parsing/Preprocessor.h b/include/slang/parsing/Preprocessor.h
@@ -48,6 +48,9 @@ struct SLANG_EXPORT PreprocessorOptions {
     /// A set of macro names to undefine at the start of file preprocessing.
     std::vector<std::string> undefines;
 
+    /// Additional include paths to use when preprocessing.
+    std::vector<std::filesystem::path> additionalIncludePaths;
+
     /// A set of preprocessor directives to be ignored.
     flat_hash_set<std::string_view> ignoreDirectives;
 };

diff --git a/include/slang/text/SourceManager.h b/include/slang/text/SourceManager.h
@@ -156,7 +156,8 @@ class SLANG_EXPORT SourceManager {
 
     /// Read in a header file from disk.
     BufferOrError readHeader(std::string_view path, SourceLocation includedFrom,
-                             const SourceLibrary* library, bool isSystemPath);
+                             const SourceLibrary* library, bool isSystemPath,
+                             std::span<std::filesystem::path const> additionalIncludePaths);
 
     /// Returns true if the given file path is already loaded and cached in the source manager.
     bool isCached(const std::filesystem::path& path) const;

diff --git a/include/slang/util/Bag.h b/include/slang/util/Bag.h
@@ -55,6 +55,14 @@ class SLANG_EXPORT Bag {
         return std::any_cast<T>(&it->second);
     }
 
+    /// Gets an element of type T from the bag, if it exists.
+    /// Otherwise adds a default constructed element to the bag
+    /// and returns a reference to it.
+    template<typename T>
+    T& insertOrGet() {
+        return *std::any_cast<T>(&items[SLANG_TYPEOF(T)]);
+    }
+
     /// Gets an element of type T from the bag, if it exists.
     /// Otherwise returns a default constructed T.
     template<typename T>

diff --git a/source/driver/SourceLoader.cpp b/source/driver/SourceLoader.cpp
@@ -9,6 +9,7 @@
 
 #include <fmt/core.h>
 
+#include "slang/parsing/Preprocessor.h"
 #include "slang/syntax/AllSyntax.h"
 #include "slang/syntax/SyntaxTree.h"
 #include "slang/text/SourceManager.h"
@@ -76,12 +77,19 @@ void SourceLoader::addLibraryMaps(std::string_view pattern, const fs::path& base
 void SourceLoader::addSeparateUnit(std::span<const std::string> filePatterns,
                                    std::vector<std::string> includePaths,
                                    std::vector<std::string> defines, std::string libraryName) {
+    std::error_code ec;
+    SmallVector<fs::path> includeDirs;
+    for (auto& str : includePaths)
+        svGlob({}, str, GlobMode::Directories, includeDirs, /* expandEnvVars */ false, ec);
+
     auto& unit = unitEntries.emplace_back();
-    unit.includePaths = std::move(includePaths);
     unit.defines = std::move(defines);
     unit.library = getOrAddLibrary(libraryName);
-    const bool isLibraryFile = unit.library != nullptr;
 
+    for (auto&& path : includeDirs)
+        unit.includePaths.emplace_back(std::move(path));
+
+    const bool isLibraryFile = unit.library != nullptr;
     for (auto pattern : filePatterns) {
         addFilesInternal(pattern, {}, isLibraryFile, unit.library, &unit,
                          /* expandEnvVars */ false);
@@ -217,8 +225,14 @@ SourceLoader::SyntaxTreeList SourceLoader::loadAndParseSources(const Bag& option
     };
 
     auto parseSeparateUnit = [&](const UnitEntry& unit, const std::vector<SourceBuffer>& buffers) {
-        // TODO: handle other unit features
-        auto tree = SyntaxTree::fromBuffers(buffers, sourceManager, optionBag, inheritedMacros);
+        auto unitOptions = optionBag;
+        auto& ppOptions = unitOptions.insertOrGet<parsing::PreprocessorOptions>();
+        ppOptions.predefines.insert(ppOptions.predefines.end(), unit.defines.begin(),
+                                    unit.defines.end());
+        ppOptions.additionalIncludePaths.insert(ppOptions.additionalIncludePaths.end(),
+                                                unit.includePaths.begin(), unit.includePaths.end());
+
+        auto tree = SyntaxTree::fromBuffers(buffers, sourceManager, unitOptions, inheritedMacros);
         tree->isLibraryUnit = srcOptions.onlyLint || unit.library != nullptr;
         return tree;
     };

diff --git a/source/parsing/Preprocessor.cpp b/source/parsing/Preprocessor.cpp
@@ -466,7 +466,7 @@ Trivia Preprocessor::handleIncludeDirective(Token directive) {
         path = path.substr(1, path.length() - 2);
 
         auto buffer = sourceManager.readHeader(path, directive.location(), getCurrentLibrary(),
-                                               isSystem);
+                                               isSystem, options.additionalIncludePaths);
         if (!buffer) {
             addDiag(diag::CouldNotOpenIncludeFile, fileName.range())
                 << path << buffer.error().message();

diff --git a/source/text/SourceManager.cpp b/source/text/SourceManager.cpp
@@ -277,10 +277,10 @@ SourceManager::BufferOrError SourceManager::readSource(const fs::path& path,
     return openCached(path, SourceLocation(), library);
 }
 
-SourceManager::BufferOrError SourceManager::readHeader(std::string_view path,
-                                                       SourceLocation includedFrom,
-                                                       const SourceLibrary* library,
-                                                       bool isSystemPath) {
+SourceManager::BufferOrError SourceManager::readHeader(
+    std::string_view path, SourceLocation includedFrom, const SourceLibrary* library,
+    bool isSystemPath, std::span<std::filesystem::path const> additionalIncludePaths) {
+
     // if the header is specified as an absolute path, just do a straight lookup
     SLANG_ASSERT(!path.empty());
     fs::path p = path;
@@ -316,6 +316,12 @@ SourceManager::BufferOrError SourceManager::readHeader(std::string_view path,
             return result;
     }
 
+    for (auto& dir : additionalIncludePaths) {
+        auto result = openCached(dir / p, includedFrom, library);
+        if (result)
+            return result;
+    }
+
     // Use library-specific include dirs if they exist.
     if (library) {
         for (auto& dir : library->includeDirs) {

diff --git a/tests/unittests/DriverTests.cpp b/tests/unittests/DriverTests.cpp
@@ -645,4 +645,11 @@ TEST_CASE("Driver separate unit listing") {
     REQUIRE(units[2]->getSourceLibrary() != nullptr);
     CHECK(units[1]->getSourceLibrary()->name == "lib2");
     CHECK(units[2]->getSourceLibrary()->name == "mylib");
+
+    auto defs = compilation->getDefinitions();
+    auto it = std::ranges::find_if(defs, [](auto sym) {
+        return sym->kind == SymbolKind::Definition && sym->name == "mod1" &&
+               sym->getSourceLibrary() && sym->getSourceLibrary()->name == "mylib";
+    });
+    CHECK(it != defs.end());
 }
diff --git a/tests/unittests/FileTests.cpp b/tests/unittests/FileTests.cpp
@@ -28,17 +28,17 @@ TEST_CASE("Read header (absolute)") {
     std::string testPath = getTestInclude();
 
     // check load failure
-    auto result = manager.readHeader("X:\\nonsense.txt", SourceLocation(), nullptr, false);
+    auto result = manager.readHeader("X:\\nonsense.txt", SourceLocation(), nullptr, false, {});
     CHECK(!result);
     CHECK(result.error() == std::errc::no_such_file_or_directory);
 
     // successful load
-    auto buffer1 = manager.readHeader(testPath, SourceLocation(), nullptr, false);
+    auto buffer1 = manager.readHeader(testPath, SourceLocation(), nullptr, false, {});
     REQUIRE(buffer1);
     CHECK(!buffer1->data.empty());
 
     // next load should be cached
-    auto buffer2 = manager.readHeader(testPath, SourceLocation(), nullptr, false);
+    auto buffer2 = manager.readHeader(testPath, SourceLocation(), nullptr, false, {});
     REQUIRE(buffer2);
     CHECK(!buffer2->data.empty());
     CHECK(buffer1->data.data() == buffer2->data.data());
@@ -48,45 +48,47 @@ TEST_CASE("Read header (relative)") {
     SourceManager manager;
 
     // relative to nothing should never return anything
-    auto result = manager.readHeader("relative", SourceLocation(), nullptr, false);
+    auto result = manager.readHeader("relative", SourceLocation(), nullptr, false, {});
     CHECK(!result);
     CHECK(result.error() == std::errc::no_such_file_or_directory);
 
     // get a file ID to load relative to
-    auto buffer1 = manager.readHeader(getTestInclude(), SourceLocation(), nullptr, false);
+    auto buffer1 = manager.readHeader(getTestInclude(), SourceLocation(), nullptr, false, {});
     REQUIRE(buffer1);
 
     // reading the same header by name should return the same data pointer
-    auto buffer2 = manager.readHeader("include.svh", SourceLocation(buffer1->id, 0), nullptr,
-                                      false);
+    auto buffer2 = manager.readHeader("include.svh", SourceLocation(buffer1->id, 0), nullptr, false,
+                                      {});
     CHECK(buffer2->data.data() == buffer1->data.data());
 
     // should be able to load relative
-    buffer2 = manager.readHeader("nested/file.svh", SourceLocation(buffer1->id, 0), nullptr, false);
+    buffer2 = manager.readHeader("nested/file.svh", SourceLocation(buffer1->id, 0), nullptr, false,
+                                 {});
     REQUIRE(buffer2);
     CHECK(!buffer2->data.empty());
 
     // load another level of relative
-    CHECK(manager.readHeader("nested_local.svh", SourceLocation(buffer2->id, 0), nullptr, false));
+    CHECK(
+        manager.readHeader("nested_local.svh", SourceLocation(buffer2->id, 0), nullptr, false, {}));
 }
 
 TEST_CASE("Read header (include dirs)") {
     SourceManager manager;
     CHECK(!manager.addSystemDirectories(findTestDir()));
 
-    auto buffer = manager.readHeader("include.svh", SourceLocation(), nullptr, true);
+    auto buffer = manager.readHeader("include.svh", SourceLocation(), nullptr, true, {});
     REQUIRE(buffer);
 
     CHECK(!manager.addUserDirectories(findTestDir() + "/nested"));
     buffer = manager.readHeader("../infinite_chain.svh", SourceLocation(buffer->id, 0), nullptr,
-                                false);
+                                false, {});
     CHECK(buffer);
 }
 
 TEST_CASE("Read header (dev/null)") {
     if (fs::exists("/dev/null")) {
         SourceManager manager;
-        auto buffer = manager.readHeader("/dev/null", SourceLocation(), nullptr, true);
+        auto buffer = manager.readHeader("/dev/null", SourceLocation(), nullptr, true, {});
         CHECK(buffer);
     }
 }

diff --git a/tests/unittests/data/test.sv b/tests/unittests/data/test.sv
@@ -8,3 +8,7 @@ module m;
 
     begin end
 endmodule
+
+`ifdef FOOBAR
+`include "mod1.sv"
+`endif
diff --git a/tests/unittests/data/unit.f b/tests/unittests/data/unit.f
@@ -1,3 +1,5 @@
 --library mylib
 test.sv
 test2.sv
+-DFOOBAR
+-Ilibtest