Replace operator< with semanticallyLess.

p4lang · Mar 5, 2024 · cd9f797 · asl · Mar 5, 2024 · fruffy
1 parent a07494a
commit cd9f797
Show file tree

Hide file tree

Showing 17 changed files with 99 additions and 30 deletions.
diff --git a/backends/p4tools/common/lib/model.h b/backends/p4tools/common/lib/model.h
@@ -3,18 +3,19 @@
 
 #include <map>
 #include <utility>
-#include <vector>
 
 #include <boost/container/flat_map.hpp>
 
 #include "ir/ir.h"
+#include "ir/node.h"
 #include "ir/solver.h"
 #include "ir/visitor.h"
 
 namespace P4Tools {
 
 /// Symbolic maps map a state variable to a IR::Expression.
-using SymbolicMapType = boost::container::flat_map<IR::StateVariable, const IR::Expression *>;
+using SymbolicMapType = boost::container::flat_map<IR::StateVariable, const IR::Expression *,
+                                                   IR::IsSemanticallyLessComparator>;
 
 /// Represents a solution found by the solver. A model is a concretized form of a symbolic
 /// environment. All the expressions in a Model must be of type IR::Literal.

diff --git a/backends/p4tools/modules/testgen/lib/concolic.h b/backends/p4tools/modules/testgen/lib/concolic.h
@@ -27,7 +27,7 @@ namespace P4Tools::P4Testgen {
 /// they actually are present, but not expressions. The reason expressions need to be keys is
 /// that sometimes entire expressions are mapped to a particular constant.
 using ConcolicVariableMap = ordered_map<std::reference_wrapper<const IR::Expression>,
-                                        const IR::Expression *, std::less<const IR::Expression>>;
+                                        const IR::Expression *, IR::IsSemanticallyLessComparator>;
 
 /// Encapsulates a set of concolic method implementations.
 class ConcolicMethodImpls {

diff --git a/backends/p4tools/p4tools.def b/backends/p4tools/p4tools.def
@@ -28,7 +28,7 @@ class StateVariable : Expression {
         return *ref == *other.ref;
     }
 
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
         if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name();
         auto &a = static_cast<const StateVariable &>(a_);
@@ -135,7 +135,7 @@ class TaintExpression : Expression {
 class SymbolicVariable : Expression {
 #noconstructor
 
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
         if (node_type_name() !=  a_.node_type_name()) return node_type_name() <  a_.node_type_name();
         auto &a = static_cast<const SymbolicVariable &>(a_);
@@ -224,7 +224,7 @@ public:
         out << "Concolic_" << label << "(" << arguments << ")";
     }
 
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
         if (node_type_name() !=  a_.node_type_name()) return node_type_name() <  a_.node_type_name();
         auto &a = static_cast<const ConcolicVariable &>(a_);

diff --git a/frontends/common/constantParsing.cpp b/frontends/common/constantParsing.cpp
@@ -27,6 +27,10 @@ std::ostream &operator<<(std::ostream &out, const UnparsedConstant &constant) {
     return out;
 }
 
+bool operator<(const UnparsedConstant &a, const UnparsedConstant &b) {
+    return a.text < b.text || a.skip < b.skip || a.base < b.base || a.hasWidth < b.hasWidth;
+}
+
 /// A helper to parse constants which have an explicit width;
 /// @see UnparsedConstant for an explanation of the parameters.
 static IR::Constant *parseConstantWithWidth(Util::SourceInfo srcInfo, const char *text,

diff --git a/frontends/common/constantParsing.h b/frontends/common/constantParsing.h
@@ -69,6 +69,8 @@ struct UnparsedConstant {
 
 std::ostream &operator<<(std::ostream &out, const UnparsedConstant &constant);
 
+bool operator<(const UnparsedConstant &a, const UnparsedConstant &b);
+
 /**
  * Parses an UnparsedConstant @constant into an IR::Constant object, with
  * location information taken from @srcInfo. If parsing fails, an IR::Constant

diff --git a/ir/base.def b/ir/base.def
@@ -133,9 +133,9 @@ abstract Declaration : StatOrDecl, IDeclaration {
     long declid = nextId++;
     ID getName() const override { return name; }
     equiv { return name == a.name; /* ignore declid */ }
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
-        if (StatOrDecl::operator<(a_)) return true;
+        if (StatOrDecl::isSemanticallyLess(a_)) return true;
         auto &a = static_cast<const Declaration &>(a_);
         return name < a.name;  /* ignore declid */
     }
@@ -155,9 +155,9 @@ abstract Type_Declaration : Type, IDeclaration {
     long declid = nextId++;
     ID getName() const override { return name; }
     equiv { return name == a.name; /* ignore declid */ }
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
-        if (Type::operator<(a_)) return true;
+        if (Type::isSemanticallyLess(a_)) return true;
         auto &a = static_cast<const Declaration &>(a_);
         return name < a.name;  /* ignore declid */
     }
@@ -228,6 +228,14 @@ class AnnotationToken {
     cstring text;
     optional NullOK UnparsedConstant* constInfo = nullptr;
     dbprint { out << text; }
+    isSemanticallyLess {
+        if (static_cast<const Node *>(this) == &a_) return false;
+        if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name();
+        auto &a = static_cast<const AnnotationToken &>(a_);
+        return IR::isSemanticallyLess(token_type, a.token_type)
+        || IR::isSemanticallyLess(text, a.text)
+        || IR::isSemanticallyLess(*constInfo, *a.constInfo);
+    }
 }
 
 /// Annotations are used to provide additional information to the compiler

diff --git a/ir/expression.def b/ir/expression.def
@@ -252,10 +252,10 @@ class Constant : Literal {
         return Util::toString(value, width, sign, base);
     }
     visit_children { v.visit(type, "type"); }
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
         if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name();
-        if (Literal::operator<(a_)) return true;
+        if (Literal::isSemanticallyLess(a_)) return true;
         auto &a = static_cast<const Constant &>(a_);
         return value < a.value; /* ignore base */
     }

diff --git a/ir/node.h b/ir/node.h
@@ -154,7 +154,9 @@ class Node : public virtual INode {
     /* 'equiv' does a deep-equals comparison, comparing all non-pointer fields and recursing
      * though all Node subclass pointers to compare them with 'equiv' as well. */
     virtual bool equiv(const Node &a) const { return this->typeId() == a.typeId(); }
-    virtual bool operator<(const Node &a) const { return node_type_name() < a.node_type_name(); }
+    virtual bool isSemanticallyLess(const Node &a) const {
+        return node_type_name() < a.node_type_name();
+    }
 #define DEFINE_OPEQ_FUNC(CLASS, BASE) \
     virtual bool operator==(const CLASS &) const { return false; }
     IRNODE_ALL_SUBCLASSES(DEFINE_OPEQ_FUNC)
@@ -173,9 +175,18 @@ inline bool equal(const INode *a, const INode *b) {
     return a == b || (a && b && *a->getNode() == *b->getNode());
 }
 inline bool equiv(const Node *a, const Node *b) { return a == b || (a && b && a->equiv(*b)); }
+
 inline bool equiv(const INode *a, const INode *b) {
     return a == b || (a && b && a->getNode()->equiv(*b->getNode()));
 }
+struct IsSemanticallyLessComparator {
+    bool operator()(const IR::Node *s1, const IR::Node *s2) const {
+        return s1->isSemanticallyLess(*s2);
+    }
+    bool operator()(const IR::Node &s1, const IR::Node &s2) const {
+        return s1.isSemanticallyLess(s2);
+    }
+};
 // NOLINTBEGIN(bugprone-macro-parentheses)
 /* common things that ALL Node subclasses must define */
 #define IRNODE_SUBCLASS(T)                             \

diff --git a/ir/semantic_less.h b/ir/semantic_less.h
@@ -0,0 +1,29 @@
+#ifndef IR_SEMANTIC_LESS_H_
+#define IR_SEMANTIC_LESS_H_
+
+#include <type_traits>
+
+namespace IR {
+
+template <class T, typename std::enable_if<std::is_integral<T>::value, T>::type * = nullptr>
+bool isSemanticallyLess(const T &a, const T &b) {
+    return a < b;
+}
+
+template <class T, typename std::enable_if<std::is_enum<T>::value, T>::type * = nullptr>
+inline bool isSemanticallyLess(const T &a, const T &b) {
+    return a < b;
+}
+
+/// TODO: This also includes containers such as safe::vectors.
+// These containers may use operator< for comparison.
+// Should it be the responsibility of the IR node implementer to ensure they are implementing
+// container comparison correctly?
+template <class T, typename std::enable_if<std::is_class<T>::value, T>::type * = nullptr>
+inline bool isSemanticallyLess(const T &a, const T &b) {
+    return a < b;
+}
+
+}  // namespace IR
+
+#endif  // IR_SEMANTIC_LESS_H_
diff --git a/ir/solver.h b/ir/solver.h
@@ -18,7 +18,7 @@ using Constraint = IR::Expression;
 /// Comparator to compare SymbolicVariable pointers.
 struct SymbolicVarComp {
     bool operator()(const IR::SymbolicVariable *s1, const IR::SymbolicVariable *s2) const {
-        return s1->operator<(*s2);
+        return s1->isSemanticallyLess(*s2);
     }
 };
 

diff --git a/ir/type.def b/ir/type.def
@@ -17,6 +17,10 @@ enum class Direction {
     InOut
 };
 
+inline bool isSemanticallyLess(const IR::Direction &a, const IR::Direction &b) {
+    return a < b;
+}
+
 inline cstring directionToString(IR::Direction direction) {
     switch (direction) {
         case IR::Direction::None:
@@ -82,9 +86,9 @@ class Type_Any : Type, ITypeVar {
         (void)a;  // silence unused warning
         return true; /* ignore declid */
     }
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
-        if (Type::operator<(a_)) return true;
+        if (Type::isSemanticallyLess(a_)) return true;
         return node_type_name() < a_.node_type_name(); /* ignore declid */
     }
 }
@@ -233,9 +237,9 @@ class Type_InfInt : Type, ITypeVar {
         (void)a;  // silence unused warning
         return true; /* ignore declid */
     }
-    operator< {
+    isSemanticallyLess {
         if (static_cast<const Node *>(this) == &a_) return false;
-        if (Type::operator<(a_)) return true;
+        if (Type::isSemanticallyLess(a_)) return true;
         return node_type_name() < a_.node_type_name(); /* ignore declid */
     }
     const Type* getP4Type() const override { return this; }

diff --git a/ir/v1.def b/ir/v1.def
@@ -231,10 +231,11 @@ class CalculatedField : IAnnotated {
         ID                      name;
         Expression              cond;
         update_or_verify() { }  // FIXME -- needed by umpack_json(safe_vector) -- should not be
-        bool operator<(update_or_verify const & a) const {
+        // update_or_verify is not an IR node, we have to implement our own comparator.
+        bool isSemanticallyLess(update_or_verify const & a) const {
             return update < a.update
             || name < a.name
-            || (cond != nullptr ? a.cond != nullptr ? cond->operator<(*a.cond) : false : a.cond != nullptr);
+            || (cond != nullptr ? a.cond != nullptr ? cond->isSemanticallyLess(*a.cond) : false : a.cond != nullptr);
         }
     }
     safe_vector<update_or_verify> specs = {};
@@ -244,6 +245,14 @@ class CalculatedField : IAnnotated {
         v.visit(field, "field");
         for (auto &s : specs) v.visit(s.cond, s.name.name);
         v.visit(annotations, "annotations"); }
+     isSemanticallyLess {
+            if (static_cast<const Node *>(this) == &a_) return false;
+            if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name();
+            auto &a = static_cast<const CalculatedField &>(a_);
+            return (field != nullptr ? a.field != nullptr ? field->isSemanticallyLess(*a.field) : false : a.field != nullptr)
+            || std::lexicographical_compare(specs.begin(), specs.end(), a.specs.begin(), a.specs.end(), [](const update_or_verify &a, const update_or_verify &b) { return a.isSemanticallyLess(b); })
+            || (annotations != nullptr ? a.annotations != nullptr ? annotations->isSemanticallyLess(*a.annotations) : false : a.annotations != nullptr);
+    }
 }
 
 class ParserValueSet : IAnnotated {

diff --git a/ir/vector.h b/ir/vector.h
@@ -197,17 +197,17 @@ class Vector : public VectorBase {
         }
         return true;
     }
-    bool operator<(const Node &a_) const override {
+    bool isSemanticallyLess(const Node &a_) const override {
         if (static_cast<const Node *>(this) == &a_) {
             return false;
         }
-        if (VectorBase::operator<(a_)) {
+        if (VectorBase::isSemanticallyLess(a_)) {
             return true;
         }
         auto &a = static_cast<const Vector<T> &>(a_);
         return std::lexicographical_compare(
             vec.begin(), vec.end(), a.vec.begin(), a.vec.end(),
-            [](const T *a, const T *b) { return a->operator<(*b); });
+            [](const T *a, const T *b) { return a->isSemanticallyLess(*b); });
     }
     cstring node_type_name() const override { return "Vector<" + T::static_type_name() + ">"; }
     static cstring static_type_name() { return "Vector<" + T::static_type_name() + ">"; }

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -39,7 +39,7 @@ set (GTEST_UNITTEST_SOURCES
   gtest/midend_test.cpp
   gtest/frontend_test.cpp
   gtest/opeq_test.cpp
-  gtest/opless_test.cpp
+  gtest/semantically_less_test.cpp
   gtest/ordered_map.cpp
   gtest/ordered_set.cpp
   gtest/parser_unroll.cpp

diff --git a/test/gtest/opless_test.cpp → test/gtest/semantically_less_test.cpp b/test/gtest/opless_test.cpp → test/gtest/semantically_less_test.cpp
@@ -21,7 +21,7 @@ limitations under the License.
 #include "ir/visitor.h"
 
 static inline bool isSemanticallyLess(const IR::Node &a, const IR::Node &b) {
-    return a.operator<(b);
+    return a.isSemanticallyLess(b);
 }
 
 #define EXECUTE_FUNCTION_FOR_P4C_NODE(BASE_TYPE, a, b, function)                       \

diff --git a/tools/ir-generator/irclass.cpp b/tools/ir-generator/irclass.cpp
@@ -113,6 +113,7 @@ void IrDefinitions::generate(std::ostream &t, std::ostream &out, std::ostream &i
          << "#include \"ir/ir-inline.h\"       // IWYU pragma: keep\n"
          << "#include \"ir/json_generator.h\"  // IWYU pragma: keep\n"
          << "#include \"ir/json_loader.h\"     // IWYU pragma: keep\n"
+         << "#include \"ir/semantic_less.h\"   // IWYU pragma: keep\n"
          << "#include \"ir/visitor.h\"         // IWYU pragma: keep\n"
          << "#include \"lib/algorithm.h\"      // IWYU pragma: keep\n"
          << "#include \"lib/log.h\"            // IWYU pragma: keep\n"

diff --git a/tools/ir-generator/methods.cpp b/tools/ir-generator/methods.cpp
@@ -125,7 +125,7 @@ const ordered_map<cstring, IrMethod::info_t> IrMethod::Generate = {
           buf << cl->indent << "}";
           return buf.str();
       }}},
-    {"operator<",
+    {"isSemanticallyLess",
      {&NamedType::Bool(),
       {new IrField(new ReferenceType(new NamedType(IrClass::nodeClass()), true), "a_")},
       CONST + IN_IMPL + OVERRIDE,
@@ -141,7 +141,7 @@ const ordered_map<cstring, IrMethod::info_t> IrMethod::Generate = {
               if (parent->name != "Node") {
                   buf << cl->indent << cl->indent << "if ("
                       << parent->qualified_name(cl->containedIn)
-                      << "::operator<(a_)) return true;\n";
+                      << "::isSemanticallyLess(a_)) return true;\n";
               }
           }
           if (body) {
@@ -163,12 +163,12 @@ const ordered_map<cstring, IrMethod::info_t> IrMethod::Generate = {
                   }
                   if (f->type->resolve(cl->containedIn) == nullptr) {
                       // This is not an IR pointer
-                      buf << f->name << " < a." << f->name;
+                      buf << "IR::isSemanticallyLess(" << f->name << ", a." << f->name << ")";
                   } else if (f->isInline) {
-                      buf << f->name << ".operator<(a." << f->name << ")";
+                      buf << f->name << ".isSemanticallyLess(a." << f->name << ")";
                   } else {
                       buf << "(" << f->name << " != nullptr ? a." << f->name << " != nullptr ? "
-                          << f->name << "->operator<(*a." << f->name << ")"
+                          << f->name << "->isSemanticallyLess(*a." << f->name << ")"
                           << " : false : a." << f->name << " != nullptr)";
                   }
               }