From cd9f797e5690fe9c5d7a5c1d8cf1f089372bf030 Mon Sep 17 00:00:00 2001 From: fruffy Date: Tue, 16 Jan 2024 10:15:13 +0100 Subject: [PATCH] Replace operator< with semanticallyLess. --- backends/p4tools/common/lib/model.h | 5 ++-- .../p4tools/modules/testgen/lib/concolic.h | 2 +- backends/p4tools/p4tools.def | 6 ++-- frontends/common/constantParsing.cpp | 4 +++ frontends/common/constantParsing.h | 2 ++ ir/base.def | 16 +++++++--- ir/expression.def | 4 +-- ir/node.h | 13 ++++++++- ir/semantic_less.h | 29 +++++++++++++++++++ ir/solver.h | 2 +- ir/type.def | 12 +++++--- ir/v1.def | 13 +++++++-- ir/vector.h | 6 ++-- test/CMakeLists.txt | 2 +- ...ss_test.cpp => semantically_less_test.cpp} | 2 +- tools/ir-generator/irclass.cpp | 1 + tools/ir-generator/methods.cpp | 10 +++---- 17 files changed, 99 insertions(+), 30 deletions(-) create mode 100644 ir/semantic_less.h rename test/gtest/{opless_test.cpp => semantically_less_test.cpp} (99%) diff --git a/backends/p4tools/common/lib/model.h b/backends/p4tools/common/lib/model.h index ffc63aef3ea..62d3b3db4bc 100644 --- a/backends/p4tools/common/lib/model.h +++ b/backends/p4tools/common/lib/model.h @@ -3,18 +3,19 @@ #include #include -#include #include #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; +using SymbolicMapType = boost::container::flat_map; /// 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 index f3cb6c066f8..ba929c54f62 100644 --- 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, - const IR::Expression *, std::less>; + 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 index aec5bb13de0..062a7db451b 100644 --- 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(this) == &a_) return false; if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name(); auto &a = static_cast(a_); @@ -135,7 +135,7 @@ class TaintExpression : Expression { class SymbolicVariable : Expression { #noconstructor - operator< { + isSemanticallyLess { if (static_cast(this) == &a_) return false; if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name(); auto &a = static_cast(a_); @@ -224,7 +224,7 @@ public: out << "Concolic_" << label << "(" << arguments << ")"; } - operator< { + isSemanticallyLess { if (static_cast(this) == &a_) return false; if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name(); auto &a = static_cast(a_); diff --git a/frontends/common/constantParsing.cpp b/frontends/common/constantParsing.cpp index 5851a0eed6c..6823b3042ce 100644 --- 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 index 00b34111f06..bb9621c5672 100644 --- 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 index b4c8022830a..58e7a3ef94a 100644 --- 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(this) == &a_) return false; - if (StatOrDecl::operator<(a_)) return true; + if (StatOrDecl::isSemanticallyLess(a_)) return true; auto &a = static_cast(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(this) == &a_) return false; - if (Type::operator<(a_)) return true; + if (Type::isSemanticallyLess(a_)) return true; auto &a = static_cast(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(this) == &a_) return false; + if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name(); + auto &a = static_cast(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 index f758c9f4b29..4dbcbdfbe95 100644 --- 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(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(a_); return value < a.value; /* ignore base */ } diff --git a/ir/node.h b/ir/node.h index 49861dba936..e1ae698ec8e 100644 --- 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 new file mode 100644 index 00000000000..aa6eeea5d83 --- /dev/null +++ b/ir/semantic_less.h @@ -0,0 +1,29 @@ +#ifndef IR_SEMANTIC_LESS_H_ +#define IR_SEMANTIC_LESS_H_ + +#include + +namespace IR { + +template ::value, T>::type * = nullptr> +bool isSemanticallyLess(const T &a, const T &b) { + return a < b; +} + +template ::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 ::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 index 459be62fa28..6343879e1d7 100644 --- 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 index 3416521c019..c181906178f 100644 --- 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(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(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 index b92c6e9707c..75f05390793 100644 --- 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 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(this) == &a_) return false; + if (node_type_name() != a_.node_type_name()) return node_type_name() < a_.node_type_name(); + auto &a = static_cast(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 index c55355c5486..af7b02861e4 100644 --- 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(this) == &a_) { return false; } - if (VectorBase::operator<(a_)) { + if (VectorBase::isSemanticallyLess(a_)) { return true; } auto &a = static_cast &>(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 index 81da4382531..53f3694e21f 100644 --- 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 b/test/gtest/semantically_less_test.cpp similarity index 99% rename from test/gtest/opless_test.cpp rename to test/gtest/semantically_less_test.cpp index eb99b8f874a..eb75ee96ee3 100644 --- a/test/gtest/opless_test.cpp +++ b/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 index cfbdfc166d8..f4727fb595c 100644 --- 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 index 8f6bf31fe8f..e3e03391fac 100644 --- a/tools/ir-generator/methods.cpp +++ b/tools/ir-generator/methods.cpp @@ -125,7 +125,7 @@ const ordered_map 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 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 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)"; } }