XML: overhaul the tag-comparison operators.

First and foremost, this patch implements the latest best practices in C++ (i.e. comparison operators are not member functions).

The main objective of this rewrite is to have these operators callable in slightly more general contexts (*this->asFontTag() == *rhs.asFontTag() yielded an error because no member function could be called). This feature is no more required.

The rewrite took place in the context of https://www.lyx.org/trac/ticket/12585. The first iteration was using more complex code to circumvent XMLStream in DocBook (see details in the bug above and in the mailing list), while this one uses XMLStream fully. The bug was due to font tags being considered equal when they were not, due to the comparison done only on the XML tag and not on the actual font change (XML attributes can complement the tag for various font changes).

src/xml.cpp

@@ -123,12 +123,6 @@ docstring StartTag::writeEndTag() const
 }
 
 
-bool StartTag::operator==(FontTag const &rhs) const
-{
-	return rhs == *this;
-}
-
-
 docstring EndTag::writeEndTag() const
 {
 	return from_utf8("</") + tag_ + from_utf8(">");
@@ -152,15 +146,6 @@ docstring CompTag::writeTag() const
 	return output;
 }
 
-
-bool FontTag::operator==(StartTag const & tag) const
-{
-	FontTag const * const ftag = tag.asFontTag();
-	if (!ftag)
-		return false;
-	return (font_type_ == ftag->font_type_);
-}
-
 } // namespace xml
 
 
@@ -679,6 +664,44 @@ docstring xml::cleanID(docstring const & orig)
 }
 
 
+bool operator==(xml::StartTag const & lhs, xml::StartTag const & rhs)
+{
+	xml::FontTag const * const lhs_ft = lhs.asFontTag();
+	xml::FontTag const * const rhs_ft = rhs.asFontTag();
+
+	if ((!lhs_ft && rhs_ft) || (lhs_ft && !rhs_ft))
+		return false;
+	if (!lhs_ft && !rhs_ft)
+		return lhs.tag_ == rhs.tag_;
+	return lhs_ft->tag_ == rhs_ft->tag_ && lhs_ft->font_type_ == rhs_ft->font_type_;
+}
+
+
+bool operator==(xml::EndTag const & lhs, xml::StartTag const & rhs)
+{
+	xml::EndFontTag const * const lhs_ft = lhs.asFontTag();
+	xml::FontTag const * const rhs_ft = rhs.asFontTag();
+
+	if ((!lhs_ft && rhs_ft) || (lhs_ft && !rhs_ft))
+		return false;
+	if (!lhs_ft && !rhs_ft)
+		return lhs.tag_ == rhs.tag_;
+	return lhs_ft->tag_ == rhs_ft->tag_ && lhs_ft->font_type_ == rhs_ft->font_type_;
+}
+
+
+bool operator!=(xml::EndTag const & lhs, xml::StartTag const & rhs)
+{
+	return !(lhs == rhs);
+}
+
+
+bool operator!=(xml::StartTag const & lhs, xml::StartTag const & rhs)
+{
+	return !(lhs == rhs);
+}
+
+
 void xml::openTag(odocstream & os, string const & name, string const & attribute)
 {
     // FIXME UNICODE
@@ -800,10 +823,10 @@ void closeBlockTag(XMLStream & xs, const docstring & tag)
 
 void xml::openTag(XMLStream & xs, const docstring & tag, const docstring & attr, const std::string & tagtype)
 {
-	if (tag.empty() || tag == "NONE") // Common check to be performed elsewhere, if it was not here.
+	if (tag.empty() || tag == from_ascii("NONE")) // Common check to be performed elsewhere, if it was not here.
 		return;
 
-	if (tag == "para" || tagtype == "paragraph") // Special case for <para>: always considered as a paragraph.
+	if (tag == from_ascii("para") || tagtype == "paragraph") // Special case for <para>: always considered as a paragraph.
 		openParTag(xs, tag, attr);
 	else if (tagtype == "block")
 		openBlockTag(xs, tag, attr);

src/xml.h

@@ -196,14 +196,6 @@ struct StartTag
 	///
 	virtual FontTag const * asFontTag() const { return nullptr; }
 	///
-	virtual bool operator==(StartTag const & rhs) const
-	{ return tag_ == rhs.tag_; }
-	///
-	virtual bool operator!=(StartTag const & rhs) const
-	{ return !(*this == rhs); }
-	///
-	virtual bool operator==(FontTag const & rhs) const;
-	///
 	docstring tag_;
 	///
 	docstring attr_;
@@ -229,12 +221,6 @@ struct EndTag
 	/// </tag_>
 	virtual docstring writeEndTag() const;
 	///
-	bool operator==(StartTag const & rhs) const
-	{ return tag_ == rhs.tag_; }
-	///
-	bool operator!=(StartTag const & rhs) const
-	{ return !(*this == rhs); }
-	///
 	virtual EndFontTag const * asFontTag() const { return nullptr; }
 	///
 	docstring tag_;
@@ -342,8 +328,6 @@ struct FontTag : public StartTag
 	///
 	FontTag const * asFontTag() const override { return this; }
 	///
-	bool operator==(StartTag const &) const override;
-	///
 	FontTypes font_type_;
 };
 
@@ -419,6 +403,15 @@ void compTag(XMLStream & xs, const std::string & tag, const docstring & attr, co
 
 } // namespace xml
 
+
+/// Comparison operators for tags. They are defined as free functions, otherwise comparison of casts does not work.
+/// For font tags, do not only compare the XML tag, but also the font type: several fonts can be using the same tag.
+/// In XHTML, <span>; in DocBook, <emphasis>.
+bool operator==(xml::StartTag const & lhs, xml::StartTag const & rhs);
+bool operator==(xml::EndTag const & lhs, xml::StartTag const & rhs);
+bool operator!=(xml::EndTag const & lhs, xml::StartTag const & rhs);
+bool operator!=(xml::StartTag const & lhs, xml::StartTag const & rhs);
+
 } // namespace lyx
 
 #endif // XML_H