diff --git a/src/ansi-c/README.md b/src/ansi-c/README.md index 4625559f11c..de73b8e8d79 100644 --- a/src/ansi-c/README.md +++ b/src/ansi-c/README.md @@ -78,9 +78,9 @@ data structures. In particular, \ref exprt "expressions", An irep is a tree of ireps. A subtlety is that an irep is actually the root of _three_ (possibly empty) trees, i.e. it has three disjoint sets of children: \ref irept::get_sub() returns a list of children, and -\ref irept::get_named_sub() and \ref irept::get_comments() each return an -association from names to children. **Most clients never use these -functions directly**, as subtypes of irept generally provide more +\ref irept::get_named_sub() returns an association from names to children. +**Most clients never use these functions directly**, +as subtypes of irept generally provide more descriptive functions. For example, the operands of an \ref exprt "expression" (\ref exprt::op0() "op0", op1 etc) are really that expression's children; the diff --git a/src/cpp/cpp_type2name.cpp b/src/cpp/cpp_type2name.cpp index 7465abdb47b..d2f2c13bc24 100644 --- a/src/cpp/cpp_type2name.cpp +++ b/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) diff --git a/src/util/README.md b/src/util/README.md index f4831e0f0de..52432e42b42 100644 --- a/src/util/README.md +++ b/src/util/README.md @@ -14,15 +14,15 @@ CPROVER codebase. See detailed documentation at \ref irept. -[irept](\ref irept)s are generic tree nodes. You -should think of each node as holding a single string ([data](\ref irept::data), -actually an \ref irep_idt) and lots of child nodes, some of which are numbered -([sub](\ref irept::dt::sub)) and some of which are labelled, and the label -can either start with a “\#” ([comments](\ref irept::dt::comments)) or without -one ([named_sub](\ref irept::dt::named_sub)). The meaning of the “\#” is that -this child shouldn't be counted when comparing two [irept](\ref irept)s for -equality; this is usually used when making an advisory annotation which does -not alter the semantics of the program. +[irept](\ref irept)s are generic tree nodes. You should think of each node +as holding a single string ([data](\ref irept::data), actually an \ref +irep_idt) and lots of child nodes, some of which are numbered ([sub](\ref +irept::dt::sub)) and some of which are labelled ([named_sub](\ref +irept::dt::named_sub)). The the label can either start with a “\#” or +without one. The meaning of the “\#” is that this child shouldn't be +considered when comparing two [irept](\ref irept)s for equality; this is +usually used when making an advisory annotation which does not alter the +semantics of the program. They are used to represent many kinds of structured objects throughout the CPROVER codebase, such as expressions, types and code. An \ref exprt represents @@ -72,9 +72,8 @@ standard data structures as in irept. \subsection irep_idt_section Strings: dstringt, the string_container and the ID_* -Within cbmc, strings are represented using \ref irep_idt, or \ref irep_namet -for keys to [named_sub](\ref irept::dt::named_sub) or -[comments](\ref irept::dt::comments). By default these are both +Within cbmc, strings are represented using \ref irep_idt or \ref irep_namet +for keys to [named_sub](\ref irept::dt::named_sub). By default these are both typedefed to \ref dstringt. For debugging purposes you can set `USE_STD_STRING`, in which case they are both typedefed to `std::string`. You can also easily convert an [irep_idt](\ref irep_idt) or [irep_namet](\ref irep_namet) to a @@ -91,11 +90,10 @@ in `irep_ids.def` is `“IREP_ID_ONE(type)”`, so the string “type” has ind You can refer to this \ref irep_idt as `ID_type`. The other kind of line you see is \c "IREP_ID_TWO(C_source_location, #source_location)", which means the \ref irep_idt for the string “\#source_location” can be referred to as -`ID_C_source_location`. The “C” is for comment, meaning that it should be -stored in the [comments](\ref irept::dt::comments). Any strings that need -to be stored as [irep_idt](\ref irep_idt)s which aren't in `irep_ids.def` -are added to the end of the table when they are first encountered, and the -same index is used for all instances. +`ID_C_source_location`. The “C” is for comment, meaning that it starts with +“\#”. Any strings that need to be stored as [irep_idt](\ref irep_idt)s +which aren't in `irep_ids.def` are added to the end of the table when they +are first encountered, and the same index is used for all instances. See documentation at \ref dstringt. @@ -125,8 +123,8 @@ of size 2 (for the two arguments of minus). Recall that every \ref irept has one piece of data of its own, i.e. its [id()](\ref irept::id()), and all other information is in its -[named_sub](\ref irept::dt::named_sub), [comments](\ref irept::dt::comments) -or [sub](\ref irept::dt::sub). For [exprt](\ref exprt)s, the +[named_sub](\ref irept::dt::named_sub) or [sub](\ref irept::dt::sub). +For [exprt](\ref exprt)s, the [id()](\ref irept::id()) is used to specify why kind of \ref exprt this is, so a \ref minus_exprt has `ID_minus` as its [id()](\ref irept::id()). This means that a \ref minus_exprt can be passed wherever an \ref exprt is diff --git a/src/util/irep.cpp b/src/util/irep.cpp index b403ea96550..b5fb653ae3f 100644 --- a/src/util/irep.cpp +++ b/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,7 +212,7 @@ 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()) @@ -231,7 +220,7 @@ const irep_idt &irept::get(const irep_namet &name) const static const irep_idt empty; return empty; } - #endif +#endif return it->second.id(); } @@ -268,46 +257,43 @@ 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() || @@ -315,17 +301,16 @@ irept &irept::add(const irep_namet &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,7 +320,7 @@ irept &irept::add(const irep_namet &name, const irept &irep) it->second=irep; return it->second; - #else +#else std::pair entry= s.insert(std::make_pair(name, irep)); @@ -343,7 +328,7 @@ irept &irept::add(const irep_namet &name, const irept &irep) entry.first->second=irep; return entry.first->second; - #endif +#endif } #ifdef IREP_HASH_STATS @@ -361,9 +346,7 @@ 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; @@ -371,8 +354,44 @@ bool irept::operator==(const irept &other) const 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; ifirst!=i2_it->first || - !i1_it->second.full_eq(i2_it->second)) - return false; - } - return true; } @@ -494,6 +501,7 @@ bool irept::ordering(const irept &other) const } /// defines ordering on the internal representation +/// comments are ignored int irept::compare(const irept &i) const { int r; @@ -504,6 +512,7 @@ int irept::compare(const irept &i) const const subt::size_type size=get_sub().size(), i_size=i.get_sub().size(); + if(sizei_size) @@ -527,8 +536,9 @@ int irept::compare(const irept &i) const "Unequal lengths will return before this"); } - const named_subt::size_type n_size=get_named_sub().size(), - i_n_size=i.get_named_sub().size(); + const auto n_size = number_of_non_comments(get_named_sub()), + i_n_size = number_of_non_comments(i.get_named_sub()); + if(n_sizei_n_size) @@ -537,12 +547,33 @@ int irept::compare(const irept &i) const { irept::named_subt::const_iterator it1, it2; - for(it1=get_named_sub().begin(), - it2=i.get_named_sub().begin(); - it1!=get_named_sub().end() && it2!=i.get_named_sub().end(); - it1++, - it2++) + // clang-format off + for(it1 = get_named_sub().begin(), + it2 = i.get_named_sub().begin(); + it1 != get_named_sub().end() || + it2 != i.get_named_sub().end(); + ) // no iterator increments + // clang-format on { + if(it1 != get_named_sub().end() && is_comment(it1->first)) + { + it1++; + continue; + } + + if(it2 != i.get_named_sub().end() && is_comment(it2->first)) + { + it2++; + continue; + } + + // the case that both it1 and it2 are .end() is treated + // by the loop guard; furthermore, the number of non-comments + // must be the same + INVARIANT(it1 != get_named_sub().end(), "not at end of get_named_sub()"); + INVARIANT( + it2 != i.get_named_sub().end(), "not at end of i.get_named_sub()"); + r=it1->first.compare(it2->first); if(r!=0) return r; @@ -550,11 +581,14 @@ int irept::compare(const irept &i) const r=it1->second.compare(it2->second); if(r!=0) return r; + + it1++; + it2++; } - INVARIANT(it1==get_named_sub().end() && - it2==i.get_named_sub().end(), - "Unequal lengths will return before this"); + INVARIANT( + it1 == get_named_sub().end() && it2 == i.get_named_sub().end(), + "Unequal number of non-comments will return before this"); } // equal @@ -571,6 +605,17 @@ bool irept::operator<(const irept &other) const unsigned long long irep_hash_cnt=0; #endif +std::size_t irept::number_of_non_comments(const named_subt &named_sub) +{ + std::size_t result = 0; + + for(const auto &n : named_sub) + if(!is_comment(n.first)) + result++; + + return result; +} + std::size_t irept::hash() const { #ifdef HASH_CODE @@ -585,20 +630,24 @@ std::size_t irept::hash() const forall_irep(it, sub) result=hash_combine(result, it->hash()); + std::size_t number_of_named_ireps = 0; + forall_named_irep(it, named_sub) - { - result=hash_combine(result, hash_string(it->first)); - result=hash_combine(result, it->second.hash()); - } + if(!is_comment(it->first)) // this variant ignores comments + { + result = hash_combine(result, hash_string(it->first)); + result = hash_combine(result, it->second.hash()); + number_of_named_ireps++; + } - result=hash_finalize(result, named_sub.size()+sub.size()); + result = hash_finalize(result, sub.size() + number_of_named_ireps); - #ifdef HASH_CODE +#ifdef HASH_CODE read().hash_code=result; - #endif - #ifdef IREP_HASH_STATS +#endif +#ifdef IREP_HASH_STATS ++irep_hash_cnt; - #endif +#endif return result; } @@ -606,27 +655,19 @@ 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()); forall_irep(it, sub) result=hash_combine(result, it->full_hash()); + // this variant includes all named_sub elements forall_named_irep(it, named_sub) { result=hash_combine(result, hash_string(it->first)); 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 +703,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()) diff --git a/src/util/irep.h b/src/util/irep.h index 706fb60db26..91c94b4cf6b 100644 --- a/src/util/irep.h +++ b/src/util/irep.h @@ -99,11 +99,8 @@ const irept &get_nil_irep(); /// /// * \ref irept::dt::named_sub : A map from `irep_namet` (a string) to \ref /// irept. This is used for named children, i.e. subexpressions, parameters, -/// etc. -/// -/// * \ref irept::dt::comments : Another map from `irep_namet` to \ref irept -/// which is used for annotations and other ‘non-semantic’ information. Note -/// that this map is ignored by the default \ref operator==. +/// etc. Children whose name begins with '#' are ignored by the +/// default \ref operator==. /// /// * \ref irept::dt::sub : A vector of \ref irept which is used to store /// ordered but unnamed children. @@ -318,8 +315,6 @@ class irept const subt &get_sub() const { return read().sub; } named_subt &get_named_sub() { return write().named_sub; } // DANGEROUS const named_subt &get_named_sub() const { return read().named_sub; } - named_subt &get_comments() { return write().comments; } // DANGEROUS - const named_subt &get_comments() const { return read().comments; } std::size_t hash() const; std::size_t full_hash() const; @@ -328,11 +323,13 @@ class irept std::string pretty(unsigned indent=0, unsigned max_indent=0) const; -protected: +public: static bool is_comment(const irep_namet &name) { return !name.empty() && name[0]=='#'; } -public: + /// count the number of named_sub elements that are not comments + static std::size_t number_of_non_comments(const named_subt &); + class dt { private: @@ -347,7 +344,6 @@ class irept irep_idt data; named_subt named_sub; - named_subt comments; subt sub; #ifdef HASH_CODE @@ -359,7 +355,6 @@ class irept data.clear(); sub.clear(); named_sub.clear(); - comments.clear(); #ifdef HASH_CODE hash_code=0; #endif @@ -370,7 +365,6 @@ class irept d.data.swap(data); d.sub.swap(sub); d.named_sub.swap(named_sub); - d.comments.swap(comments); #ifdef HASH_CODE std::swap(d.hash_code, hash_code); #endif diff --git a/src/util/irep_hash_container.cpp b/src/util/irep_hash_container.cpp index dc79d098670..ee8bc99d90c 100644 --- a/src/util/irep_hash_container.cpp +++ b/src/util/irep_hash_container.cpp @@ -49,32 +49,43 @@ void irep_hash_container_baset::pack( { const irept::subt &sub=irep.get_sub(); const irept::named_subt &named_sub=irep.get_named_sub(); - const irept::named_subt &comments=irep.get_comments(); - packed.reserve( - 1+1+sub.size()+named_sub.size()*2+ - (full?comments.size()*2:0)); + if(full) + { + packed.reserve(1 + 1 + sub.size() + named_sub.size()); - packed.push_back(irep_id_hash()(irep.id())); + packed.push_back(irep_id_hash()(irep.id())); - packed.push_back(sub.size()); - forall_irep(it, sub) - packed.push_back(number(*it)); + packed.push_back(sub.size()); + forall_irep(it, sub) + packed.push_back(number(*it)); - packed.push_back(named_sub.size()); - forall_named_irep(it, named_sub) - { - packed.push_back(irep_id_hash()(it->first)); // id - packed.push_back(number(it->second)); // sub-irep - } - - if(full) - { - packed.push_back(comments.size()); - forall_named_irep(it, comments) + packed.push_back(named_sub.size()); + for(const auto &sub_irep : named_sub) { - packed.push_back(irep_id_hash()(it->first)); // id - packed.push_back(number(it->second)); // sub-irep + packed.push_back(irep_id_hash()(sub_irep.first)); // id + packed.push_back(number(sub_irep.second)); // sub-irep } } + else + { + const std::size_t non_comment_count = + irept::number_of_non_comments(named_sub); + + packed.reserve(1 + 1 + sub.size() + non_comment_count); + + packed.push_back(irep_id_hash()(irep.id())); + + packed.push_back(sub.size()); + forall_irep(it, sub) + packed.push_back(number(*it)); + + packed.push_back(non_comment_count); + for(const auto &sub_irep : named_sub) + if(!irept::is_comment(sub_irep.first)) + { + packed.push_back(irep_id_hash()(sub_irep.first)); // id + packed.push_back(number(sub_irep.second)); // sub-irep + } + } } diff --git a/src/util/irep_serialization.cpp b/src/util/irep_serialization.cpp index 1366727bee4..d3840505b2d 100644 --- a/src/util/irep_serialization.cpp +++ b/src/util/irep_serialization.cpp @@ -38,13 +38,6 @@ void irep_serializationt::write_irep( reference_convert(it->second, out); } - forall_named_irep(it, irep.get_comments()) - { - out.put('C'); - write_string_ref(out, it->first); - reference_convert(it->second, out); - } - out.put(0); // terminator } diff --git a/src/util/json_irep.cpp b/src/util/json_irep.cpp index 7a1e3b5e9ba..c999b89ff0c 100644 --- a/src/util/json_irep.cpp +++ b/src/util/json_irep.cpp @@ -40,9 +40,6 @@ json_objectt json_irept::convert_from_irep(const irept &irep) const convert_sub_tree("sub", irep.get_sub(), irep_object); convert_named_sub_tree("namedSub", irep.get_named_sub(), irep_object); - if(include_comments) - convert_named_sub_tree("comment", irep.get_comments(), irep_object); - return irep_object; } @@ -86,10 +83,11 @@ void json_irept::convert_named_sub_tree( { json_objectt sub_objects; for(const auto &sub_tree : sub_trees) - { - json_objectt sub_object=convert_from_irep(sub_tree.second); - sub_objects[id2string(sub_tree.first)]=sub_object; - } + if(include_comments || !irept::is_comment(sub_tree.first)) + { + json_objectt sub_object = convert_from_irep(sub_tree.second); + sub_objects[id2string(sub_tree.first)] = sub_object; + } parent[sub_tree_id]=sub_objects; } } diff --git a/src/util/lispirep.cpp b/src/util/lispirep.cpp index 6f223d0c7f6..9ba1a3ceae7 100644 --- a/src/util/lispirep.cpp +++ b/src/util/lispirep.cpp @@ -46,9 +46,7 @@ void irep2lisp(const irept &src, lispexprt &dest) dest.value=""; dest.type=lispexprt::List; - dest.reserve(2+2*src.get_sub().size() - +2*src.get_named_sub().size() - +2*src.get_comments().size()); + dest.reserve(2 + 2 * src.get_sub().size() + 2 * src.get_named_sub().size()); lispexprt id; id.type=lispexprt::String; @@ -83,19 +81,6 @@ void irep2lisp(const irept &src, lispexprt &dest) dest.push_back(sub); } - forall_named_irep(it, src.get_comments()) - { - lispexprt name; - name.type=lispexprt::String; - name.value=name2string(it->first); - dest.push_back(name); - - lispexprt sub; - - irep2lisp(it->second, sub); - dest.push_back(sub); - } - lispexprt nil; nil.type=lispexprt::Symbol; nil.value="nil"; diff --git a/src/util/merge_irep.cpp b/src/util/merge_irep.cpp index 02e04b461e5..db4df77d74e 100644 --- a/src/util/merge_irep.cpp +++ b/src/util/merge_irep.cpp @@ -148,17 +148,6 @@ const irept &merge_irept::merged(const irept &irep) dest_named_sub[it->first]=merged(it->second); // recursive call #endif - const irept::named_subt &src_comments=irep.get_comments(); - irept::named_subt &dest_comments=new_irep.get_comments(); - - forall_named_irep(it, src_comments) - #ifdef SUB_IS_LIST - dest_comments.push_back( - std::make_pair(it->first, merged(it->second))); // recursive call - #else - dest_comments[it->first]=merged(it->second); // recursive call - #endif - return *irep_store.insert(new_irep).first; } @@ -196,16 +185,5 @@ const irept &merge_full_irept::merged(const irept &irep) dest_named_sub[it->first]=merged(it->second); // recursive call #endif - const irept::named_subt &src_comments=irep.get_comments(); - irept::named_subt &dest_comments=new_irep.get_comments(); - - forall_named_irep(it, src_comments) - #ifdef SUB_IS_LIST - dest_comments.push_back( - std::make_pair(it->first, merged(it->second))); // recursive call - #else - dest_comments[it->first]=merged(it->second); // recursive call - #endif - return *irep_store.insert(new_irep).first; } diff --git a/src/util/xml_irep.cpp b/src/util/xml_irep.cpp index 33d09f42c94..d66cfc1fbbe 100644 --- a/src/util/xml_irep.cpp +++ b/src/util/xml_irep.cpp @@ -30,18 +30,20 @@ void convert( } forall_named_irep(it, irep.get_named_sub()) - { - xmlt &x_nsub=xml.new_element("named_sub"); - x_nsub.set_attribute("name", name2string(it->first)); - convert(it->second, x_nsub); - } + if(!irept::is_comment(it->first)) + { + xmlt &x_nsub = xml.new_element("named_sub"); + x_nsub.set_attribute("name", name2string(it->first)); + convert(it->second, x_nsub); + } - forall_named_irep(it, irep.get_comments()) - { - xmlt &x_com = xml.new_element("comment"); - x_com.set_attribute("name", name2string(it->first)); - convert(it->second, x_com); - } + forall_named_irep(it, irep.get_named_sub()) + if(!irept::is_comment(it->first)) + { + xmlt &x_com = xml.new_element("comment"); + x_com.set_attribute("name", name2string(it->first)); + convert(it->second, x_com); + } } void convert( diff --git a/unit/util/irep.cpp b/unit/util/irep.cpp index ea97126f19e..f22bbfd35e9 100644 --- a/unit/util/irep.cpp +++ b/unit/util/irep.cpp @@ -61,8 +61,7 @@ SCENARIO("irept_memory", "[core][utils][irept]") REQUIRE( sizeof(irept::dt) == - ref_count_size + data_size + sub_size + 2 * named_size + - hash_code_size); + ref_count_size + data_size + sub_size + named_size + hash_code_size); } THEN("get_nil_irep yields ID_nil") @@ -131,7 +130,6 @@ SCENARIO("irept_memory", "[core][utils][irept]") { REQUIRE(irep.get_sub().empty()); REQUIRE(irep.get_named_sub().empty()); - REQUIRE(irep.get_comments().empty()); irept &e = irep.add("a_new_element"); REQUIRE(e.id().empty()); @@ -140,12 +138,15 @@ SCENARIO("irept_memory", "[core][utils][irept]") irept irep2("second_irep"); irep.add("a_new_element", irep2); + REQUIRE(!irept::is_comment("a_new_element")); REQUIRE(irep.find("a_new_element").id() == "second_irep"); REQUIRE(irep.get_named_sub().size() == 1); irep.add("#a_comment", irep2); + REQUIRE(irept::is_comment("#a_comment")); REQUIRE(irep.find("#a_comment").id() == "second_irep"); - REQUIRE(irep.get_comments().size() == 1); + REQUIRE(irep.get_named_sub().size() == 2); + REQUIRE(irept::number_of_non_comments(irep.get_named_sub()) == 1); irept bak(irep); irep.remove("no_such_id"); @@ -159,19 +160,17 @@ SCENARIO("irept_memory", "[core][utils][irept]") irep.move_to_named_sub("another_entry", irep2); REQUIRE(irep.get_sub().size() == 1); - REQUIRE(irep.get_named_sub().size() == 1); - REQUIRE(irep.get_comments().size() == 1); + REQUIRE(irep.get_named_sub().size() == 2); irept irep3; irep.move_to_named_sub("#a_comment", irep3); REQUIRE(irep.find("#a_comment").id().empty()); REQUIRE(irep.get_sub().size() == 1); - REQUIRE(irep.get_named_sub().size() == 1); - REQUIRE(irep.get_comments().size() == 1); + REQUIRE(irep.get_named_sub().size() == 2); irept irep4; irep.move_to_named_sub("#another_comment", irep4); - REQUIRE(irep.get_comments().size() == 2); + REQUIRE(irep.get_named_sub().size() == 3); } THEN("Setting and getting works") @@ -205,13 +204,11 @@ SCENARIO("irept_memory", "[core][utils][irept]") REQUIRE(irep.id().empty()); REQUIRE(irep.get_sub().empty()); REQUIRE(irep.get_named_sub().empty()); - REQUIRE(irep.get_comments().empty()); irep.make_nil(); REQUIRE(irep.id() == ID_nil); REQUIRE(irep.get_sub().empty()); REQUIRE(irep.get_named_sub().empty()); - REQUIRE(irep.get_comments().empty()); } THEN("Pretty printing works")