From cbbf958d1cedc84416e786130490f4c700c2f320 Mon Sep 17 00:00:00 2001
From: Stephane Del Pino <stephane.delpino44@gmail.com>
Date: Wed, 24 Jul 2019 14:47:47 +0200
Subject: [PATCH] Add location of variable creation to attributes of symbol
 tale

Additionally store position of the end of the declaration instruction so that
when searching a symbol in a table one also check that the declaration point has
been passed.

This fix issues generated by the storage of symbol tables within node
description. The following code is now interpreted correctly

``
Z x = 1;
{
  Z x = x + 1;
  cout << x << "\n";
}
cout << x << "\n";
``

It now produces the expected output

``
2
1
``
---
 .../ASTNodeAffectationExpressionBuilder.cpp   |  4 +-
 src/language/ASTNodeDataTypeBuilder.cpp       |  4 +-
 src/language/ASTNodeDataVariant.hpp           |  2 +-
 src/language/ASTNodeExpressionBuilder.cpp     |  2 +-
 .../ASTNodeIncDecExpressionBuilder.cpp        |  2 +-
 .../ASTSymbolInitializationChecker.cpp        |  6 +--
 src/language/ASTSymbolTableBuilder.cpp        |  4 +-
 src/language/PugsParser.cpp                   |  1 -
 src/language/SymbolTable.hpp                  | 40 ++++++++++++++++---
 tests/test_ASTSymbolInitializationChecker.cpp | 20 ++++++----
 tests/test_SymbolTable.cpp                    | 31 +++++++++-----
 11 files changed, 82 insertions(+), 34 deletions(-)

diff --git a/src/language/ASTNodeAffectationExpressionBuilder.cpp b/src/language/ASTNodeAffectationExpressionBuilder.cpp
index d6aefe532..690ddabcf 100644
--- a/src/language/ASTNodeAffectationExpressionBuilder.cpp
+++ b/src/language/ASTNodeAffectationExpressionBuilder.cpp
@@ -120,7 +120,7 @@ class AffectationProcessor final : public INodeProcessor
   {
     if constexpr (_is_defined) {
       const std::string& symbol = m_node.children[0]->string();
-      auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol);
+      auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol, m_node.children[0]->begin());
       Assert(found);
       p_value = &i_symbol->second.value();
     } else {
@@ -168,7 +168,7 @@ class AffectationToStringProcessor final : public INodeProcessor
   {
     if constexpr (_is_defined) {
       const std::string& symbol = m_node.children[0]->string();
-      auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol);
+      auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol, m_node.children[0]->begin());
       Assert(found);
       p_value = &i_symbol->second.value();
     } else {
diff --git a/src/language/ASTNodeDataTypeBuilder.cpp b/src/language/ASTNodeDataTypeBuilder.cpp
index b7e22e626..088eada9b 100644
--- a/src/language/ASTNodeDataTypeBuilder.cpp
+++ b/src/language/ASTNodeDataTypeBuilder.cpp
@@ -47,14 +47,14 @@ ASTNodeDataTypeBuilder::_buildNodeDataTypes(ASTNode& n)
 
         std::shared_ptr<SymbolTable>& symbol_table = n.m_symbol_table;
 
-        auto [i_symbol, found] = symbol_table->find(symbol);
+        auto [i_symbol, found] = symbol_table->find(symbol, n.children[1]->begin());
         Assert(found);
         i_symbol->second.setDataType(data_type);
         n.m_data_type = data_type;
       } else if (n.is<language::name>()) {
         std::shared_ptr<SymbolTable>& symbol_table = n.m_symbol_table;
 
-        auto [i_symbol, found] = symbol_table->find(n.string());
+        auto [i_symbol, found] = symbol_table->find(n.string(), n.begin());
         Assert(found);
         n.m_data_type = i_symbol->second.dataType();
       }
diff --git a/src/language/ASTNodeDataVariant.hpp b/src/language/ASTNodeDataVariant.hpp
index 8572eb5fa..6a3821729 100644
--- a/src/language/ASTNodeDataVariant.hpp
+++ b/src/language/ASTNodeDataVariant.hpp
@@ -5,4 +5,4 @@
 
 using ASTNodeDataVariant = std::variant<std::monostate, bool, uint64_t, int64_t, double, std::string>;
 
-#endif // AST_NODE_DATA_VARIANT_HPP
+#endif   // AST_NODE_DATA_VARIANT_HPP
diff --git a/src/language/ASTNodeExpressionBuilder.cpp b/src/language/ASTNodeExpressionBuilder.cpp
index fd6d3fca6..bb56d9b78 100644
--- a/src/language/ASTNodeExpressionBuilder.cpp
+++ b/src/language/ASTNodeExpressionBuilder.cpp
@@ -193,7 +193,7 @@ class NameExpression final : public INodeProcessor
   NameExpression(ASTNode& node) : m_node{node}
   {
     const std::string& symbol = m_node.string();
-    auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol);
+    auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol, m_node.begin());
     Assert(found);
     p_value = &(i_symbol->second.value());
   }
diff --git a/src/language/ASTNodeIncDecExpressionBuilder.cpp b/src/language/ASTNodeIncDecExpressionBuilder.cpp
index fac93f761..ec11df799 100644
--- a/src/language/ASTNodeIncDecExpressionBuilder.cpp
+++ b/src/language/ASTNodeIncDecExpressionBuilder.cpp
@@ -74,7 +74,7 @@ class IncDecExpressionProcessor final : public INodeProcessor
       Assert(m_node.children[0]->is<language::name>());
       // It is sure at this point that children 0 is a variable name
       const std::string& symbol = m_node.children[0]->string();
-      auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol);
+      auto [i_symbol, found]    = m_node.m_symbol_table->find(symbol, m_node.children[0]->begin());
       Assert(found);
       p_value = &i_symbol->second.value();
     } else {
diff --git a/src/language/ASTSymbolInitializationChecker.cpp b/src/language/ASTSymbolInitializationChecker.cpp
index 2763db1ad..06b0e9975 100644
--- a/src/language/ASTSymbolInitializationChecker.cpp
+++ b/src/language/ASTSymbolInitializationChecker.cpp
@@ -9,7 +9,7 @@ ASTSymbolInitializationChecker::_checkSymbolInitialization(ASTNode& node)
 {
   if (node.is<language::declaration>()) {
     const std::string& symbol = node.children[1]->string();
-    auto [i_symbol, found]    = node.m_symbol_table->find(symbol);
+    auto [i_symbol, found]    = node.m_symbol_table->find(symbol, node.children[1]->begin());
     Assert(found, "unexpected error, should have been detected through declaration checking");
     if (node.children.size() == 3) {
       this->_checkSymbolInitialization(*node.children[2]);
@@ -20,11 +20,11 @@ ASTSymbolInitializationChecker::_checkSymbolInitialization(ASTNode& node)
     this->_checkSymbolInitialization(*node.children[1]);
     // then marks left hand side as initialized
     const std::string& symbol = node.children[0]->string();
-    auto [i_symbol, found]    = node.m_symbol_table->find(symbol);
+    auto [i_symbol, found]    = node.m_symbol_table->find(symbol, node.children[0]->begin());
     Assert(found, "unexpected error, should have been detected through declaration checking");
     i_symbol->second.setIsInitialized();
   } else if (node.is<language::name>()) {
-    auto [i_symbol, found] = node.m_symbol_table->find(node.string());
+    auto [i_symbol, found] = node.m_symbol_table->find(node.string(), node.begin());
     Assert(found, "unexpected error, should have been detected through declaration checking");
     if (not i_symbol->second.isInitialized()) {
       std::ostringstream error_message;
diff --git a/src/language/ASTSymbolTableBuilder.cpp b/src/language/ASTSymbolTableBuilder.cpp
index ef16c6894..205e5be56 100644
--- a/src/language/ASTSymbolTableBuilder.cpp
+++ b/src/language/ASTSymbolTableBuilder.cpp
@@ -19,14 +19,14 @@ ASTSymbolTableBuilder::buildSymbolTable(ASTNode& n, std::shared_ptr<SymbolTable>
     if (n.has_content()) {
       if (n.is<language::declaration>()) {
         const std::string& symbol = n.children[1]->string();
-        auto [i_symbol, success]  = symbol_table->add(symbol);
+        auto [i_symbol, success]  = symbol_table->add(symbol, n.children[1]->begin(), n.end());
         if (not success) {
           std::ostringstream error_message;
           error_message << "symbol '" << rang::fg::red << symbol << rang::fg::reset << '\'' << " was already defined!";
           throw parse_error(error_message.str(), std::vector{n.begin()});
         }
       } else if (n.is<language::name>()) {
-        auto [i_symbol, found] = symbol_table->find(n.string());
+        auto [i_symbol, found] = symbol_table->find(n.string(), n.begin());
         if (not found) {
           std::ostringstream error_message;
           error_message << "undefined symbol '" << rang::fg::red << n.string() << rang::fg::reset << '\'';
diff --git a/src/language/PugsParser.cpp b/src/language/PugsParser.cpp
index 9f3483851..b57dfc03a 100644
--- a/src/language/PugsParser.cpp
+++ b/src/language/PugsParser.cpp
@@ -185,7 +185,6 @@ parser(const std::string& filename)
   read_input input(filename);
   try {
     root_node = ASTBuilder::build(input);
-    std::cout << " - AST is built ...... [done]\n";
 
     ASTSymbolTableBuilder{*root_node};
 
diff --git a/src/language/SymbolTable.hpp b/src/language/SymbolTable.hpp
index 420a8fd8c..570cc3ba7 100644
--- a/src/language/SymbolTable.hpp
+++ b/src/language/SymbolTable.hpp
@@ -4,6 +4,10 @@
 #include <ASTNodeDataType.hpp>
 #include <ASTNodeDataVariant.hpp>
 
+#include <pegtl/position.hpp>
+
+#include <iostream>
+
 class SymbolTable
 {
  public:
@@ -14,6 +18,9 @@ class SymbolTable
     ASTNodeDataType m_data_type{ASTNodeDataType::undefined_t};
     ASTNodeDataVariant m_value;
 
+    TAO_PEGTL_NAMESPACE::position m_position;
+    TAO_PEGTL_NAMESPACE::position m_end_of_declaration;
+
    public:
     auto&
     value()
@@ -45,6 +52,18 @@ class SymbolTable
       return m_data_type;
     }
 
+    auto
+    position() const
+    {
+      return m_position;
+    }
+
+    auto
+    endOfDeclaration() const
+    {
+      return m_end_of_declaration;
+    }
+
     void
     setDataType(const ASTNodeDataType& data_type)
     {
@@ -67,6 +86,12 @@ class SymbolTable
 
       return os;
     }
+
+    Attributes(const TAO_PEGTL_NAMESPACE::position& position, const TAO_PEGTL_NAMESPACE::position& end_of_declaration)
+      : m_position(position), m_end_of_declaration(end_of_declaration)
+    {}
+
+    Attributes(const Attributes&) = default;
   };
 
  private:
@@ -86,7 +111,7 @@ class SymbolTable
   }
 
   auto
-  find(const std::string& symbol)
+  find(const std::string& symbol, const TAO_PEGTL_NAMESPACE::position& use_position)
   {
     auto i_symbol = m_symbol_list.end();
 
@@ -97,11 +122,12 @@ class SymbolTable
       }
     }
 
-    if (i_symbol != m_symbol_list.end()) {
+    if (i_symbol != m_symbol_list.end() and (use_position.byte > i_symbol->second.endOfDeclaration().byte or
+                                             use_position.byte == i_symbol->second.position().byte)) {
       return std::make_pair(i_symbol, true);
     } else {
       if (m_parent_table) {
-        return m_parent_table->find(symbol);
+        return m_parent_table->find(symbol, use_position);
       } else {
         return std::make_pair(i_symbol, false);
       }
@@ -109,14 +135,18 @@ class SymbolTable
   }
 
   auto
-  add(const std::string& symbol)
+  add(const std::string& symbol,
+      const TAO_PEGTL_NAMESPACE::position& symbol_position,
+      const TAO_PEGTL_NAMESPACE::position& declaration_end)
   {
     for (auto i_stored_symbol = m_symbol_list.begin(); i_stored_symbol != m_symbol_list.end(); ++i_stored_symbol) {
       if (i_stored_symbol->first == symbol) {
         return std::make_pair(i_stored_symbol, false);
       }
     }
-    return std::make_pair(m_symbol_list.emplace(m_symbol_list.end(), std::make_pair(symbol, Attributes())), true);
+    return std::make_pair(m_symbol_list.emplace(m_symbol_list.end(),
+                                                std::make_pair(symbol, Attributes(symbol_position, declaration_end))),
+                          true);
   }
 
   SymbolTable(const std::shared_ptr<SymbolTable>& parent_table = nullptr) : m_parent_table(parent_table)
diff --git a/tests/test_ASTSymbolInitializationChecker.cpp b/tests/test_ASTSymbolInitializationChecker.cpp
index 8a9caed3f..a592dfe4f 100644
--- a/tests/test_ASTSymbolInitializationChecker.cpp
+++ b/tests/test_ASTSymbolInitializationChecker.cpp
@@ -11,7 +11,7 @@ TEST_CASE("ASTSymbolInitializationChecker", "[language]")
   {
     std::string_view data = R"(
 N m = 2;
-N n = m;
+N n = m ;
 N p;
 )";
 
@@ -21,15 +21,18 @@ N p;
     ASTSymbolTableBuilder{*ast};
     ASTSymbolInitializationChecker{*ast};
 
-    auto [attribute_m, found_m] = ast->m_symbol_table->find("m");
+    position position{internal::iterator{"fixture"}, "fixture"};
+    position.byte = data.size();   // ensure that variables are declared at this point
+
+    auto [attribute_m, found_m] = ast->m_symbol_table->find("m", position);
     REQUIRE(found_m);
     REQUIRE(attribute_m->second.isInitialized());
 
-    auto [attribute_n, found_n] = ast->m_symbol_table->find("n");
+    auto [attribute_n, found_n] = ast->m_symbol_table->find("n", position);
     REQUIRE(found_n);
     REQUIRE(attribute_n->second.isInitialized());
 
-    auto [attribute_p, found_p] = ast->m_symbol_table->find("p");
+    auto [attribute_p, found_p] = ast->m_symbol_table->find("p", position);
     REQUIRE(found_p);
     REQUIRE(not attribute_p->second.isInitialized());
   }
@@ -50,15 +53,18 @@ m = n;
     ASTSymbolTableBuilder{*ast};
     ASTSymbolInitializationChecker{*ast};
 
-    auto [attribute_m, found_m] = ast->m_symbol_table->find("m");
+    position position{internal::iterator{"fixture"}, "fixture"};
+    position.byte = data.size();   // ensure that variables are declared at this point
+
+    auto [attribute_m, found_m] = ast->m_symbol_table->find("m", position);
     REQUIRE(found_m);
     REQUIRE(attribute_m->second.isInitialized());
 
-    auto [attribute_n, found_n] = ast->m_symbol_table->find("n");
+    auto [attribute_n, found_n] = ast->m_symbol_table->find("n", position);
     REQUIRE(found_n);
     REQUIRE(attribute_n->second.isInitialized());
 
-    auto [attribute_z, found_z] = ast->m_symbol_table->find("z");
+    auto [attribute_z, found_z] = ast->m_symbol_table->find("z", position);
     REQUIRE(found_z);
     REQUIRE(not attribute_z->second.isInitialized());
   }
diff --git a/tests/test_SymbolTable.cpp b/tests/test_SymbolTable.cpp
index 38b0c1c82..087003104 100644
--- a/tests/test_SymbolTable.cpp
+++ b/tests/test_SymbolTable.cpp
@@ -3,19 +3,26 @@
 #include <SymbolTable.hpp>
 #include <sstream>
 
+#include <pegtl/internal/iterator.hpp>
+
 TEST_CASE("SymbolTable", "[language]")
 {
   SECTION("Simple Symbol Table")
   {
     std::shared_ptr root_st = std::make_shared<SymbolTable>();
 
-    auto [i_symbol_a, created_a] = root_st->add("a");
+    using namespace TAO_PEGTL_NAMESPACE;
+    position begin_position{internal::iterator{"fixture"}, "fixture"};
+    position end_declaration{internal::iterator{"fixture"}, "fixture"};
+
+    auto [i_symbol_a, created_a] = root_st->add("a", begin_position, end_declaration);
     REQUIRE(created_a);
 
     // Check that one cannot build another "a" in this table
-    REQUIRE(not root_st->add("a").second);
+    REQUIRE(not root_st->add("a", begin_position, end_declaration).second);
 
-    auto [i_search_a, found_a] = root_st->find("a");
+    position use_position{internal::iterator{"fixture"}, "fixture"};
+    auto [i_search_a, found_a] = root_st->find("a", use_position);
     REQUIRE(found_a);
     REQUIRE(i_search_a == i_symbol_a);
 
@@ -83,28 +90,34 @@ TEST_CASE("SymbolTable", "[language]")
 
   SECTION("Hierarchy Symbol Table")
   {
-    std::shared_ptr root_st                = std::make_shared<SymbolTable>();
-    auto [i_root_symbol_a, created_root_a] = root_st->add("a");
+    std::shared_ptr root_st = std::make_shared<SymbolTable>();
+
+    using namespace TAO_PEGTL_NAMESPACE;
+    position begin_position{internal::iterator{"fixture"}, "fixture"};
+    position end_declaration{internal::iterator{"fixture"}, "fixture"};
+
+    auto [i_root_symbol_a, created_root_a] = root_st->add("a", begin_position, end_declaration);
     REQUIRE(created_root_a);
 
     std::shared_ptr nested_st = std::make_shared<SymbolTable>(root_st);
 
-    auto [i_search_a, found_a] = nested_st->find("a");
+    position use_position{internal::iterator{"fixture"}, "fixture"};
+    auto [i_search_a, found_a] = nested_st->find("a", use_position);
     REQUIRE(found_a);
     // symbol "a" is the one defined in root_st
     REQUIRE(i_root_symbol_a == i_search_a);
 
-    auto [i_nested_symbol_a, created_nested_a] = nested_st->add("a");
+    auto [i_nested_symbol_a, created_nested_a] = nested_st->add("a", begin_position, end_declaration);
     REQUIRE(created_nested_a);
 
-    auto [i_search_nested_a, found_nested_a] = nested_st->find("a");
+    auto [i_search_nested_a, found_nested_a] = nested_st->find("a", use_position);
 
     REQUIRE(found_nested_a);
     // found the symbol created in nested symbol table
     REQUIRE(&(i_search_nested_a->second) != &(i_root_symbol_a->second));
     REQUIRE(&(i_search_nested_a->second) == &(i_nested_symbol_a->second));
 
-    auto [i_search_b, found_b] = nested_st->find("b");
+    auto [i_search_b, found_b] = nested_st->find("b", use_position);
     REQUIRE(not found_b);   // "b" is not defined in any symbol table
   }
 }
-- 
GitLab