Reject declarations shadowing imports

9 files changed, 187 insertions, 62 deletions

diff --git a/include/elna/boot/ast.h b/include/elna/boot/ast.h
index c6a0828..f959936 100644
--- a/include/elna/boot/ast.h
+++ b/include/elna/boot/ast.h
@@ -201,7 +201,7 @@ namespace elna::boot
      * override only the node types they care about, with the guarantee that
      * unimplemented nodes are never silently ignored.
      */
-    class empty_visitor : public parser_visitor
+    class empty_visitor : public virtual parser_visitor
     {
     public:
         [[noreturn]] void visit(array_type_expression *) override;
@@ -249,7 +249,7 @@ namespace elna::boot
     /**
      * Abstract visitor that visits all nodes recursively.
      */
-    class walking_visitor : public parser_visitor
+    class walking_visitor : public virtual parser_visitor
     {
     public:
         void visit(array_type_expression *) override;
diff --git a/include/elna/boot/dependency.h b/include/elna/boot/dependency.h
index 028dcf1..18df64c 100644
--- a/include/elna/boot/dependency.h
+++ b/include/elna/boot/dependency.h
@@ -25,37 +25,83 @@ along with GCC; see the file COPYING3.  If not see
 #include <filesystem>
 #include <fstream>
 #include <unordered_set>
+#include <variant>
+#include <vector>
 
 namespace elna::boot
 {
     /**
-     * Result of a compilation step: a possibly incomplete \c value together
-     * with the diagnostics produced so far.
-     *
-     * When \c errors is non-empty, the value may be missing or incomplete;
-     * how much of it is usable is defined per instantiation.
+     * Result of the module read step: the abstract syntax tree of the
+     * module, or the diagnostics that prevented it. The tree is present
+     * exactly when the read succeeded.
      */
-    template<typename T>
-    struct outcome
+    using read_result = std::variant<std::unique_ptr<unit>, diagnostic_list>;
+
+    /**
+     * Diagnostics produced while compiling one module, paired with the
+     * module path they were produced in.
+     */
+    struct module_diagnostics : diagnostic_container
     {
-        T value;
-        diagnostic_list errors;
+        std::filesystem::path module;
+
+        module_diagnostics(std::filesystem::path module, diagnostic_list errors)
+            : diagnostic_container(std::move(errors)), module(std::move(module))
+        {
+        }
     };
 
     /**
-     * Abstract syntax tree and diagnostics of a module compilation step.
-     *
-     * The tree is non-null if and only if the module itself was processed
-     * successfully and may be used further. Depending on the step, the
-     * diagnostics may also include the diagnostics of the imported modules.
+     * Result of compiling a module and its transitive imports: the AST of
+     * the module itself and the diagnostics of every module compiled along
+     * the way, grouped by the module that produced them, imports first.
      */
-    using dependency = outcome<std::unique_ptr<unit>>;
+    struct dependency
+    {
+        std::unique_ptr<unit> value;
+        std::vector<module_diagnostics> errors;
+
+        /**
+         * Appends the diagnostics of one module, merging them with the
+         * previous entry when it belongs to the same module.
+         *
+         * \param module Module the diagnostics were produced in.
+         * \param errors Diagnostics to append. An empty list is ignored.
+         */
+        void append(std::filesystem::path module, diagnostic_list errors)
+        {
+            if (errors.empty())
+            {
+                return;
+            }
+            if (!this->errors.empty() && this->errors.back().module == module)
+            {
+                auto& existing = this->errors.back().errors();
+                for (auto& error : errors)
+                {
+                    existing.push_back(std::move(error));
+                }
+            }
+            else
+            {
+                this->errors.emplace_back(std::move(module), std::move(errors));
+            }
+        }
+    };
 
     /**
      * Module scope and diagnostics produced by the semantic analysis of one
-     * module. The bag is a valid partial scope even when the analysis failed.
+     * module.
+     *
+     * The bag is a valid partial scope even when the analysis failed: the
+     * symbols collected before the failure are still used by the importing
+     * modules.
      */
-    using analysis_result = outcome<symbol_bag>;
+    struct analysis_result
+    {
+        symbol_bag value;
+        diagnostic_list errors;
+    };
 
     /**
      * An import that closes a cycle, e.g. a module importing itself.
@@ -77,9 +123,9 @@ namespace elna::boot
      * \param entry_point Module source.
      * \param target Target machine information.
      *
-     * \return Parsed module.
+     * \return Parsed module, or the diagnostics that prevented it.
      */
-    dependency read_source(std::istream& entry_point, const target_info& target);
+    read_result read_source(std::istream& entry_point, const target_info& target);
 
     /**
      * Turns the import declaration into a relative module path, appending the
@@ -102,12 +148,15 @@ namespace elna::boot
      * \param imports Exported symbols of the imported modules.
      * \param globals Global (builtin) symbols.
      * \param target Target machine information.
+     * \param module_path Path of the module being analyzed, recorded in the
+     *        symbol infos for diagnostics.
      *
      * \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::shared_ptr<symbol_table>& globals, const target_info& target);
+            const std::shared_ptr<symbol_table>& globals, const target_info& target,
+            const std::filesystem::path& module_path);
 
     /**
      * Host interface for \c dependency_state::compile.
@@ -123,8 +172,8 @@ namespace elna::boot
             const std::filesystem::path& key,
             const std::shared_ptr<symbol_table>& module_scope)
     {
-        { loader.resolve(relative, position) } -> std::same_as<std::filesystem::path>;
-        { loader.read(key) } -> std::same_as<dependency>;
+        { loader.resolve(relative, position, key) } -> std::same_as<std::filesystem::path>;
+        { loader.read(key) } -> std::same_as<read_result>;
         loader.finalize(key, module_scope);
     };
 
@@ -170,8 +219,8 @@ namespace elna::boot
          * \param target Target machine information.
          *
          * \return Abstract syntax tree of the module itself and the
-         *         diagnostics of this module and all its imports (imports
-         *         first).
+         *         diagnostics of this module and all its imports, grouped by
+         *         the module that produced them, imports first.
          */
         template<module_loader Loader>
         dependency compile(const std::filesystem::path& key, Loader& loader,
@@ -180,55 +229,58 @@ namespace elna::boot
             auto cached = this->cache.find(key);
             if (cached != this->cache.cend())
             {
-                return { .value = nullptr, .errors = {} };
+                return {};
             }
             this->in_progress.insert(key);
-            dependency outcome = loader.read(key);
+            read_result parsed = loader.read(key);
+            dependency result;
 
-            if (!outcome.errors.empty())
+            if (diagnostic_list *read_errors = std::get_if<diagnostic_list>(&parsed))
             {
-                diagnostic_list errors;
-                std::swap(errors, outcome.errors);
                 this->cache.insert({ key, symbol_bag({}, this->globals) });
                 this->in_progress.erase(key);
-                return { .value = nullptr, .errors = std::move(errors) };
+                result.append(key, std::move(*read_errors));
+                return result;
             }
-            diagnostic_list errors;
+            auto& tree = std::get<std::unique_ptr<unit>>(parsed);
             std::vector<std::shared_ptr<symbol_table>> imports;
+            diagnostic_list circular;
 
-            for (const import_declaration* sub_tree : outcome.value->imports)
+            for (const import_declaration* sub_tree : tree->imports)
             {
                 const std::filesystem::path module_path = loader.resolve(
-                        build_path(sub_tree->segments), sub_tree->position());
+                        build_path(sub_tree->segments), sub_tree->position(), key);
 
                 if (this->in_progress.contains(module_path))
                 {
-                    errors.push_back(std::make_unique<circular_import_error>(sub_tree->position(),
+                    circular.push_back(std::make_unique<circular_import_error>(sub_tree->position(),
                             module_path.stem().string()));
                     continue;
                 }
                 dependency sub = this->compile(module_path, loader, target);
-                for (auto& error : sub.errors)
+                for (auto& diagnostics : sub.errors)
                 {
-                    errors.push_back(std::move(error));
+                    result.append(std::move(diagnostics.module), std::move(diagnostics.errors()));
                 }
                 imports.push_back(this->cache.find(module_path)->second.exported_symbols());
             }
-            analysis_result result = analyze_semantics(outcome.value, imports, this->globals, target);
-            const bool failed = !result.errors.empty();
+            result.append(key, std::move(circular));
+            analysis_result analysis = analyze_semantics(tree, imports, this->globals, target, key);
+            const bool failed = !analysis.errors.empty();
 
-            for (auto& error : result.errors)
+            if (failed)
             {
-                errors.push_back(std::move(error));
+                result.append(key, std::move(analysis.errors));
             }
-            this->cache.insert({ key, result.value });
-            loader.finalize(key, result.value.leave());
+            this->cache.insert({ key, analysis.value });
+            loader.finalize(key, analysis.value.leave());
             this->in_progress.erase(key);
             if (failed)
             {
-                outcome.value.reset();
+                tree.reset();
             }
-            return { .value = std::move(outcome.value), .errors = std::move(errors) };
+            result.value = std::move(tree);
+            return result;
         }
     };
 }
diff --git a/include/elna/boot/evaluator.h b/include/elna/boot/evaluator.h
index dd70483..35cca1a 100644
--- a/include/elna/boot/evaluator.h
+++ b/include/elna/boot/evaluator.h
@@ -45,7 +45,7 @@ namespace elna::boot
         non_constant_expression_error(const source_position position, payload_type payload);
 
         std::string what() const override;
-        std::optional<std::pair<std::string, source_position>> note() const override;
+        std::optional<diagnostic_note> note() const override;
 
     private:
         payload_type payload;
diff --git a/include/elna/boot/name_analysis.h b/include/elna/boot/name_analysis.h
index f2e2eb9..a2e9437 100644
--- a/include/elna/boot/name_analysis.h
+++ b/include/elna/boot/name_analysis.h
@@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "elna/boot/symbol.h"
 #include "elna/boot/evaluator.h"
 
+#include <filesystem>
 #include <string>
 #include <memory>
 #include <optional>
@@ -46,13 +47,18 @@ namespace elna::boot
         struct redefinition
         {
             std::optional<source_position> original;
+            /**
+             * Module the original is declared in. Empty when it is the same
+             * module as the error.
+             */
+            std::filesystem::path file;
         };
         using payload_type = std::variant<redefinition, kind>;
 
         declaration_error(const source_position position, const std::string& name, payload_type payload);
 
         std::string what() const override;
-        std::optional<std::pair<std::string, source_position>> note() const override;
+        std::optional<diagnostic_note> note() const override;
 
     private:
         std::string name;
@@ -78,7 +84,7 @@ namespace elna::boot
         using payload_type = std::variant<not_initialized, kind>;
 
         const_qualifier_error(const source_position position, payload_type payload);
-        std::optional<std::pair<std::string, source_position>> note() const override;
+        std::optional<diagnostic_note> note() const override;
 
         std::string what() const override;
 
@@ -103,7 +109,7 @@ namespace elna::boot
                 const type& composite, payload_type payload);
 
         std::string what() const override;
-        std::optional<std::pair<std::string, source_position>> note() const override;
+        std::optional<diagnostic_note> note() const override;
 
     private:
         std::string name;
@@ -128,6 +134,8 @@ namespace elna::boot
         type current_type;
         symbol_bag bag;
         evaluator constant_evaluator;
+        /// Path of the module being analyzed, recorded in the symbol infos.
+        std::filesystem::path module_file;
 
         std::pair<procedure_type, std::vector<std::string>> build_procedure(
                 procedure_type_expression& expression);
@@ -136,13 +144,24 @@ namespace elna::boot
         std::shared_ptr<variable_info> register_variable(const std::string& name,
                 const bool is_extern, const source_position position);
 
+        /**
+         * File of \p original for a redefinition note.
+         *
+         * \param original Symbol the error redefines.
+         * \return Module of the original declaration; empty when it is in the
+         *         module being analyzed, so the note renders under the
+         *         error's own file.
+         */
+        std::filesystem::path redefinition_file(const std::shared_ptr<info>& original) const;
+
         type lookup_primitive_type(const std::string& name);
         type lookup_field(const type& composite_type, const std::string& field_name);
         std::optional<type> lookup_pointer_like_field(const std::string& field_name,
                 const type& element_type);
 
     public:
-        name_analysis_visitor(symbol_bag bag, const target_info& target);
+        name_analysis_visitor(symbol_bag bag, const target_info& target,
+                std::filesystem::path module_file);
 
         void visit(array_type_expression *expression) override;
         void visit(slice_type_expression *expression) override;
diff --git a/include/elna/boot/result.h b/include/elna/boot/result.h
index b34c4d4..eacc072 100644
--- a/include/elna/boot/result.h
+++ b/include/elna/boot/result.h
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #include <cstddef>
 #include <cstdint>
+#include <filesystem>
 #include <functional>
 #include <string>
 #include <deque>
@@ -79,6 +80,21 @@ namespace elna::boot
     };
 
     /**
+     * Supplementary context shown alongside a primary diagnostic, e.g. the
+     * location of a previous declaration.
+     */
+    struct diagnostic_note
+    {
+        std::string message;
+        source_position position;
+        /**
+         * Names the module the \c position belongs to. When empty, the position
+         * is in the same module as the primary diagnostic.
+         */
+        std::filesystem::path file;
+    };
+
+    /**
      * A compilation error consists of an error message and position.
      */
     class diagnostic
@@ -98,9 +114,9 @@ namespace elna::boot
          * Supplementary context shown alongside the primary error,
          * e.g.\ the location of a previous declaration.
          *
-         * \return Optional note message and position.
+         * \return Optional note.
          */
-        virtual std::optional<std::pair<std::string, source_position>> note() const
+        virtual std::optional<diagnostic_note> note() const
         {
             return std::nullopt;
         }
@@ -115,8 +131,25 @@ namespace elna::boot
 
         diagnostic_container() = default;
 
+        /**
+         * Adopts a ready-made error list.
+         *
+         * \param errors Error list to take over.
+         */
+        explicit diagnostic_container(diagnostic_list errors)
+            : m_errors(std::move(errors))
+        {
+        }
+
     public:
+        diagnostic_container(const diagnostic_container&) = delete;
+        // deque's move constructor is not noexcept in all standard library
+        // implementations, so the implicitly declared move is not either,
+        // and derived types would fall back to the deleted copy.
+        diagnostic_container(diagnostic_container&&) noexcept = default;
+
         diagnostic_list& errors();
+        const diagnostic_list& errors() const;
 
         template<typename T, typename... Args>
         void add_error(Args&&... arguments)
@@ -188,12 +221,16 @@ namespace elna::boot
      *
      * \param original Source position of the previous declaration.
      * \param label Description what was declared previously.
+     * \param file Module the previous declaration was declared in. Empty if it
+     *        is in the same module as the primary error.
      * \return Error note if the position of the previous declaration is available.
      */
-    std::optional<std::pair<std::string, source_position>> previous_declaration_note(
-            const std::optional<source_position>& original, std::string_view label = "previously declared here");
+    std::optional<diagnostic_note> previous_declaration_note(
+            const std::optional<source_position>& original,
+            std::string_view label = "previously declared here",
+            const std::filesystem::path& file = {});
 
-    std::optional<std::pair<std::string, source_position>> identifier_list_note(
+    std::optional<diagnostic_note> identifier_list_note(
             const std::vector<identifier>& identifiers);
 
     /**
diff --git a/include/elna/boot/symbol.h b/include/elna/boot/symbol.h
index 46fb9e2..5f0de28 100644
--- a/include/elna/boot/symbol.h
+++ b/include/elna/boot/symbol.h
@@ -18,6 +18,7 @@ along with GCC; see the file COPYING3.  If not see
 #pragma once
 
 #include <cstdint>
+#include <filesystem>
 #include <forward_list>
 #include <memory>
 #include <optional>
@@ -208,6 +209,8 @@ namespace elna::boot
     public:
         bool exported{ false };
         std::optional<source_position> position;
+        /// Module the symbol is declared in. Empty for builtin symbols.
+        std::filesystem::path file;
 
         virtual ~info() = 0;
 
@@ -434,6 +437,8 @@ namespace elna::boot
         std::forward_list<std::shared_ptr<symbol_table>> imports;
         forward_table unresolved;
 
+        std::shared_ptr<info> lookup_import(const std::string& name) const;
+
     public:
 
         /**
diff --git a/include/elna/boot/validation.h b/include/elna/boot/validation.h
index 2b1de95..c99ce07 100644
--- a/include/elna/boot/validation.h
+++ b/include/elna/boot/validation.h
@@ -33,7 +33,7 @@ namespace elna::boot
         validation_error(const source_position position, source_position first);
 
         std::string what() const override;
-        std::optional<std::pair<std::string, source_position>> note() const override;
+        std::optional<diagnostic_note> note() const override;
 
     private:
         source_position first;
diff --git a/include/elna/gcc/elna-diagnostic.h b/include/elna/gcc/elna-diagnostic.h
index 10d5b44..858e21e 100644
--- a/include/elna/gcc/elna-diagnostic.h
+++ b/include/elna/gcc/elna-diagnostic.h
@@ -26,7 +26,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "diagnostic.h"
 
-#include "elna/boot/result.h"
+#include "elna/boot/dependency.h"
 
 namespace elna::gcc
 {
@@ -43,5 +43,15 @@ namespace elna::gcc
 
     location_t get_location(const boot::source_position *position);
     location_t make_range(const boot::source_position& position);
-    void report_errors(const std::deque<std::unique_ptr<boot::diagnostic>>& errors);
+
+    /**
+     * Reports the diagnostics of one module.
+     *
+     * Enters the module into the line map so that every position renders
+     * with the module's file, and leaves it afterwards. Notes pointing into
+     * other modules are rendered under their own file via a line map rename.
+     *
+     * \param diagnostics Diagnostics of one module paired with its path.
+     */
+    void report_errors(const boot::module_diagnostics& diagnostics);
 }
diff --git a/include/elna/gcc/elna-module-loader.h b/include/elna/gcc/elna-module-loader.h
index 04303a3..2b46fc5 100644
--- a/include/elna/gcc/elna-module-loader.h
+++ b/include/elna/gcc/elna-module-loader.h
@@ -56,20 +56,22 @@ namespace elna::gcc
          * \param relative Module path built from the import declaration.
          * \param position Import declaration position for the ambiguity
          *        diagnostic.
+         * \param module Module being compiled. The import declaration belongs
+         *        to it, so the ambiguity diagnostic renders under its file.
          *
          * \return The resolved module path, canonical if found.
          */
         static std::filesystem::path resolve(const std::filesystem::path& relative,
-                const boot::source_position& position);
+                const boot::source_position& position, const std::filesystem::path& module);
 
         /**
          * Opens the module source and parses it.
          *
          * \param key Resolved module path.
          *
-         * \return Parsed module.
+         * \return Parsed module, or the diagnostics that prevented it.
          */
-        static boot::dependency read(const std::filesystem::path& key);
+        static boot::read_result read(const std::filesystem::path& key);
 
         /**
          * Registers the analyzed module scope with the GCC symbol table.