Merge branch 'origin/topic/bbannier/issue-1813'

CHANGES

@@ -1,3 +1,11 @@
+1.11.0-dev.287 | 2024-07-31 11:15:49 +0200
+
+  * Remember normalized paths when checking for duplicate files in driver. (Benjamin Bannier, Corelight)
+
+  * Remember files processed by the driver. (Benjamin Bannier, Corelight)
+
+  * GH-1813: Fix equality implementation of module UID. (Benjamin Bannier, Corelight)
+
 1.11.0-dev.282 | 2024-07-30 17:02:31 +0200
 
   * GH-1810: Fix nested look-ahead switches. (Robin Sommer, Corelight)

VERSION

@@ -1 +1 @@
-1.11.0-dev.282
+1.11.0-dev.287

hilti/toolchain/include/ast/ast-context.h

@@ -168,8 +168,7 @@ class ASTContext : public std::enable_shared_from_this<ASTContext> {
                                                   std::vector<hilti::rt::filesystem::path> search_dirs);
 
     /** Adds a new, empty module to the AST. */
-    declaration::Module* newModule(Builder* builder, const ID& id,
-                                   const hilti::rt::filesystem::path& process_extension);
+    declaration::Module* newModule(Builder* builder, ID id, const hilti::rt::filesystem::path& process_extension);
 
     /**
      * Retrieves a module node from the AST given its UID. Returns null if no

hilti/toolchain/include/ast/declarations/module-uid.h

@@ -55,16 +55,16 @@ struct UID {
      * @param parse_extension language extension determining how to *parse* this module
      * @param process_extension language extension determining how to process this module *after* parsing
      **/
-    UID(const ID& id, const hilti::rt::filesystem::path& parse_extension,
-        const hilti::rt::filesystem::path& process_extension)
-        : id(id),
+    UID(ID id, hilti::rt::filesystem::path parse_extension, hilti::rt::filesystem::path process_extension)
+        : id(std::move(id)),
           unique(_makeUnique(this->id)),
-          parse_extension(parse_extension),
-          process_extension(process_extension),
+          parse_extension(std::move(parse_extension)),
+          process_extension(std::move(process_extension)),
           in_memory(true) {
         assert(this->id && ! parse_extension.empty() && ! process_extension.empty());
         //  just make up a path
-        path = util::fmt("/tmp/hilti/%s.%" PRIu64 ".%s.%s", id, ++_no_file_counter, process_extension, parse_extension);
+        path = util::fmt("/tmp/hilti/%s.%" PRIu64 ".%s.%s", unique, ++_no_file_counter, process_extension,
+                         parse_extension);
     }
 
     UID(const UID& other) = default;
@@ -73,7 +73,8 @@ struct UID {
 
     /** Hashes the UID. */
     size_t hash() const {
-        return rt::hashCombine(std::hash<std::string_view>{}(id.str()), std::hash<std::string>{}(path.native()),
+        return rt::hashCombine(std::hash<std::string>{}(id.str()), std::hash<std::string_view>{}(unique.str()),
+                               std::hash<std::string>{}(path.native()),
                                std::hash<std::string>{}(parse_extension.native()),
                                std::hash<std::string>{}(process_extension.native()));
     }
@@ -82,14 +83,14 @@ struct UID {
     UID& operator=(UID&& other) = default;
 
     bool operator==(const UID& other) const {
-        return id == other.id && path == other.path && parse_extension == other.parse_extension &&
-               process_extension == other.process_extension;
+        return std::tie(id, unique, path, parse_extension, process_extension) ==
+               std::tie(other.id, other.unique, other.path, other.parse_extension, other.process_extension);
     }
 
     bool operator!=(const UID& other) const { return ! (*this == other); }
     bool operator<(const UID& other) const {
-        return std::make_tuple(id, path, parse_extension, process_extension) <
-               std::make_tuple(other.id, other.path, other.parse_extension, other.process_extension);
+        return std::tie(id, unique, path, parse_extension, process_extension) <
+               std::tie(other.id, other.unique, other.path, other.parse_extension, other.process_extension);
     }
 
     /** Returns the module's globally uniqued name. */

hilti/toolchain/src/ast/ast-context.cc

@@ -128,9 +128,9 @@ Result<declaration::module::UID> ASTContext::importModule(
     return uid;
 }
 
-declaration::Module* ASTContext::newModule(Builder* builder, const ID& id,
+declaration::Module* ASTContext::newModule(Builder* builder, ID id,
                                            const hilti::rt::filesystem::path& process_extension) {
-    auto uid = declaration::module::UID(id, process_extension, process_extension);
+    auto uid = declaration::module::UID(std::move(id), process_extension, process_extension);
     auto m = builder->declarationModule(uid);
     _addModuleToAST(m);
     return module(uid);

hilti/toolchain/src/compiler/driver.cc

@@ -6,6 +6,7 @@
 #include <exception>
 #include <fstream>
 #include <iostream>
+#include <system_error>
 #include <utility>
 
 #include <hilti/rt/libhilti.h>
@@ -552,7 +553,15 @@ Result<void*> Driver::_symbol(const std::string& symbol) {
 }
 
 Result<Nothing> Driver::addInput(const hilti::rt::filesystem::path& path) {
-    if ( _processed_paths.find(path.native()) != _processed_paths.end() )
+    auto path_normalized = path.lexically_normal();
+    if ( path_normalized.is_relative() ) {
+        std::error_code ec;
+        path_normalized = rt::filesystem::absolute(path_normalized, ec);
+        if ( ec )
+            return result::Error(fmt("could not compute absolute path for %s", path_normalized));
+    }
+
+    if ( _processed_paths.count(path_normalized.native()) )
         return Nothing();
 
     // Calling hook before stage check so that it can execute initialize()
@@ -573,8 +582,6 @@ Result<Nothing> Driver::addInput(const hilti::rt::filesystem::path& path) {
 
         (*unit)->setRequiresCompilation();
         _addUnit(*unit);
-
-        return Nothing();
     }
 
     else if ( path.extension() == ".cc" || path.extension() == ".cxx" ) {
@@ -597,7 +604,6 @@ Result<Nothing> Driver::addInput(const hilti::rt::filesystem::path& path) {
 
         HILTI_DEBUG(logging::debug::Driver, fmt("adding external C++ file %s", path));
         _external_cxxs.push_back(path);
-        return Nothing();
     }
 
     else if ( path.extension() == ".hlto" ) {
@@ -612,11 +618,14 @@ Result<Nothing> Driver::addInput(const hilti::rt::filesystem::path& path) {
         } catch ( const hilti::rt::EnvironmentError& e ) {
             hilti::rt::fatalError(e.what());
         }
-
-        return Nothing();
     }
 
-    return error("unsupported file type", path);
+    else
+        return error("unsupported file type", path);
+
+    _processed_paths.insert(path_normalized.native());
+
+    return Nothing();
 }
 
 Result<Nothing> Driver::addInput(declaration::module::UID uid) {

tests/hilti/ast/imported-twice.hlt

@@ -0,0 +1,7 @@
+# @TEST-DOC: Test that we handle the same module declared twice fine, regression test for #1813.
+#
+# @TEST-EXEC: hiltic -dj %INPUT %INPUT
+# @TEST-EXEC: hiltic -dj %INPUT $(basename %INPUT)
+# @TEST-EXEC: hiltic -dj $(basename %INPUT) %INPUT
+
+module foo {}