Reject exporting symbols with the same name from 2 modulesHEADcpp

7 files changed, 86 insertions, 14 deletions

diff --git a/boot/dependency.cc b/boot/dependency.cc
index 1eb5953..5053a36 100644
--- a/boot/dependency.cc
+++ b/boot/dependency.cc
@@ -72,7 +72,7 @@ namespace elna::boot
     }
 
     analysis_result analyze_semantics(std::unique_ptr<unit>& tree,
-            const std::vector<std::shared_ptr<symbol_table>>& imports,
+            const std::vector<module_import>& imports,
             const std::shared_ptr<symbol_table>& globals, const target_info& target,
             const std::filesystem::path& module_path)
     {
@@ -90,7 +90,23 @@ namespace elna::boot
         }
         for (const auto& import : imports)
         {
-            result.value.add_import(import);
+            const std::vector<import_collision> collisions = result.value.add_import(import.symbols);
+
+            for (const import_collision& collision : collisions)
+            {
+                symbol_declaration_error::redefinition original_definition{
+                    .original = collision.original->position,
+                    .file = collision.original->file
+                };
+                auto error = std::make_unique<symbol_declaration_error>(import.position,
+                        collision.name, original_definition);
+
+                result.errors.push_back(std::move(error));
+            }
+        }
+        if (!result.errors.empty())
+        {
+            return result;
         }
         declaration_visitor declarations(result.value, target, module_path);
         tree->accept(&declarations);
diff --git a/boot/symbol.cc b/boot/symbol.cc
index 58524ad..962808a 100644
--- a/boot/symbol.cc
+++ b/boot/symbol.cc
@@ -17,6 +17,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "elna/boot/symbol.h"
 
+#include <algorithm>
 #include <ranges>
 #include <cassert>
 #include <utility>
@@ -511,9 +512,24 @@ namespace elna::boot
         return this->unresolved.insert({ symbol_name, std::move(forward_declaration) }).second;
     }
 
-    void symbol_bag::add_import(const std::shared_ptr<symbol_table>& symbols)
+    std::vector<import_collision> symbol_bag::add_import(const std::shared_ptr<symbol_table>& symbols)
     {
-        this->imports.push_front(symbols);
+        std::vector<import_collision> collisions;
+
+        for (const auto& [name, imported] : *symbols)
+        {
+            std::shared_ptr<info> original = lookup_import(name);
+
+            // The same module imported twice hands out the same symbol, which
+            // doesn't conflict with itself.
+            if (original != nullptr && original != imported)
+            {
+                collisions.push_back({ .name = name, .original = std::move(original) });
+            }
+        }
+        this->imports.push_back(symbols);
+
+        return collisions;
     }
 
     bool symbol_bag::is_global() const
diff --git a/include/elna/boot/dependency.h b/include/elna/boot/dependency.h
index 195d37a..a87503e 100644
--- a/include/elna/boot/dependency.h
+++ b/include/elna/boot/dependency.h
@@ -149,6 +149,16 @@ namespace elna::boot
     std::filesystem::path build_path(const std::vector<std::string>& segments);
 
     /**
+     * Symbols brought in by one import declaration, paired with the position
+     * of the declaration that requested them.
+     */
+    struct module_import
+    {
+        source_position position;
+        std::shared_ptr<symbol_table> symbols;
+    };
+
+    /**
      * Analyzes a module semantically: collects type declarations, resolves
      * names, checks types and validates the module.
      *
@@ -165,7 +175,7 @@ namespace elna::boot
      * \return Module scope and diagnostics.
      */
     analysis_result analyze_semantics(std::unique_ptr<unit>& tree,
-            const std::vector<std::shared_ptr<symbol_table>>& imports,
+            const std::vector<module_import>& imports,
             const std::shared_ptr<symbol_table>& globals, const target_info& target,
             const std::filesystem::path& module_path);
 
@@ -257,7 +267,7 @@ namespace elna::boot
                 return result;
             }
             auto& tree = std::get<std::unique_ptr<unit>>(parsed);
-            std::vector<std::shared_ptr<symbol_table>> imports;
+            std::vector<module_import> imports;
             diagnostic_list circular;
 
             if (imported && tree->entry_point.has_value())
@@ -289,7 +299,10 @@ namespace elna::boot
                 {
                     result.append(std::move(diagnostics.module), std::move(diagnostics.errors()));
                 }
-                imports.push_back(this->cache.find(module_path)->second.exported_symbols());
+                imports.push_back({
+                    .position = sub_tree->position(),
+                    .symbols = this->cache.find(module_path)->second.exported_symbols()
+                });
             }
             result.append(key, std::move(circular));
             analysis_result analysis = analyze_semantics(tree, imports, this->globals, target, key);
diff --git a/include/elna/boot/symbol.h b/include/elna/boot/symbol.h
index 5bbe2c6..a5f7ca8 100644
--- a/include/elna/boot/symbol.h
+++ b/include/elna/boot/symbol.h
@@ -19,7 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #include <cstdint>
 #include <filesystem>
-#include <forward_list>
+#include <map>
 #include <memory>
 #include <optional>
 #include <string>
@@ -300,11 +300,13 @@ namespace elna::boot
             std::is_convertible_v<U, T> || std::is_assignable_v<T, U>,
             T
         >;
-        using iterator = std::unordered_map<std::string, symbol_ptr>::iterator;
-        using const_iterator = std::unordered_map<std::string, symbol_ptr>::const_iterator;
+        using iterator = std::map<std::string, symbol_ptr>::iterator;
+        using const_iterator = std::map<std::string, symbol_ptr>::const_iterator;
 
     private:
-        std::unordered_map<std::string, symbol_ptr> entries;
+        // Ordered, so that a pass iterating the table reports in a stable
+        // order without sorting first.
+        std::map<std::string, symbol_ptr> entries;
         std::shared_ptr<symbol_map> outer_scope;
 
     public:
@@ -528,6 +530,16 @@ namespace elna::boot
     std::shared_ptr<symbol_table> builtin_symbol_table(const target_info& target);
 
     /**
+     * A name exported by more than one imported module.
+     */
+    struct import_collision
+    {
+        std::string name;
+        /// The symbol imported first, which keeps the name.
+        std::shared_ptr<info> original;
+    };
+
+    /**
      * Symbol bag contains:
      *
      * - the symbol table of a module itself
@@ -537,7 +549,7 @@ namespace elna::boot
     class symbol_bag
     {
         std::shared_ptr<symbol_table> symbols;
-        std::forward_list<std::shared_ptr<symbol_table>> imports;
+        std::vector<std::shared_ptr<symbol_table>> imports;
         forward_table unresolved;
 
         std::shared_ptr<info> lookup_import(const std::string& name) const;
@@ -619,11 +631,15 @@ namespace elna::boot
         bool forward_declare(const std::string& symbol_name, std::shared_ptr<alias_type> forward_declaration);
 
         /**
-         * Add imported symbols to the scope.
+         * Add imported symbols to the scope. The symbols are appended, so an
+         * earlier import keeps its names.
          *
          * \param symbols Exported symbol table of another module.
+         *
+         * \return Names an earlier import already brought into the scope,
+         *         sorted by name.
          */
-        void add_import(const std::shared_ptr<symbol_table>& symbols);
+        std::vector<import_collision> add_import(const std::shared_ptr<symbol_table>& symbols);
 
         /**
          * Tells whether the current scope is the module global scope.
diff --git a/testsuite/fail_compilation/import_import_collision/first.elna b/testsuite/fail_compilation/import_import_collision/first.elna
new file mode 100644
index 0000000..e60ca8e
--- /dev/null
+++ b/testsuite/fail_compilation/import_import_collision/first.elna
@@ -0,0 +1,4 @@
+var
+  X*: Int := 1
+
+end.
diff --git a/testsuite/fail_compilation/import_import_collision/second.elna b/testsuite/fail_compilation/import_import_collision/second.elna
new file mode 100644
index 0000000..0a3d8e0
--- /dev/null
+++ b/testsuite/fail_compilation/import_import_collision/second.elna
@@ -0,0 +1,4 @@
+var
+  X*: Int := 2
+
+end.
diff --git a/testsuite/fail_compilation/import_import_collision/sut.elna b/testsuite/fail_compilation/import_import_collision/sut.elna
new file mode 100644
index 0000000..208aa21
--- /dev/null
+++ b/testsuite/fail_compilation/import_import_collision/sut.elna
@@ -0,0 +1,3 @@
+import first, second (* @Error Symbol 'X' has been already defined *)
+
+end.