Reject declarations shadowing imports

6 files changed, 132 insertions, 64 deletions

diff --git a/boot/dependency.cc b/boot/dependency.cc
index 7f87e40..7e103aa 100644
--- a/boot/dependency.cc
+++ b/boot/dependency.cc
@@ -37,36 +37,36 @@ namespace elna::boot
         return "Circular import of module '" + this->module_name + "'";
     }
 
-    dependency read_source(std::istream& entry_point, const target_info& target)
+    read_result read_source(std::istream& entry_point, const target_info& target)
     {
         driver parse_driver;
         lexer tokenizer(entry_point);
         yy::parser parser(tokenizer, parse_driver);
 
-        dependency outcome;
         if (parser() != 0)
         {
-            std::swap(outcome.errors, parse_driver.errors());
-            return outcome;
-        }
-        else
-        {
-            std::swap(outcome.value, parse_driver.tree);
+            diagnostic_list errors;
+            std::swap(errors, parse_driver.errors());
+            return read_result{ std::in_place_type<diagnostic_list>, std::move(errors) };
         }
+        std::unique_ptr<unit> tree;
+        std::swap(tree, parse_driver.tree);
         materialization_visitor materialization_visitor(target);
-        outcome.value->accept(&materialization_visitor);
+        tree->accept(&materialization_visitor);
 
         if (materialization_visitor.has_errors())
         {
-            std::swap(outcome.errors, materialization_visitor.errors());
-            outcome.value.reset();
+            diagnostic_list errors;
+            std::swap(errors, materialization_visitor.errors());
+            return read_result{ std::in_place_type<diagnostic_list>, std::move(errors) };
         }
-        return outcome;
+        return read_result{ std::in_place_type<std::unique_ptr<unit>>, std::move(tree) };
     }
 
     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)
     {
         declaration_visitor declarations{};
         tree->accept(&declarations);
@@ -81,7 +81,7 @@ namespace elna::boot
         {
             result.value.add_import(import);
         }
-        name_analysis_visitor name_analyser(result.value, target);
+        name_analysis_visitor name_analyser(result.value, target, module_path);
         tree->accept(&name_analyser);
 
         if (name_analyser.has_errors())
diff --git a/boot/evaluator.cc b/boot/evaluator.cc
index 1d10346..6a9fb3b 100644
--- a/boot/evaluator.cc
+++ b/boot/evaluator.cc
@@ -51,7 +51,7 @@ namespace elna::boot
         }, this->payload);
     }
 
-    std::optional<std::pair<std::string, source_position>> non_constant_expression_error::note() const
+    std::optional<diagnostic_note> non_constant_expression_error::note() const
     {
         if (std::holds_alternative<initializer>(this->payload))
         {
@@ -297,25 +297,37 @@ namespace elna::boot
 
     std::optional<constant_value> evaluator::evaluate_field_access(field_access_expression& subject)
     {
-        auto type_to_check = subject.base().type_decoration;
-        if (type_to_check.empty())
+        // Accessing a member of an enumeration. The base is the type name,
+        // so the enumeration is looked up in the symbol table instead of
+        // reading the type decoration set by name analysis.
+        if (auto *base_designator = subject.base().is_designator())
         {
-            type_to_check = subject.type_decoration;
-        }
-        auto resolved_base = resolve_underlying_type(type_to_check);
-        if (auto enumeration = resolved_base.get<enumeration_type>())
-        {
-            auto enumeration_distance = std::distance(enumeration->members.begin(),
-                    std::ranges::find(enumeration->members, subject.field().name()));
-            const std::size_t enumeration_position = static_cast<std::size_t>(enumeration_distance);
-
-            if (enumeration_position >= enumeration->members.size())
+            if (auto *base_name = base_designator->is_named())
             {
-                return std::nullopt;
+                auto symbol = this->bag.lookup(base_name->name);
+
+                if (symbol != nullptr)
+                {
+                    if (auto type_symbol = symbol->is_type())
+                    {
+                        if (auto enumeration = resolve_underlying_type(type_symbol->symbol).get<enumeration_type>())
+                        {
+                            auto member_iterator = std::ranges::find(enumeration->members, subject.field().name());
+
+                            if (member_iterator == enumeration->members.end())
+                            {
+                                return std::nullopt;
+                            }
+                            const std::size_t enumeration_position = static_cast<std::size_t>(
+                                    std::distance(enumeration->members.begin(), member_iterator));
+
+                            return constant_value{ integer_literal::from(enumeration_position + 1U) };
+                        }
+                    }
+                }
             }
-            return constant_value{ integer_literal::from(enumeration_position + 1U) };
         }
-        else if (auto base = evaluate(subject.base()))
+        if (auto base = evaluate(subject.base()))
         {
             if (auto *record = std::get_if<constant_aggregate<ordered_map>>(&base.value()))
             {
@@ -732,19 +744,32 @@ namespace elna::boot
     {
         auto value = evaluate(subject.value());
 
-        if (!value.has_value() || subject.type_decoration.empty())
+        if (!value.has_value())
         {
             return std::nullopt;
         }
-        const type resolved = resolve_underlying_type(subject.type_decoration);
-
-        if (is_primitive_type(resolved, "Int"))
-        {
-            return cast_to_int(value.value());
-        }
-        else if (is_primitive_type(resolved, "Word"))
+        // The target type is looked up in the symbol table instead of reading
+        // the type decoration set by name analysis.
+        if (auto *target = subject.target().is_named())
         {
-            return cast_to_word(value.value());
+            auto symbol = this->bag.lookup(target->name);
+
+            if (symbol != nullptr)
+            {
+                if (auto type_symbol = symbol->is_type())
+                {
+                    const type resolved = resolve_underlying_type(type_symbol->symbol);
+
+                    if (is_primitive_type(resolved, "Int"))
+                    {
+                        return cast_to_int(value.value());
+                    }
+                    else if (is_primitive_type(resolved, "Word"))
+                    {
+                        return cast_to_word(value.value());
+                    }
+                }
+            }
         }
 
         return constant_value{ value.value() };
diff --git a/boot/name_analysis.cc b/boot/name_analysis.cc
index 30d3cda..10b8a8d 100644
--- a/boot/name_analysis.cc
+++ b/boot/name_analysis.cc
@@ -24,7 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 namespace elna::boot
 {
     declaration_error::declaration_error(const source_position position, const std::string& name, payload_type payload)
-        : diagnostic(position), name(name), payload(payload)
+        : diagnostic(position), name(name), payload(std::move(payload))
     {
     }
 
@@ -57,11 +57,12 @@ namespace elna::boot
         }, this->payload);
     }
 
-    std::optional<std::pair<std::string, source_position>> declaration_error::note() const
+    std::optional<diagnostic_note> declaration_error::note() const
     {
         if (std::holds_alternative<redefinition>(this->payload))
         {
-            return previous_declaration_note(std::get<redefinition>(this->payload).original);
+            return previous_declaration_note(std::get<redefinition>(this->payload).original,
+                    "previously declared here", std::get<redefinition>(this->payload).file);
         }
         else
         {
@@ -99,7 +100,7 @@ namespace elna::boot
         }, this->payload);
     }
 
-    std::optional<std::pair<std::string, source_position>> const_qualifier_error::note() const
+    std::optional<diagnostic_note> const_qualifier_error::note() const
     {
         if (std::holds_alternative<not_initialized>(this->payload))
         {
@@ -175,7 +176,7 @@ namespace elna::boot
         }, payload);
     }
 
-    std::optional<std::pair<std::string, source_position>> member_error::note() const
+    std::optional<diagnostic_note> member_error::note() const
     {
         if (std::holds_alternative<duplicate>(this->payload))
         {
@@ -201,8 +202,9 @@ namespace elna::boot
         }
     }
 
-    name_analysis_visitor::name_analysis_visitor(symbol_bag bag, const target_info& target)
-        :  bag(std::move(bag)), constant_evaluator(this->bag, target)
+    name_analysis_visitor::name_analysis_visitor(symbol_bag bag, const target_info& target,
+            std::filesystem::path module_file)
+        :  bag(std::move(bag)), constant_evaluator(this->bag, target), module_file(std::move(module_file))
     {
     }
 
@@ -295,7 +297,15 @@ namespace elna::boot
 
         info->exported = declaration->identifier.exported();
         info->position.emplace(declaration->position());
-        this->bag.enter(declaration->identifier.name(), info);
+        info->file = this->module_file;
+
+        if (!this->bag.enter(declaration->identifier.name(), info))
+        {
+            auto original = this->bag.lookup(declaration->identifier.name());
+            add_error<declaration_error>(declaration->identifier.id().position(), declaration->identifier.name(),
+                    declaration_error::redefinition{ .original = original->position,
+                            .file = this->redefinition_file(original) });
+        }
     }
 
     void name_analysis_visitor::visit(pointer_type_expression *expression)
@@ -542,17 +552,25 @@ namespace elna::boot
         this->current_type = type(result_type);
     }
 
+    std::filesystem::path name_analysis_visitor::redefinition_file(
+            const std::shared_ptr<info>& original) const
+    {
+        return original->file != this->module_file ? original->file : std::filesystem::path{};
+    }
+
     std::shared_ptr<variable_info> name_analysis_visitor::register_variable(const std::string& name,
             const bool is_extern, const source_position position)
     {
         auto variable_symbol = std::make_shared<variable_info>(this->current_type, is_extern);
         variable_symbol->position.emplace(position);
+        variable_symbol->file = this->module_file;
 
         if (!this->bag.enter(name, variable_symbol))
         {
             auto original = this->bag.lookup(name);
             add_error<declaration_error>(position, name,
-                    declaration_error::redefinition{ .original = original->position });
+                    declaration_error::redefinition{ .original = original->position,
+                            .file = this->redefinition_file(original) });
         }
         return variable_symbol;
     }
@@ -631,7 +649,14 @@ namespace elna::boot
         }
         info->exported = declaration->identifier.exported();
         info->position.emplace(declaration->position());
-        this->bag.enter(declaration->identifier.name(), info);
+        info->file = this->module_file;
+        if (!this->bag.enter(declaration->identifier.name(), info))
+        {
+            auto original = this->bag.lookup(declaration->identifier.name());
+            add_error<declaration_error>(declaration->identifier.id().position(), declaration->identifier.name(),
+                    declaration_error::redefinition{ .original = original->position,
+                            .file = this->redefinition_file(original) });
+        }
     }
 
     void name_analysis_visitor::visit(procedure_call *call)
@@ -666,12 +691,16 @@ namespace elna::boot
         {
             this->bag.enter();
             auto variable_type = lookup_primitive_type("Int");
-            this->bag.enter("count", std::make_shared<variable_info>(variable_type, false));
+            auto count_symbol = std::make_shared<variable_info>(variable_type, false);
+            count_symbol->file = this->module_file;
+            this->bag.enter("count", count_symbol);
 
             variable_type = lookup_primitive_type("Word8");
             variable_type = type(std::make_shared<pointer_type>(variable_type));
             variable_type = type(std::make_shared<pointer_type>(variable_type));
-            this->bag.enter("parameters", std::make_shared<variable_info>(variable_type, false));
+            auto parameters_symbol = std::make_shared<variable_info>(variable_type, false);
+            parameters_symbol->file = this->module_file;
+            this->bag.enter("parameters", parameters_symbol);
 
             for (statement *const statement : unit->entry_point)
             {
@@ -971,7 +1000,7 @@ namespace elna::boot
         {
             add_error<declaration_error>(declaration->identifier.id().position(),
                     declaration->identifier.id().name(),
-                    declaration_error::redefinition{ .original = declaration->position() });
+                    declaration_error::redefinition{ .original = declaration->position(), .file = {} });
         }
     }
 
diff --git a/boot/result.cc b/boot/result.cc
index d25b85b..635a121 100644
--- a/boot/result.cc
+++ b/boot/result.cc
@@ -75,6 +75,11 @@ namespace elna::boot
         return m_errors;
     }
 
+    const std::deque<std::unique_ptr<diagnostic>>& diagnostic_container::errors() const
+    {
+        return m_errors;
+    }
+
     bool diagnostic_container::has_errors() const
     {
         return !m_errors.empty();
@@ -131,12 +136,13 @@ namespace elna::boot
         return this->m_exported;
     }
 
-    std::optional<std::pair<std::string, source_position>> previous_declaration_note(
-            const std::optional<source_position>& original, std::string_view label)
+    std::optional<diagnostic_note> previous_declaration_note(
+            const std::optional<source_position>& original, std::string_view label,
+            const std::filesystem::path& file)
     {
         if (original.has_value() && original.value().start().available())
         {
-            return std::make_pair(std::string(label), original.value());
+            return diagnostic_note{ .message = std::string(label), .position = original.value(), .file = file };
         }
         else
         {
@@ -144,13 +150,13 @@ namespace elna::boot
         }
     }
 
-    std::optional<std::pair<std::string, source_position>> identifier_list_note(
+    std::optional<diagnostic_note> identifier_list_note(
             const std::vector<identifier>& identifiers)
     {
         auto position_span = source_position(identifiers.front().position().start(),
                 identifiers.back().position().end());
 
-        return std::make_optional(std::make_pair(join(identifiers), position_span));
+        return diagnostic_note{ .message = join(identifiers), .position = position_span, .file = {} };
     }
 
     std::vector<identifier> extract_identifiers(const std::vector<identifier_definition>& identifiers)
diff --git a/boot/symbol.cc b/boot/symbol.cc
index 8855d52..7ef09c9 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 <ranges>
 #include <cassert>
 #include <utility>
 
@@ -356,21 +357,28 @@ namespace elna::boot
         this->symbols = std::make_shared<symbol_table>(global_table);
     }
 
+    std::shared_ptr<info> symbol_bag::lookup_import(const std::string& name) const
+    {
+        const auto found = std::ranges::find_if(this->imports,
+            [&name](const std::shared_ptr<symbol_table>& import_bag) {
+                return import_bag->lookup(name) != nullptr;
+            }
+        );
+        return found == this->imports.cend() ? nullptr : (*found)->lookup(name);
+    }
+
     std::shared_ptr<info> symbol_bag::lookup(const std::string& name)
     {
-        for (const auto& import_bag : this->imports)
+        if (auto result = this->lookup_import(name))
         {
-            if (auto result = import_bag->lookup(name))
-            {
-                return result;
-            }
+            return result;
         }
         return this->symbols->lookup(name);
     }
 
     bool symbol_bag::enter(const std::string& name, const std::shared_ptr<info>& entry)
     {
-        return this->symbols->enter(name, entry);
+        return this->lookup_import(name) == nullptr && this->symbols->enter(name, entry);
     }
 
     std::shared_ptr<symbol_table> symbol_bag::enter()
diff --git a/boot/validation.cc b/boot/validation.cc
index a81282b..b046b4e 100644
--- a/boot/validation.cc
+++ b/boot/validation.cc
@@ -31,7 +31,7 @@ namespace elna::boot
         return "Duplicate case label";
     }
 
-    std::optional<std::pair<std::string, source_position>> validation_error::note() const
+    std::optional<diagnostic_note> validation_error::note() const
     {
         return previous_declaration_note(this->first, "Previous label here");
     }