Throw eval exception if a variable or function is redefined #28

include/chaiscript/dispatchkit/bootstrap.hpp

@@ -422,9 +422,6 @@ namespace chaiscript
         m->add(fun(&Type_Info::name), "cpp_name");
         m->add(fun(&Type_Info::bare_name), "cpp_bare_name");
         m->add(fun(&Type_Info::bare_equal), "bare_equal");
-        typedef bool (Type_Info::*typeinfocompare)(const Type_Info &) const;
-        m->add(fun(typeinfocompare(&Type_Info::operator==)), "==");
-        m->add(fun(&Type_Info::bare_equal), "bare_equal");
 
 
         basic_constructors<bool>("bool", m);

include/chaiscript/dispatchkit/dispatchkit.hpp

@@ -35,10 +35,7 @@ namespace chaiscript
   namespace exception
   {
     /**
-     * Exception thrown in the case that a multi method dispatch fails
+     * Exception thrown in the case that an object name is invalid because it is a reserved word
-     * because no matching function was found
-     * at runtime due to either an arity_error, a guard_error or a bad_boxed_cast
-     * exception
      */
     class reserved_word_error : public std::runtime_error
     {
@@ -60,6 +57,30 @@ namespace chaiscript
 
     };
 
+    /**
+     * Exception thrown in the case that an object name is invalid because it already exists in current context
+     */
+    class name_conflict_error : public std::runtime_error
+    {
+      public:
+        name_conflict_error(const std::string &t_name) throw()
+          : std::runtime_error("Name already exists in current context " + t_name), m_name(t_name)
+        {
+        }
+
+        virtual ~name_conflict_error() throw() {}
+
+        std::string name() const
+        {
+          return m_name;
+        }
+
+      private:
+        std::string m_name;
+
+    };
+
+
     /**
      * Exception thrown in the case that a non-const object was added as a shared object
      */
@@ -146,7 +167,12 @@ namespace chaiscript
         {
           while (begin != end)
           {
-            t.add(begin->first, begin->second);
+            try {
+              t.add(begin->first, begin->second);
+            } catch (const exception::name_conflict_error &) {
+              /// \todo Should we throw an error if there's a name conflict 
+              ///       while applying a module?
+            }
             ++begin;
           }
         }
@@ -378,16 +404,17 @@ namespace chaiscript
         /**
          * Add a new named Proxy_Function to the system
          */
-        bool add(const Proxy_Function &f, const std::string &name)
+        void add(const Proxy_Function &f, const std::string &name)
         {
           validate_object_name(name);
-          return add_function(f, name);
+          add_function(f, name);
         }
 
         /**
          * Set the value of an object, by name. If the object
          * is not available in the current scope it is created
          */
+        /*
         void add(const Boxed_Value &obj, const std::string &name)
         {
           validate_object_name(name);
@@ -405,6 +432,7 @@ namespace chaiscript
 
           add_object(name, obj);
         }
+        */
 
         /**
          * Adds a named object to the current scope
@@ -413,7 +441,15 @@ namespace chaiscript
         {
           StackData &stack = get_stack_data();
           validate_object_name(name);
-          stack.back()[name] = obj;
+          
+          Scope &scope = stack.back();
+          Scope::iterator itr = scope.find(name);
+          if (itr != stack.back().end())
+          {
+            throw exception::name_conflict_error(name);
+          } else {
+            stack.back().insert(std::make_pair(name, obj));
+          }
         }
 
         /**
@@ -429,7 +465,12 @@ namespace chaiscript
 
           chaiscript::detail::threading::unique_lock<chaiscript::detail::threading::shared_mutex> l(m_global_object_mutex);
 
-          m_state.m_global_objects[name] = obj;
+          if (m_state.m_global_objects.find(name) != m_state.m_global_objects.end())
+          {
+            throw exception::name_conflict_error(name);
+          } else {
+            m_state.m_global_objects.insert(std::make_pair(name, obj));
+          }
         }
 
         /**
@@ -936,11 +977,10 @@ namespace chaiscript
         }
 
         /**
-         * Implementation detail for adding a function. Returns
+         * Implementation detail for adding a function. 
-         * true if the function was added, false if a function with the
+         * \throws exception::name_conflict_error if there's a function matching the given one being added
-         * same signature and name already exists.
          */
-        bool add_function(const Proxy_Function &t_f, const std::string &t_name)
+        void add_function(const Proxy_Function &t_f, const std::string &t_name)
         {
           chaiscript::detail::threading::unique_lock<chaiscript::detail::threading::shared_mutex> l(m_mutex);
 
@@ -958,7 +998,7 @@ namespace chaiscript
             {
               if ((*t_f) == *(*itr2))
               {
-                return false;
+                throw exception::name_conflict_error(t_name);
               }
             }
 
@@ -969,8 +1009,6 @@ namespace chaiscript
             vec.push_back(t_f);
             funcs.insert(std::make_pair(t_name, vec));
           }
-
-          return true;
         }
 
         mutable chaiscript::detail::threading::shared_mutex m_mutex;

include/chaiscript/dispatchkit/proxy_functions.hpp

@@ -219,7 +219,12 @@ namespace chaiscript
 
         virtual bool operator==(const Proxy_Function_Base &rhs) const
         {
-          return this == &rhs;
+          const Dynamic_Proxy_Function *prhs = dynamic_cast<const Dynamic_Proxy_Function *>(&rhs);
+
+          return this == &rhs
+            || (prhs
+                && this->m_arity == prhs->m_arity
+                && !this->m_guard && !prhs->m_guard);
         }
 
         virtual bool call_match(const std::vector<Boxed_Value> &vals) const

include/chaiscript/language/chaiscript_eval.hpp

@@ -340,6 +340,8 @@ namespace chaiscript
           }
           catch (const exception::reserved_word_error &) {
             throw exception::eval_error("Reserved word used as variable '" + this->children[0]->text + "'");
+          } catch (const exception::name_conflict_error &e) {
+            throw exception::eval_error("Variable redefined '" + e.name() + "'");
           }
           return t_ss.get_object(this->children[0]->text);
         }
@@ -639,6 +641,8 @@ namespace chaiscript
           }
           catch (const exception::reserved_word_error &e) {
             throw exception::eval_error("Reserved word used as function name '" + e.word() + "'");
+          } catch (const exception::name_conflict_error &e) {
+            throw exception::eval_error("Function redefined '" + e.name() + "'");
           }
           return Boxed_Value();
         }
@@ -950,6 +954,7 @@ namespace chaiscript
               end_point = this->children.size() - 1;
             }
             for (unsigned int i = 1; i < end_point; ++i) {
+              chaiscript::eval::detail::Scope_Push_Pop catchscope(t_ss);
               AST_NodePtr catch_block = this->children[i];
 
               if (catch_block->children.size() == 1) {
@@ -992,6 +997,7 @@ namespace chaiscript
           }
           catch (Boxed_Value &except) {
             for (size_t i = 1; i < this->children.size(); ++i) {
+              chaiscript::eval::detail::Scope_Push_Pop catchscope(t_ss);
               AST_NodePtr catch_block = this->children[i];
 
               if (catch_block->children.size() == 1) {
@@ -1006,7 +1012,7 @@ namespace chaiscript
                 break;
               }
               else if (catch_block->children.size() == 3) {
-                //Variable capture, no guards
+                //Variable capture, guards
                 t_ss.add_object(catch_block->children[0]->text, except);
 
                 bool guard;
@@ -1134,6 +1140,8 @@ namespace chaiscript
           }
           catch (const exception::reserved_word_error &e) {
             throw exception::eval_error("Reserved word used as method name '" + e.word() + "'");
+          } catch (const exception::name_conflict_error &e) {
+            throw exception::eval_error("Method redefined '" + e.name() + "'");
           }
           return Boxed_Value();
         }
@@ -1161,6 +1169,8 @@ namespace chaiscript
           }
           catch (const exception::reserved_word_error &) {
             throw exception::eval_error("Reserved word used as attribute '" + this->children[1]->text + "'");
+          } catch (const exception::name_conflict_error &e) {
+            throw exception::eval_error("Attribute redefined '" + e.name() + "'");
           }
           return Boxed_Value();
         }

unittests/function_ordering_test.cpp

@@ -17,7 +17,7 @@ int main()
   chaiscript::ChaiScript chai;
   chai.eval("def test_fun(x) { return 3; }");
   chai.eval("def test_fun(x) : x == \"hi\" { return 4; }");
-  chai.eval("def test_fun(x) { return 5; }");
+//  chai.eval("def test_fun(x) { return 5; }");
   chai.add(chaiscript::fun(&test_one), "test_fun");
   chai.add(chaiscript::fun(&test_two), "test_fun");
 

unittests/function_redefinition.chai

@@ -0,0 +1,2 @@
+assert_throws("Function already defined", fun() { def foo(x) { x + 1 }; def foo(x) { x + 1 } } );
+

unittests/object_lifetime_test.cpp

@@ -38,19 +38,19 @@ int main()
 
   chaiscript::ChaiScript chai;
   chai.add(m);
-  chai.add(chaiscript::fun(&Test::count), "count");
+//  chai.add(chaiscript::fun(&Test::count), "count");
 
   int count = chai.eval<int>("count()");
 
   int count2 = chai.eval<int>("var i = 0; { var t = Test(); } return i;");
 
-  int count3 = chai.eval<int>("var i = 0; { var t = Test(); i = count(); } return i;");
+  int count3 = chai.eval<int>("i = 0; { var t = Test(); i = count(); } return i;");
 
-  int count4 = chai.eval<int>("var i = 0; { var t = Test(); { var t2 = Test(); i = count(); } } return i;");
+  int count4 = chai.eval<int>("i = 0; { var t = Test(); { var t2 = Test(); i = count(); } } return i;");
  
-  int count5 = chai.eval<int>("var i = 0; { var t = Test(); { var t2 = Test(); } i = count(); } return i;");
+  int count5 = chai.eval<int>("i = 0; { var t = Test(); { var t2 = Test(); } i = count(); } return i;");
 
-  int count6 = chai.eval<int>("var i = 0; { var t = Test(); { var t2 = Test(); }  } i = count(); return i;");
+  int count6 = chai.eval<int>("i = 0; { var t = Test(); { var t2 = Test(); }  } i = count(); return i;");
 
   if (count == 0
       && count2 == 0

unittests/utility_test.cpp

@@ -30,8 +30,8 @@ int main()
   chaiscript::ChaiScript chai;
   chai.add(m);
   if (chai.eval<std::string>("var t = Test(); t.function2(); ") == "Function2"
-      && chai.eval<std::string>("var t = Test(); t.functionOverload(1); ") == "int"
+      && chai.eval<std::string>("var t2 = Test(); t2.functionOverload(1); ") == "int"
-      && chai.eval<std::string>("var t = Test(); t.functionOverload(1.1); ") == "double")
+      && chai.eval<std::string>("var t3 = Test(); t3.functionOverload(1.1); ") == "double")
   {
     chai.eval("t = Test();");
     return EXIT_SUCCESS;

unittests/variable_redefinition.chai

@@ -0,0 +1,2 @@
+assert_throws("Variable already defined", fun() { var y = 10; var y = 20; })
+