irept: use single map for all named sub-nodes

Author: kroening

src/cpp/cpp_type2name.cpp

@@ -36,30 +36,29 @@ static void irep2name(const irept &irep, std::string &result)
   if(irep.id()!="")
     result+=do_prefix(irep.id_string());
 
-  if(irep.get_named_sub().empty() &&
-     irep.get_sub().empty() &&
-     irep.get_comments().empty())
+  if(irep.get_named_sub().empty() && irep.get_sub().empty())
     return;
 
   result+='(';
   bool first=true;
 
   forall_named_irep(it, irep.get_named_sub())
-  {
-    if(first)
-      first=false;
-    else
-      result+=',';
+    if(!irept::is_comment(it->first))
+    {
+      if(first)
+        first = false;
+      else
+        result += ',';
 
-    result+=do_prefix(name2string(it->first));
+      result += do_prefix(name2string(it->first));
 
-    result+='=';
-    std::string tmp;
-    irep2name(it->second, tmp);
-    result+=tmp;
-  }
+      result += '=';
+      std::string tmp;
+      irep2name(it->second, tmp);
+      result += tmp;
+    }
 
-  forall_named_irep(it, irep.get_comments())
+  forall_named_irep(it, irep.get_named_sub())
     if(it->first==ID_C_constant ||
        it->first==ID_C_volatile ||
        it->first==ID_C_restricted)

src/goto-programs/json_goto_trace.cpp

@@ -155,7 +155,7 @@ void convert_decl(
   if(trace_options.json_full_lhs)
   {
     // Not language specific, still mangled, fully-qualified name of lhs
-    json_assignment["rawLhs"] = json_irept(true).convert_from_irep(simplified);
+    json_assignment["rawLhs"] = json_irept().convert_from_irep(simplified);
   }
   json_assignment["hidden"] = jsont::json_boolean(step.hidden);
   json_assignment["internal"] = jsont::json_boolean(step.internal);

src/goto-programs/show_goto_functions_json.cpp

@@ -40,7 +40,7 @@ json_objectt show_goto_functions_jsont::convert(
   const goto_functionst &goto_functions)
 {
   json_arrayt json_functions;
-  const json_irept no_comments_irep_converter(false);
+  const json_irept no_comments_irep_converter;
 
   const auto sorted = goto_functions.sorted();
 

src/goto-programs/show_symbol_table.cpp

@@ -166,7 +166,7 @@ static void show_symbol_table_json_ui(
     result_wrapper.push_back_stream_object("symbolTable");
 
   const namespacet ns(symbol_table);
-  json_irept irep_converter(true);
+  json_irept irep_converter;
 
   for(const auto &id_and_symbol : symbol_table.symbols)
   {
@@ -241,7 +241,7 @@ static void show_symbol_table_brief_json_ui(
     result_wrapper.push_back_stream_object("symbolTable");
 
   const namespacet ns(symbol_table);
-  json_irept irep_converter(true);
+  json_irept irep_converter;
 
   for(const auto &id_and_symbol : symbol_table.symbols)
   {

src/json-symtab-language/json_symbol.cpp

@@ -47,7 +47,7 @@ symbolt symbol_from_json(const jsont &in)
   if(!in.is_object())
     throw deserialization_exceptiont("symbol_from_json takes an object");
   symbolt result;
-  json_irept json2irep(true);
+  json_irept json2irep;
   for(const auto &kv : in.object)
   {
     if(kv.first == "type")

src/util/irep.cpp

@@ -154,7 +154,6 @@ void irept::nonrecursive_destructor(dt *old_data)
     {
       stack.reserve(stack.size()+
                     d->named_sub.size()+
-                    d->comments.size()+
                     d->sub.size());
 
       for(named_subt::iterator
@@ -166,15 +165,6 @@ void irept::nonrecursive_destructor(dt *old_data)
         it->second.data=&empty_d;
       }
 
-      for(named_subt::iterator
-          it=d->comments.begin();
-          it!=d->comments.end();
-          it++)
-      {
-        stack.push_back(it->second.data);
-        it->second.data=&empty_d;
-      }
-
       for(subt::iterator
           it=d->sub.begin();
           it!=d->sub.end();
@@ -211,10 +201,9 @@ void irept::move_to_sub(irept &irep)
 
 const irep_idt &irept::get(const irep_namet &name) const
 {
-  const named_subt &s=
-    is_comment(name)?get_comments():get_named_sub();
+  const named_subt &s = get_named_sub();
 
-  #ifdef SUB_IS_LIST
+#ifdef SUB_IS_LIST
   named_subt::const_iterator it=named_subt_lower_bound(s, name);
 
   if(it==s.end() ||
@@ -223,15 +212,15 @@ const irep_idt &irept::get(const irep_namet &name) const
     static const irep_idt empty;
     return empty;
   }
-  #else
+#else
   named_subt::const_iterator it=s.find(name);
 
   if(it==s.end())
   {
     static const irep_idt empty;
     return empty;
   }
-  #endif
+#endif
 
   return it->second.id();
 }
@@ -268,64 +257,60 @@ void irept::set(const irep_namet &name, const long long value)
 
 void irept::remove(const irep_namet &name)
 {
-  named_subt &s=
-    is_comment(name)?get_comments():get_named_sub();
+  named_subt &s = get_named_sub();
 
-  #ifdef SUB_IS_LIST
+#ifdef SUB_IS_LIST
   named_subt::iterator it=named_subt_lower_bound(s, name);
 
   if(it!=s.end() && it->first==name)
     s.erase(it);
-  #else
+#else
   s.erase(name);
-  #endif
+#endif
 }
 
 const irept &irept::find(const irep_namet &name) const
 {
-  const named_subt &s=
-    is_comment(name)?get_comments():get_named_sub();
+  const named_subt &s = get_named_sub();
 
-  #ifdef SUB_IS_LIST
+#ifdef SUB_IS_LIST
   named_subt::const_iterator it=named_subt_lower_bound(s, name);
 
   if(it==s.end() ||
      it->first!=name)
     return get_nil_irep();
-  #else
+#else
   named_subt::const_iterator it=s.find(name);
 
   if(it==s.end())
     return get_nil_irep();
-  #endif
+#endif
 
   return it->second;
 }
 
 irept &irept::add(const irep_namet &name)
 {
-  named_subt &s=
-    is_comment(name)?get_comments():get_named_sub();
+  named_subt &s = get_named_sub();
 
-  #ifdef SUB_IS_LIST
+#ifdef SUB_IS_LIST
   named_subt::iterator it=named_subt_lower_bound(s, name);
 
   if(it==s.end() ||
      it->first!=name)
     it=s.insert(it, std::make_pair(name, irept()));
 
   return it->second;
-  #else
+#else
   return s[name];
-  #endif
+#endif
 }
 
 irept &irept::add(const irep_namet &name, const irept &irep)
 {
-  named_subt &s=
-    is_comment(name)?get_comments():get_named_sub();
+  named_subt &s = get_named_sub();
 
-  #ifdef SUB_IS_LIST
+#ifdef SUB_IS_LIST
   named_subt::iterator it=named_subt_lower_bound(s, name);
 
   if(it==s.end() ||
@@ -335,15 +320,15 @@ irept &irept::add(const irep_namet &name, const irept &irep)
     it->second=irep;
 
   return it->second;
-  #else
+#else
   std::pair<named_subt::iterator, bool> entry=
     s.insert(std::make_pair(name, irep));
 
   if(!entry.second)
     entry.first->second=irep;
 
   return entry.first->second;
-  #endif
+#endif
 }
 
 #ifdef IREP_HASH_STATS
@@ -361,18 +346,52 @@ bool irept::operator==(const irept &other) const
     return true;
   #endif
 
-  if(id()!=other.id() ||
-     get_sub()!=other.get_sub() || // recursive call
-     get_named_sub()!=other.get_named_sub()) // recursive call
+  if(id() != other.id() || get_sub() != other.get_sub()) // recursive call
   {
     #ifdef IREP_HASH_STATS
     ++irep_cmp_ne_cnt;
     #endif
     return false;
   }
 
-  // comments are NOT checked
+  const auto &this_named_sub = get_named_sub();
+  const auto &other_named_sub = other.get_named_sub();
+
+  // walk in sync, ignoring comments, until end of both maps
+  named_subt::const_iterator this_it = this_named_sub.begin();
+  named_subt::const_iterator other_it = other_named_sub.begin();
+
+  while(this_it != this_named_sub.end() || other_it != other_named_sub.end())
+  {
+    if(this_it != this_named_sub.end() && is_comment(this_it->first))
+    {
+      this_it++;
+      continue;
+    }
+
+    if(other_it != other_named_sub.end() && is_comment(other_it->first))
+    {
+      other_it++;
+      continue;
+    }
+
+    if(
+      this_it == this_named_sub.end() ||   // reached end of 'this'
+      other_it == other_named_sub.end() || // reached the end of 'other'
+      this_it->first != other_it->first ||
+      this_it->second != other_it->second) // recursive call
+    {
+#ifdef IREP_HASH_STATS
+      ++irep_cmp_ne_cnt;
+#endif
+      return false;
+    }
 
+    this_it++;
+    other_it++;
+  }
+
+  // reached the end of both
   return true;
 }
 
@@ -390,12 +409,10 @@ bool irept::full_eq(const irept &other) const
   const irept::subt &i2_sub=other.get_sub();
   const irept::named_subt &i1_named_sub=get_named_sub();
   const irept::named_subt &i2_named_sub=other.get_named_sub();
-  const irept::named_subt &i1_comments=get_comments();
-  const irept::named_subt &i2_comments=other.get_comments();
 
-  if(i1_sub.size()!=i2_sub.size() ||
-     i1_named_sub.size()!=i2_named_sub.size() ||
-     i1_comments.size()!=i2_comments.size())
+  if(
+    i1_sub.size() != i2_sub.size() ||
+    i1_named_sub.size() != i2_named_sub.size())
     return false;
 
   for(std::size_t i=0; i<i1_sub.size(); i++)
@@ -412,16 +429,6 @@ bool irept::full_eq(const irept &other) const
         return false;
   }
 
-  {
-    irept::named_subt::const_iterator i1_it=i1_comments.begin();
-    irept::named_subt::const_iterator i2_it=i2_comments.begin();
-
-    for(; i1_it!=i1_comments.end(); i1_it++, i2_it++)
-      if(i1_it->first!=i2_it->first ||
-         !i1_it->second.full_eq(i2_it->second))
-        return false;
-  }
-
   return true;
 }
 
@@ -606,7 +613,6 @@ std::size_t irept::full_hash() const
 {
   const irept::subt &sub=get_sub();
   const irept::named_subt &named_sub=get_named_sub();
-  const irept::named_subt &comments=get_comments();
 
   std::size_t result=hash_string(id());
 
@@ -618,15 +624,7 @@ std::size_t irept::full_hash() const
     result=hash_combine(result, it->second.full_hash());
   }
 
-  forall_named_irep(it, comments)
-  {
-    result=hash_combine(result, hash_string(it->first));
-    result=hash_combine(result, it->second.full_hash());
-  }
-
-  result=hash_finalize(
-    result,
-    named_sub.size()+sub.size()+comments.size());
+  result = hash_finalize(result, named_sub.size() + sub.size());
 
   return result;
 }
@@ -662,18 +660,6 @@ std::string irept::pretty(unsigned indent, unsigned max_indent) const
     result+=it->second.pretty(indent+2, max_indent);
   }
 
-  forall_named_irep(it, get_comments())
-  {
-    result+="\n";
-    indent_str(result, indent);
-
-    result+="* ";
-    result+=id2string(it->first);
-    result+=": ";
-
-    result+=it->second.pretty(indent+2, max_indent);
-  }
-
   std::size_t count=0;
 
   forall_irep(it, get_sub())