Cleanup some P4Testgen code. (#3978)

* Refactoring and cleanup.

* Fix header location in gtest.

* More small edits.

* Review comments.

backends/p4tools/common/CMakeLists.txt

@@ -5,11 +5,6 @@ include(common)
 
 project(p4tools-common)
 
-# Boost filesystem is required for path handling.
-set(THREADS_PREFER_PTHREAD_FLAG ON)
-find_package(Threads REQUIRED)
-find_package(Boost REQUIRED COMPONENTS filesystem system iostreams)
-
 # Store the current git commit hash as part of version information.
 execute_process(
   COMMAND git log --pretty=format:-%h -n 1
@@ -39,9 +34,11 @@ set(
   core/target.cpp
   core/z3_solver.cpp
 
+  lib/arch_spec.cpp
   lib/format_int.cpp
   lib/formulae.cpp
   lib/model.cpp
+  lib/namespace_context.cpp
   lib/symbolic_env.cpp
   lib/taint.cpp
   lib/trace_event.cpp
@@ -54,8 +51,6 @@ add_p4tools_library(p4tools-common ${P4C_TOOLS_COMMON_SOURCES})
 
 target_link_libraries(
   p4tools-common
-  PUBLIC ${Boost_SYSTEM_LIBRARY}
-  PUBLIC ${Boost_FILESYSTEM_LIBRARY}
   PUBLIC z3::z3
 )
 

backends/p4tools/common/compiler/compiler_target.cpp

@@ -1,6 +1,7 @@
 #include "backends/p4tools/common/compiler/compiler_target.h"
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "backends/p4tools/common/compiler/context.h"
@@ -17,10 +18,10 @@
 
 namespace P4Tools {
 
-ICompileContext *CompilerTarget::makeContext() { return get().makeContext_impl(); }
+ICompileContext *CompilerTarget::makeContext() { return get().makeContextImpl(); }
 
 std::vector<const char *> *CompilerTarget::initCompiler(int argc, char **argv) {
-    return get().initCompiler_impl(argc, argv);
+    return get().initCompilerImpl(argc, argv);
 }
 
 std::optional<const IR::P4Program *> CompilerTarget::runCompiler() {
@@ -42,10 +43,10 @@ std::optional<const IR::P4Program *> CompilerTarget::runCompiler(const std::stri
 }
 
 std::optional<const IR::P4Program *> CompilerTarget::runCompiler(const IR::P4Program *program) {
-    return get().runCompiler_impl(program);
+    return get().runCompilerImpl(program);
 }
 
-std::optional<const IR::P4Program *> CompilerTarget::runCompiler_impl(
+std::optional<const IR::P4Program *> CompilerTarget::runCompilerImpl(
     const IR::P4Program *program) const {
     const auto &self = get();
 
@@ -67,11 +68,11 @@ std::optional<const IR::P4Program *> CompilerTarget::runCompiler_impl(
     return program;
 }
 
-ICompileContext *CompilerTarget::makeContext_impl() const {
+ICompileContext *CompilerTarget::makeContextImpl() const {
     return new CompileContext<CompilerOptions>();
 }
 
-std::vector<const char *> *CompilerTarget::initCompiler_impl(int argc, char **argv) const {
+std::vector<const char *> *CompilerTarget::initCompilerImpl(int argc, char **argv) const {
     auto *result = P4CContext::get().options().process(argc, argv);
     return ::errorCount() > 0 ? nullptr : result;
 }
@@ -120,7 +121,7 @@ const IR::P4Program *CompilerTarget::runMidEnd(const IR::P4Program *program) con
 }
 
 CompilerTarget::CompilerTarget(std::string deviceName, std::string archName)
-    : Target("compiler", deviceName, archName) {}
+    : Target("compiler", std::move(deviceName), std::move(archName)) {}
 
 const CompilerTarget &CompilerTarget::get() { return Target::get<CompilerTarget>("compiler"); }
 

backends/p4tools/common/compiler/compiler_target.h

@@ -43,15 +43,15 @@ class CompilerTarget : public Target {
 
  protected:
     /// @see @makeContext.
-    virtual ICompileContext *makeContext_impl() const;
+    virtual ICompileContext *makeContextImpl() const;
 
     /// @see runCompiler.
-    virtual std::optional<const IR::P4Program *> runCompiler_impl(const IR::P4Program *) const;
+    virtual std::optional<const IR::P4Program *> runCompilerImpl(const IR::P4Program *) const;
 
     /// This implementation just forwards the given arguments to the compiler.
     ///
     /// @see @initCompiler.
-    virtual std::vector<const char *> *initCompiler_impl(int argc, char **argv) const;
+    virtual std::vector<const char *> *initCompilerImpl(int argc, char **argv) const;
 
     /// Parses the P4 program specified on the command line.
     ///
@@ -64,10 +64,10 @@ class CompilerTarget : public Target {
     const IR::P4Program *runFrontend(const IR::P4Program *program) const;
 
     /// A factory method for providing a target-specific mid end implementation.
-    virtual MidEnd mkMidEnd(const CompilerOptions &options) const;
+    [[nodiscard]] virtual MidEnd mkMidEnd(const CompilerOptions &options) const;
 
     /// A factory method for providing a target-specific front end implementation.
-    virtual P4::FrontEnd mkFrontEnd() const;
+    [[nodiscard]] virtual P4::FrontEnd mkFrontEnd() const;
 
     /// Runs the mid end provided by @mkMidEnd on the given program.
     ///

backends/p4tools/modules/testgen/core/arch_spec.cpp → backends/p4tools/common/lib/arch_spec.cpp

@@ -1,13 +1,11 @@
-#include "backends/p4tools/modules/testgen/core/arch_spec.h"
+#include "backends/p4tools/common/lib/arch_spec.h"
 
 #include <utility>
 
 #include "lib/exceptions.h"
 
 namespace P4Tools {
 
-namespace P4Testgen {
-
 ArchSpec::ArchSpec(cstring packageName, const std::vector<ArchMember> &archVectorInput)
     : packageName(packageName) {
     for (size_t idx = 0; idx < archVectorInput.size(); ++idx) {
@@ -52,6 +50,4 @@ const ArchSpec::ArchMember *ArchSpec::getArchMember(size_t blockIndex) const {
 size_t ArchSpec::getArchVectorSize() const { return archVector.size(); }
 
 cstring ArchSpec::getPackageName() const { return packageName; }
-}  // namespace P4Testgen
-
 }  // namespace P4Tools

backends/p4tools/modules/testgen/core/arch_spec.h → backends/p4tools/common/lib/arch_spec.h

@@ -1,17 +1,14 @@
-#ifndef BACKENDS_P4TOOLS_MODULES_TESTGEN_CORE_ARCH_SPEC_H_
-#define BACKENDS_P4TOOLS_MODULES_TESTGEN_CORE_ARCH_SPEC_H_
-
-#include <stddef.h>
+#ifndef BACKENDS_P4TOOLS_COMMON_LIB_ARCH_SPEC_H_
+#define BACKENDS_P4TOOLS_COMMON_LIB_ARCH_SPEC_H_
 
+#include <cstddef>
 #include <map>
 #include <vector>
 
 #include "lib/cstring.h"
 
 namespace P4Tools {
 
-namespace P4Testgen {
-
 /// Specifies a canonical representation of the target pipeline as documented in P4 code.
 class ArchSpec {
  public:
@@ -37,30 +34,28 @@ class ArchSpec {
 
     /// @returns the index that corresponds to the given name in this architecture specification.
     /// A bug is thrown if the index does not exist.
-    size_t getBlockIndex(cstring blockName) const;
+    [[nodiscard]] size_t getBlockIndex(cstring blockName) const;
 
     /// @returns the architecture member that corresponds to the given index in this architecture
     /// specification. A bug is thrown if the index does not exist.
-    const ArchMember *getArchMember(size_t blockIndex) const;
+    [[nodiscard]] const ArchMember *getArchMember(size_t blockIndex) const;
 
     /// @returns name of the parameter for the given block and parameter index in this architecture
     /// specification. A bug is thrown if the indices are out of range.
-    cstring getParamName(size_t blockIndex, size_t paramIndex) const;
+    [[nodiscard]] cstring getParamName(size_t blockIndex, size_t paramIndex) const;
 
     /// @returns name of the parameter for the given block label and parameter index in this
     /// architecture specification. A bug is thrown if the index is out of range or the block label
     /// does not exist.
-    cstring getParamName(cstring blockName, size_t paramIndex) const;
+    [[nodiscard]] cstring getParamName(cstring blockName, size_t paramIndex) const;
 
     /// @returns the size of the architecture specification vector.
-    size_t getArchVectorSize() const;
+    [[nodiscard]] size_t getArchVectorSize() const;
 
     /// @returns the label of the architecture specification.
-    cstring getPackageName() const;
+    [[nodiscard]] cstring getPackageName() const;
 };
 
-}  // namespace P4Testgen
-
 }  // namespace P4Tools
 
-#endif /* BACKENDS_P4TOOLS_MODULES_TESTGEN_CORE_ARCH_SPEC_H_ */
+#endif /* BACKENDS_P4TOOLS_COMMON_LIB_ARCH_SPEC_H_ */

backends/p4tools/modules/testgen/lib/namespace_context.cpp → backends/p4tools/common/lib/namespace_context.cpp

@@ -1,4 +1,4 @@
-#include "backends/p4tools/modules/testgen/lib/namespace_context.h"
+#include "backends/p4tools/common/lib/namespace_context.h"
 
 #include <set>
 #include <vector>
@@ -9,8 +9,6 @@
 
 namespace P4Tools {
 
-namespace P4Testgen {
-
 const NamespaceContext *NamespaceContext::Empty = new NamespaceContext(nullptr, nullptr);
 
 const NamespaceContext *NamespaceContext::push(const IR::INamespace *ns) const {
@@ -120,10 +118,8 @@ const std::set<cstring> &NamespaceContext::getUsedNames() const {
     return *usedNames;
 }
 
-const cstring NamespaceContext::genName(cstring name, char sep) const {
+cstring NamespaceContext::genName(cstring name, char sep) const {
     return cstring::make_unique(getUsedNames(), name, sep);
 }
 
-}  // namespace P4Testgen
-
 }  // namespace P4Tools

backends/p4tools/modules/testgen/lib/namespace_context.h → backends/p4tools/common/lib/namespace_context.h

@@ -1,5 +1,5 @@
-#ifndef BACKENDS_P4TOOLS_MODULES_TESTGEN_LIB_NAMESPACE_CONTEXT_H_
-#define BACKENDS_P4TOOLS_MODULES_TESTGEN_LIB_NAMESPACE_CONTEXT_H_
+#ifndef BACKENDS_P4TOOLS_COMMON_LIB_NAMESPACE_CONTEXT_H_
+#define BACKENDS_P4TOOLS_COMMON_LIB_NAMESPACE_CONTEXT_H_
 
 #include <optional>
 #include <set>
@@ -10,8 +10,6 @@
 
 namespace P4Tools {
 
-namespace P4Testgen {
-
 /// Represents a stack of namespaces.
 class NamespaceContext {
  private:
@@ -49,11 +47,9 @@ class NamespaceContext {
 
     /// @returns a name that is fresh in this context. The given @name and @sep are passed to
     /// cstring::make_unique.
-    const cstring genName(cstring name, char sep) const;
+    cstring genName(cstring name, char sep) const;
 };
 
-}  // namespace P4Testgen
-
 }  // namespace P4Tools
 
-#endif /* BACKENDS_P4TOOLS_MODULES_TESTGEN_LIB_NAMESPACE_CONTEXT_H_ */
+#endif /* BACKENDS_P4TOOLS_COMMON_LIB_NAMESPACE_CONTEXT_H_ */

backends/p4tools/common/lib/symbolic_env.h

@@ -18,30 +18,30 @@ class SymbolicEnv {
     // Maybe coerce from Model for concrete execution?
 
     /// @returns the symbolic value for the given variable.
-    const IR::Expression *get(const StateVariable &var) const;
+    [[nodiscard]] const IR::Expression *get(const StateVariable &var) const;
 
     /// Checks whether the given variable exists in the symbolic environment.
-    bool exists(const StateVariable &var) const;
+    [[nodiscard]] bool exists(const StateVariable &var) const;
 
     /// Sets the symbolic value of the given state variable to the given value. Constant folding is
     /// done on the given value before updating the symbolic state.
     void set(const StateVariable &var, const IR::Expression *value);
 
     /// Completes the model with all variables referenced in the symbolic environment.
-    Model *complete(const Model &model) const;
+    [[nodiscard]] Model *complete(const Model &model) const;
 
     /// Evaluates this symbolic environment in the context of the given model.
     ///
     /// A BUG occurs if any symbolic value in this environment refers to a variable that is not
     /// bound by the given model.
-    Model *evaluate(const Model &model) const;
+    [[nodiscard]] Model *evaluate(const Model &model) const;
 
     /// Substitutes state variables in @expr for their symbolic value in this environment.
     /// Variables that are unbound by this environment are left untouched.
     const IR::Expression *subst(const IR::Expression *expr) const;
 
     /// @returns The immutable map that is internal to this symbolic environment.
-    const SymbolicMapType &getInternalMap() const;
+    [[nodiscard]] const SymbolicMapType &getInternalMap() const;
 
     /// Determines whether the given node represents a symbolic value. Symbolic values may be
     /// stored in the symbolic environment.

backends/p4tools/modules/testgen/CMakeLists.txt

@@ -1,5 +1,4 @@
-# CMake file for Barefoot p4testgen.
-cmake_minimum_required(VERSION 3.0.2 FATAL_ERROR)
+# CMake file for p4testgen.
 
 include(common)
 
@@ -11,7 +10,6 @@ set(
   options.cpp
   testgen.cpp
 
-  core/arch_spec.cpp
   core/externs.cpp
   core/program_info.cpp
   core/small_step/abstract_stepper.cpp
@@ -35,7 +33,6 @@ set(
   lib/final_state.cpp
   lib/gen_eq.cpp
   lib/logging.cpp
-  lib/namespace_context.cpp
   lib/test_backend.cpp
   lib/test_spec.cpp
   lib/tf.cpp
@@ -135,17 +132,6 @@ if(ENABLE_GTESTS)
     PRIVATE gtest
   )
 
-  add_custom_target(
-    linkgtest
-    # Link P4 include files in a more convenient location.
-    COMMAND
-    for incl in p4include p4_14include \; do
-    ${CMAKE_COMMAND} -E create_symlink
-    ${P4C_BINARY_DIR}/\$$incl ${CMAKE_CURRENT_BINARY_DIR}/\$$incl \;
-    done
-  )
-  add_dependencies(testgen-gtest linkgtest)
-
   if(ENABLE_TESTING)
     add_definitions("-DGTEST_HAS_PTHREAD=0")
     add_test(NAME testgen-gtest COMMAND testgen-gtest)

backends/p4tools/modules/testgen/core/constants.h

@@ -3,9 +3,7 @@
 
 #include "ir/ir.h"
 
-namespace P4Tools {
-
-namespace P4Testgen {
+namespace P4Tools::P4Testgen {
 
 class P4Constants {
  public:
@@ -33,8 +31,6 @@ class P4Constants {
     static constexpr const char *MATCH_KIND_LPM = "lpm";
 };
 
-}  // namespace P4Testgen
-
-}  // namespace P4Tools
+}  // namespace P4Tools::P4Testgen
 
 #endif /* BACKENDS_P4TOOLS_MODULES_TESTGEN_CORE_CONSTANTS_H_ */

backends/p4tools/modules/testgen/core/externs.cpp

@@ -13,9 +13,7 @@
 #include "backends/p4tools/modules/testgen/core/small_step/small_step.h"
 #include "backends/p4tools/modules/testgen/lib/execution_state.h"
 
-namespace P4Tools {
-
-namespace P4Testgen {
+namespace P4Tools::P4Testgen {
 
 bool ExternMethodImpls::exec(const IR::MethodCallExpression *call, const IR::Expression *receiver,
                              IR::ID &name, const IR::Vector<IR::Argument> *args,
@@ -107,6 +105,4 @@ ExternMethodImpls::ExternMethodImpls(const ImplList &inputImplList) {
     }
 }
 
-}  // namespace P4Testgen
-
-}  // namespace P4Tools
+}  // namespace P4Tools::P4Testgen