util: Pull "usage()" out of the call types in the m5 utility.
authorGabe Black <gabeblack@google.com>
Thu, 9 Apr 2020 06:55:14 +0000 (23:55 -0700)
committerGabe Black <gabeblack@google.com>
Wed, 12 Aug 2020 01:36:53 +0000 (01:36 +0000)
Also pull common implementations of some call type methods into the base
class, and make disappearing call types clean themselves up to make the
test a little simpler and less error prone.

Change-Id: Ie178fe02d41587647ddc90a084d1d1142b84dde9
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/27687
Reviewed-by: Giacomo Travaglini <giacomo.travaglini@arm.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
Maintainer: Gabe Black <gabeblack@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
util/m5/src/SConscript.native
util/m5/src/call_type.cc
util/m5/src/call_type.hh
util/m5/src/call_type.test.cc
util/m5/src/call_type/addr.cc
util/m5/src/call_type/inst.cc
util/m5/src/call_type/semi.cc
util/m5/src/m5.cc

index e4897ce91f7166d493d9099751431d59a6ff1939..b9ae6bcc77e3aac1ec767d26367fe3ce567aef93 100644 (file)
@@ -28,7 +28,7 @@ Import('*')
 env.Append(CPPPATH=Dir('.'))
 
 env.GTest('args')
-env.GTest('call_type')
+env.GTest('call_type', 'call_type.test.cc', 'call_type.cc', 'args.cc')
 env.GTest('command', 'command.test.cc', 'command.cc', 'args.cc')
 
 command_tests = env.SConscript('command/SConscript.native', exports='env')
index 5deaae1c54438aa9b7e01c14a6d10559235bae70..839a92c4e54e9d535fb2be4b241667619cbe1360 100644 (file)
 #include <cassert>
 #include <sstream>
 
+#include "args.hh"
 #include "call_type.hh"
 
-std::vector<CallType *> &
-CallType::allTypes()
+std::map<std::string, CallType &> &
+CallType::map()
 {
-    static std::vector<CallType *> all;
+    static std::map<std::string, CallType &> all;
     return all;
 }
 
-CallType &
+CallType::CheckArgsResult
+CallType::checkArgs(Args &args)
+{
+    if (args.size() && args[0] == "--" + name) {
+        args.pop();
+        return CheckArgsResult::Match;
+    }
+    return CheckArgsResult::NoMatch;
+}
+
+CallType *
 CallType::detect(Args &args)
 {
     CallType *def = nullptr;
 
-    for (auto *ct: allTypes()) {
-        if (ct->checkArgs(args)) {
-            ct->init();
-            return *ct;
+    for (auto p: map()) {
+        auto &ct = p.second;
+        if (ct.isDefault())
+            def = &ct;
+        auto result = ct.checkArgs(args);
+        switch (result) {
+          case CheckArgsResult::Match:
+            ct.init();
+            return &ct;
+          case CheckArgsResult::NoMatch:
+            continue;
+          case CheckArgsResult::Usage:
+            return nullptr;
+          default:
+            assert(!"Bad checkArgs result");
         }
-        if (ct->isDefault())
-            def = ct;
     }
 
     assert(def);
     def->init();
-    return *def;
+    return def;
 }
 
 std::string
 CallType::usageSummary()
 {
     std::string summary = "";
-    for (auto *ct: allTypes())
-        summary += ct->formattedUsage();
+    for (auto p: map())
+        summary += p.second.formattedUsage();
     return summary;
 }
 
index 4b2b3d12081d3b5275fdd9c79c19ec6ae9977def..9744d3d0c038b413939833689eb2123ad5cb2a76 100644 (file)
 #define __CALL_TYPE_HH__
 
 #include <iostream>
+#include <map>
 #include <string>
-#include <vector>
+#include <utility>
 
 class Args;
 class DispatchTable;
 
 class CallType
 {
+  public:
+    enum class CheckArgsResult {
+        Match,
+        NoMatch,
+        Usage
+    };
+
   protected:
+    const std::string name;
+
     virtual bool isDefault() const = 0;
-    virtual bool checkArgs(Args &args) = 0;
+    virtual CheckArgsResult checkArgs(Args &args);
     virtual void init() {}
 
-    static std::vector<CallType *> &allTypes();
+    static std::map<std::string, CallType &> &map();
 
-    virtual void printBrief(std::ostream &os) const = 0;
+    virtual void printBrief(std::ostream &os) const { os << "--" << name; }
     virtual void printDesc(std::ostream &os) const = 0;
     std::string formattedUsage() const;
 
   public:
-    CallType() { allTypes().push_back(this); }
+    CallType(const std::string &_name) : name(_name)
+    {
+        map().emplace(std::piecewise_construct,
+            std::forward_as_tuple(std::string(_name)),
+            std::forward_as_tuple(*this));
+    }
+
+    ~CallType()
+    {
+        map().erase(name);
+    }
 
-    static CallType &detect(Args &args);
+    static CallType *detect(Args &args);
     static std::string usageSummary();
 
     virtual const DispatchTable &getDispatch() const = 0;
index 0aad8439b4c58e3bf7d293a85b46cb133fda3338..daf6ddf690a9146a41d84e9cc2f6c956ef57ad86 100644 (file)
 // For EXPECT_THAT and HasSubstr
 #include <gmock/gmock.h>
 
+#include "args.hh"
 #include "call_type.hh"
 
 // Simple substitute definitions for Args and DispatchTable, with an int so
 // we can tell instances apart.
-class Args { public: int i; };
 class DispatchTable { public: int i; };
 
 class TestCallType : public CallType
 {
   protected:
     bool isDefault() const override { return testIsDefault; }
-    bool checkArgs(Args &args) override { return args.i == testAcceptArgs; }
     void
     init() override
     {
@@ -52,7 +51,9 @@ class TestCallType : public CallType
     void printBrief(std::ostream &os) const override { os << testBrief; }
     void printDesc(std::ostream &os) const override { os << testDesc; }
   public:
-    static std::vector<CallType *> &testGetAllTypes() { return allTypes(); }
+    TestCallType(const std::string &_name) : CallType(_name) {}
+
+    static std::map<std::string, CallType &> &testGetMap() { return map(); }
 
     // Usage strings to return.
     std::string testBrief;
@@ -62,9 +63,6 @@ class TestCallType : public CallType
     DispatchTable testDt = { 0 };
     const DispatchTable &getDispatch() const override { return testDt; }
 
-    // If an Args has this value, accept it.
-    int testAcceptArgs = 0;
-
     // Whether this call type should be considered default.
     bool testIsDefault = false;
 
@@ -74,124 +72,112 @@ class TestCallType : public CallType
 
 TEST(CallTypeTest, Constructor)
 {
-    auto &all_types = TestCallType::testGetAllTypes();
+    auto &map = TestCallType::testGetMap();
 
     // There should be no call types yet.
-    EXPECT_EQ(all_types.size(), 0);
+    EXPECT_EQ(map.size(), 0);
 
     // Create one.
-    TestCallType test_ct;
+    TestCallType test_ct("test_ct");
 
     // Set the dispatch table to something we'll recognize.
     test_ct.testDt.i = 0xaa55;
 
     // Verify that the list of all call types has one in it now.
-    EXPECT_EQ(all_types.size(), 1);
+    EXPECT_EQ(map.size(), 1);
 
     // Verify that that was our call type by verifying that the dispatch table
     // has our signature.
-    EXPECT_EQ(all_types[0]->getDispatch().i, 0xaa55);
-
-    // Clear all_types, since call types don't clean up after themselves as
-    // they go away.
-    all_types.clear();
+    EXPECT_EQ(map.begin()->second.getDispatch().i, 0xaa55);
 }
 
 TEST(CallTypeTest, DetectOne)
 {
-    auto &all_types = TestCallType::testGetAllTypes();
+    auto &map = TestCallType::testGetMap();
 
     // One option selected.
-    TestCallType option1;
-    option1.testAcceptArgs = 1;
+    TestCallType option1("option1");
     option1.testIsDefault = true;
     option1.testDt.i = 1;
 
-    EXPECT_EQ(all_types.size(), 1);
+    EXPECT_EQ(map.size(), 1);
 
-    Args args;
-    args.i = 1;
+    Args args1({"--option1"});
 
     EXPECT_FALSE(option1.testInitHappened);
 
-    auto &ct = CallType::detect(args);
+    auto *ct = CallType::detect(args1);
 
     // Verify that we selected the only option.
     EXPECT_TRUE(option1.testInitHappened);
-    EXPECT_EQ(ct.getDispatch().i, 1);
+    EXPECT_EQ(ct, &option1);
 
     // One option, selecting the default.
     option1.testInitHappened = false;
 
     // Args will not match.
-    args.i = 2;
+    Args args2({"--option2"});
 
-    auto &def_ct = CallType::detect(args);
+    auto *def_ct = CallType::detect(args2);
 
     // Verify that the one option was defaulted to.
     EXPECT_TRUE(option1.testInitHappened);
-    EXPECT_EQ(def_ct.getDispatch().i, 1);
-
-    // Reset all_types.
-    all_types.clear();
+    EXPECT_EQ(def_ct, &option1);
 }
 
 TEST(CallTypeTest, DetectTwo)
 {
-    auto &all_types = TestCallType::testGetAllTypes();
+    auto &map = TestCallType::testGetMap();
 
     // One of two options selected.
-    TestCallType option1;
-    option1.testAcceptArgs = 1;
+    TestCallType option1("option1");
     option1.testIsDefault = true;
     option1.testDt.i = 1;
 
-    TestCallType option2;
-    option2.testAcceptArgs = 2;
+    TestCallType option2("option2");
     option2.testIsDefault = false;
     option2.testDt.i = 2;
 
-    EXPECT_EQ(all_types.size(), 2);
+    EXPECT_EQ(map.size(), 2);
 
     // Select the first option.
-    Args args;
-    args.i = 1;
+    Args args1({"--option1"});
 
     EXPECT_FALSE(option1.testInitHappened);
     EXPECT_FALSE(option2.testInitHappened);
 
-    auto &ct1 = CallType::detect(args);
+    auto *ct1 = CallType::detect(args1);
 
     // Verify that we selected the first option.
     EXPECT_TRUE(option1.testInitHappened);
     EXPECT_FALSE(option2.testInitHappened);
-    EXPECT_EQ(ct1.getDispatch().i, 1);
+    EXPECT_EQ(ct1, &option1);
 
     option1.testInitHappened = false;
     option2.testInitHappened = false;
 
     // Select the second option.
-    args.i = 2;
+    Args args2({"--option2"});
 
-    auto &ct2 = CallType::detect(args);
+    auto *ct2 = CallType::detect(args2);
 
     // Verify that we selected the second option.
     EXPECT_FALSE(option1.testInitHappened);
     EXPECT_TRUE(option2.testInitHappened);
-    EXPECT_EQ(ct2.getDispatch().i, 2);
+    EXPECT_EQ(ct2, &option2);
 
     option1.testInitHappened = false;
     option2.testInitHappened = false;
 
     // Default to the first option.
-    args.i = 3;
+    Args args3({"--option3"});
 
-    auto &def_ct1 = CallType::detect(args);
+    auto *def_ct1 = CallType::detect(args3);
 
     // Verify that we selected the first option.
     EXPECT_TRUE(option1.testInitHappened);
     EXPECT_FALSE(option2.testInitHappened);
-    EXPECT_EQ(ct1.getDispatch().i, 1);
+    EXPECT_EQ(def_ct1, &option1);
 
     option1.testInitHappened = false;
     option2.testInitHappened = false;
@@ -200,34 +186,31 @@ TEST(CallTypeTest, DetectTwo)
     option1.testIsDefault = false;
     option2.testIsDefault = true;
 
-    auto &def_ct2 = CallType::detect(args);
+    auto *def_ct2 = CallType::detect(args3);
 
     // Verify that we selected the second option.
     EXPECT_FALSE(option1.testInitHappened);
     EXPECT_TRUE(option2.testInitHappened);
-    EXPECT_EQ(ct2.getDispatch().i, 2);
+    EXPECT_EQ(def_ct2, &option2);
 
     option1.testInitHappened = false;
     option2.testInitHappened = false;
-
-    // Reset all_types.
-    all_types.clear();
 }
 
 TEST(CallTypeTest, Usage)
 {
-    auto &all_types = TestCallType::testGetAllTypes();
+    auto &map = TestCallType::testGetMap();
 
-    TestCallType ct1;
+    TestCallType ct1("ct1");
     ct1.testBrief = "brief 1";
     ct1.testDesc = "A longer description of call type 1, which is long.";
 
-    TestCallType ct2;
+    TestCallType ct2("ct2");
     ct2.testBrief = "short 2";
     ct2.testDesc = "Very verbose text saying what call type 2 is, "
         "and is different from 1.";
 
-    EXPECT_EQ(all_types.size(), 2);
+    EXPECT_EQ(map.size(), 2);
 
     auto summary = CallType::usageSummary();
 
@@ -241,7 +224,4 @@ TEST(CallTypeTest, Usage)
     EXPECT_THAT(summary, testing::HasSubstr(ct2.testBrief));
     EXPECT_THAT(summary, testing::HasSubstr(ct2.testDesc));
     EXPECT_THAT(summary, testing::HasSubstr(ct2.testDesc));
-
-    // Reset all_types.
-    all_types.clear();
 }
index ab26db73f94cfb62f030c298c0cbb66542e75540..9a5fee3f2d241c9b4a355dd33de84883fe61e990 100644 (file)
@@ -59,16 +59,17 @@ constexpr uint64_t DefaultAddress = 0;
 
 class AddrCallType : public CallType
 {
-  private:
   public:
+    AddrCallType() : CallType("addr") {}
+
     bool isDefault() const override { return CALL_TYPE_IS_DEFAULT; }
     const DispatchTable &getDispatch() const override { return addr_dispatch; }
 
     void
     printBrief(std::ostream &os) const override
     {
-        os << "--addr " << (DefaultAddrDefined ? "[address override]" :
-                                                 "<address override>");
+        os << "--" << name << (DefaultAddrDefined ? " [address override]" :
+                                                    " <address override>");
     }
 
     void
@@ -81,15 +82,15 @@ class AddrCallType : public CallType
         }
     }
 
-    bool
+    CheckArgsResult
     checkArgs(Args &args) override
     {
-        static const std::string prefix = "--addr";
+        const std::string prefix = "--" + name;
         uint64_t addr_override;
 
         // If the first argument doesn't start with --addr...
         if (!args.size() || args[0].substr(0, prefix.size()) != prefix)
-            return false;
+            return CheckArgsResult::NoMatch;
 
         const std::string &arg = args.pop().substr(prefix.size());
 
@@ -97,25 +98,25 @@ class AddrCallType : public CallType
         if (arg.size()) {
             // If it doesn't start with '=', it's malformed.
             if (arg[0] != '=')
-                usage();
+                return CheckArgsResult::Usage;
             // Attempt to extract an address after the '='.
             if (!args.stoi(arg.substr(1), addr_override))
-                usage();
+                return CheckArgsResult::Usage;
             // If we found an address, use it to override m5op_addr.
             m5op_addr = addr_override;
-            return true;
+            return CheckArgsResult::Match;
         }
         // If an address override wasn't part of the first argument, check if
         // it's the second argument. If not, then there's no override.
         if (args.pop(addr_override)) {
             m5op_addr = addr_override;
-            return true;
+            return CheckArgsResult::Match;
         }
         // If the default address was not defined, an override is required.
         if (!DefaultAddrDefined)
-            usage();
+            return CheckArgsResult::Usage;
 
-        return true;
+        return CheckArgsResult::Match;
     }
 
     void init() override { map_m5_mem(); }
index a6af595506475c4c8cf9083ec89cbbbbd4c32fe6..97153f4fa50adc3a09268da18dc8d8431ad9308a 100644 (file)
@@ -43,20 +43,11 @@ M5OP_FOREACH
 class InstCallType : public CallType
 {
   public:
+    InstCallType() : CallType("inst") {}
+
     bool isDefault() const override { return CALL_TYPE_IS_DEFAULT; }
     const DispatchTable &getDispatch() const override { return inst_dispatch; }
 
-    bool
-    checkArgs(Args &args) override
-    {
-        if (args.size() && args[0] == "--inst") {
-            args.pop();
-            return true;
-        }
-        return false;
-    }
-
-    void printBrief(std::ostream &os) const override { os << "--inst"; }
     void
     printDesc(std::ostream &os) const override
     {
index ddc192565da643f77e17e85c9a6421516f836f3f..40a8e8d837cfe5f9cfdf98f5fe2b09382e3e822c 100644 (file)
@@ -50,20 +50,11 @@ M5OP_FOREACH
 class SemiCallType : public CallType
 {
   public:
+    SemiCallType() : CallType("semi") {}
+
     bool isDefault() const override { return CALL_TYPE_IS_DEFAULT; }
     const DispatchTable &getDispatch() const override { return semi_dispatch; }
 
-    bool
-    checkArgs(Args &args) override
-    {
-        if (args.size() && args[0] == "--semi") {
-            args.pop();
-            return true;
-        }
-        return false;
-    }
-
-    void printBrief(std::ostream &os) const override { os << "--semi"; }
     void
     printDesc(std::ostream &os) const override
     {
index f8c1101f09b91e163bbe5a94b86e57d858e36cdd..0e86817e18ce1c2b3672abc1c5a6dd412bfb9103 100644 (file)
@@ -53,9 +53,11 @@ main(int argc, const char *argv[])
     if (!args.size())
         usage();
 
-    const DispatchTable &dt = CallType::detect(args).getDispatch();
+    CallType *ct = CallType::detect(args);
+    if (!ct)
+        usage();
 
-    if (!Command::run(dt, args))
+    if (!Command::run(ct->getDispatch(), args))
         usage();
 
     exit(0);