Changed interface to symbol_tablet::insert

Author: NathanJPhillips

src/ansi-c/ansi_c_entry_point.cpp

@@ -478,7 +478,7 @@ bool generate_ansi_c_start_function(
   new_symbol.value.swap(init_code);
   new_symbol.mode=symbol.mode;
 
-  if(!symbol_table.insert(std::move(new_symbol)))
+  if(!symbol_table.insert(std::move(new_symbol)).second)
   {
     messaget message;
     message.set_message_handler(message_handler);

src/cpp/cpp_typecheck_compound_type.cpp

@@ -536,7 +536,7 @@ void cpp_typecheckt::typecheck_compound_declarator(
         vt_symb_type.type.set(ID_name, vt_symb_type.name);
         vt_symb_type.is_type=true;
 
-        const bool failed=!symbol_table.insert(std::move(vt_symb_type));
+        const bool failed=!symbol_table.insert(std::move(vt_symb_type)).second;
         assert(!failed);
 
         // add a virtual-table pointer
@@ -612,7 +612,7 @@ void cpp_typecheckt::typecheck_compound_declarator(
           arg.set(ID_C_identifier, arg_symb.name);
 
           // add the parameter to the symbol table
-          const bool failed=!symbol_table.insert(std::move(arg_symb));
+          const bool failed=!symbol_table.insert(std::move(arg_symb)).second;
           assert(!failed);
         }
 
@@ -670,7 +670,7 @@ void cpp_typecheckt::typecheck_compound_declarator(
 
         // add the function to the symbol table
         {
-          bool failed=!symbol_table.insert(std::move(func_symb));
+          bool failed=!symbol_table.insert(std::move(func_symb)).second;
           assert(!failed);
         }
 

src/cpp/cpp_typecheck_namespace.cpp

@@ -73,7 +73,7 @@ void cpp_typecheckt::convert(cpp_namespace_spect &namespace_spec)
     symbol.module=module;
     symbol.type=typet(ID_namespace);
 
-    if(!symbol_table.insert(std::move(symbol)))
+    if(!symbol_table.insert(std::move(symbol)).second)
     {
       error().source_location=symbol.location;
       error() << "cpp_typecheckt::convert_namespace: symbol_table.move() failed"

src/cpp/cpp_typecheck_virtual_table.cpp

@@ -95,7 +95,7 @@ void cpp_typecheckt::do_virtual_table(const symbolt &symbol)
     }
     vt_symb_var.value=values;
 
-    bool failed=!symbol_table.insert(std::move(vt_symb_var));
+    bool failed=!symbol_table.insert(std::move(vt_symb_var)).second;
     assert(!failed);
   }
 }

src/java_bytecode/java_entry_point.cpp

@@ -669,7 +669,7 @@ bool generate_java_start_function(
   new_symbol.value.swap(init_code);
   new_symbol.mode=ID_java;
 
-  if(!symbol_table.insert(std::move(new_symbol)))
+  if(!symbol_table.insert(std::move(new_symbol)).second)
   {
     message.error() << "failed to move main symbol" << messaget::eom;
     return true;

src/java_bytecode/java_utils.cpp

@@ -86,10 +86,9 @@ void generate_class_stub(
   new_symbol.mode=ID_java;
   new_symbol.is_type=true;
 
-  optionalt<std::reference_wrapper<symbolt>> res=
-    symbol_table.insert(std::move(new_symbol));
+  std::pair<symbolt &, bool> res=symbol_table.insert(std::move(new_symbol));
 
-  if(!res)
+  if(!res.second)
   {
     messaget message(message_handler);
     message.warning() <<
@@ -100,7 +99,7 @@ void generate_class_stub(
   else
   {
     // create the class identifier etc
-    java_root_class(*res);
+    java_root_class(res.first);
   }
 }
 

src/jsil/jsil_entry_point.cpp

@@ -162,7 +162,7 @@ bool jsil_entry_point(
   new_symbol.type.swap(main_type);
   new_symbol.value.swap(init_code);
 
-  if(!symbol_table.insert(std::move(new_symbol)))
+  if(!symbol_table.insert(std::move(new_symbol)).second)
   {
     messaget message;
     message.set_message_handler(message_handler);

src/util/fresh_symbol.cpp

@@ -30,9 +30,8 @@ symbolt &get_fresh_aux_symbol(
   static size_t temporary_counter=0;
   auxiliary_symbolt new_symbol;
 
-  optionalt<std::reference_wrapper<symbolt>> res;
   // Loop until find a name that doesn't clash with a non-auxilliary symbol
-  while(!res)
+  while(true)
   {
     // Distinguish local variables with the same name
     new_symbol.base_name=
@@ -44,7 +43,8 @@ symbolt &get_fresh_aux_symbol(
     new_symbol.type=type;
     new_symbol.location=source_location;
     new_symbol.mode=symbol_mode;
-    res=symbol_table.insert(std::move(new_symbol));
+    std::pair<symbolt &, bool> res=symbol_table.insert(std::move(new_symbol));
+    if(res.second)
+      return res.first;
   }
-  return *res;
 }

src/util/symbol_table.cpp

@@ -11,28 +11,38 @@
 ///   there is a symbol with the same name already in the symbol table.
 bool symbol_tablet::add(const symbolt &symbol)
 {
-  std::pair<symbolst::iterator, bool> result=
-  internal_symbols.emplace(symbol.name, symbol);
-  if(!result.second)
-    return true;
-  add_base_and_module(result.first);
-  return false;
+  return !insert(symbol).second;
 }
 
-/// Move a new symbol to the symbol table
-/// \remark: This is a nicer interface than move but achieves the same result
-/// \param symbol: The symbol to be added to the symbol table
-/// \return Returns an optional reference to the newly inserted symbol, without
-///   a value if a symbol with the same name already exists in the symbol table
-symbol_tablet::opt_symbol_reft symbol_tablet::insert(symbolt &&symbol)
+std::pair<symbolt &, bool> symbol_tablet::insert(symbolt symbol)
 {
   // Add the symbol to the table or retrieve existing symbol with the same name
   std::pair<symbolst::iterator, bool> result=
     internal_symbols.emplace(symbol.name, std::move(symbol));
-  if(!result.second)
-    return opt_symbol_reft();
-  add_base_and_module(result.first);
-  return std::ref(result.first->second);
+  symbolt &new_symbol=result.first->second;
+  if(result.second)
+  {
+    try
+    {
+      symbol_base_mapt::iterator base_result=
+        internal_symbol_base_map.emplace(new_symbol.base_name, new_symbol.name);
+      try
+      {
+        internal_symbol_module_map.emplace(new_symbol.module, new_symbol.name);
+      }
+      catch(...)
+      {
+        internal_symbol_base_map.erase(base_result);
+        throw;
+      }
+    }
+    catch(...)
+    {
+      internal_symbols.erase(result.first);
+      throw;
+    }
+  }
+  return std::make_pair(std::ref(new_symbol), result.second);
 }
 
 /// Move a symbol into the symbol table. If there is already a symbol with the
@@ -53,49 +63,22 @@ symbol_tablet::opt_symbol_reft symbol_tablet::insert(symbolt &&symbol)
 bool symbol_tablet::move(symbolt &symbol, symbolt *&new_symbol)
 {
   // Add an empty symbol to the table or retrieve existing symbol with same name
-  std::pair<symbolst::iterator, bool> result=
-    internal_symbols.emplace(symbol.name, symbolt());
-
-  if(!result.second)
-  {
-    // Return the address of the symbol that already existed in the table
-    new_symbol=&result.first->second;
-    return true;
-  }
-
-  // Move the provided symbol into the symbol table
-  result.first->second.swap(symbol);
-
-  add_base_and_module(result.first);
-
-  // Return the address of the new symbol in the table
-  new_symbol=&result.first->second;
-
-  return false;
-}
-
-void symbol_tablet::add_base_and_module(symbolst::iterator added_symbol)
-{
-  symbolt &symbol=added_symbol->second;
-  try
-  {
-    symbol_base_mapt::iterator base_result=
-      internal_symbol_base_map.emplace(symbol.base_name, symbol.name);
-    try
-    {
-      internal_symbol_module_map.emplace(symbol.module, symbol.name);
-    }
-    catch(...)
-    {
-      internal_symbol_base_map.erase(base_result);
-      throw;
-    }
-  }
-  catch(...)
+  symbolt temp_symbol;
+  // This is not copying the symbol, this is passing the three required
+  // parameters to insert (just in the symbol)
+  temp_symbol.name=symbol.name;
+  temp_symbol.base_name=symbol.base_name;
+  temp_symbol.module=symbol.module;
+  std::pair<symbolt &, bool> result=insert(std::move(temp_symbol));
+  if(result.second)
   {
-    internal_symbols.erase(added_symbol);
-    throw;
+    // Move the provided symbol into the symbol table, this can't be done
+    // earlier
+    result.first.swap(symbol);
   }
+  // Return the address of the symbol in the table
+  new_symbol=&result.first;
+  return !result.second;
 }
 
 /// Remove a symbol from the symbol table

src/util/symbol_table.h

@@ -106,12 +106,17 @@ class symbol_tablet
   opt_symbol_reft get_writeable(const irep_idt &identifier);
 
   bool add(const symbolt &symbol);
-  opt_symbol_reft insert(symbolt &&symbol);
+  /// Move or copy a new symbol to the symbol table
+  /// \remark: This is a nicer interface than move and achieves the same
+  /// result as both move and add
+  /// \param symbol: The symbol to be added to the symbol table - can be
+  /// moved or copied in
+  /// \return Returns a reference to the newly inserted symbol or to the
+  /// existing symbol if a symbol with the same name already exists in the
+  /// symbol table, along with a bool that is true if a new symbol was inserted.
+  std::pair<symbolt &, bool> insert(symbolt symbol);
   bool move(symbolt &symbol, symbolt *&new_symbol);
-private:
-  void add_base_and_module(symbolst::iterator added_symbol);
 
-public:
   void clear()
   {
     internal_symbols.clear();