Daniel Carvalho has submitted this change. ( https://gem5-review.googlesource.com/c/public/gem5/+/38736 )

Change subject: base: Remove dubious/unused Flags functions
......................................................................

base: Remove dubious/unused Flags functions

The functions isSet(), noneSet(), and allSet() assume that
all bits of the underlying container have a corresponding
flag. This is typically not true, and the likelihood of
incorrectly using these functions is high.

Fortunately these functions are not being used anywhere,
and can be safely removed.

Alternatively, a mask could be provided on construction to
determine which bits of the underlying container correspond
to a flag bit, so that the proper bits are checked in these
functions.

Change-Id: Ia7cbfd0726943506a3f04dc417e67a0b57cdbf95
Signed-off-by: Daniel R. Carvalho <oda...@yahoo.com.br>
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38736
Maintainer: Bobby R. Bruce <bbr...@ucdavis.edu>
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Jason Lowe-Power <power...@gmail.com>
Reviewed-by: Bobby R. Bruce <bbr...@ucdavis.edu>
---
M src/base/flags.hh
M src/base/flags.test.cc
2 files changed, 1 insertion(+), 55 deletions(-)

Approvals:
  Jason Lowe-Power: Looks good to me, approved
  Bobby R. Bruce: Looks good to me, approved; Looks good to me, approved
  kokoro: Regressions pass



diff --git a/src/base/flags.hh b/src/base/flags.hh
index 170abc5..6e82210 100644
--- a/src/base/flags.hh
+++ b/src/base/flags.hh
@@ -72,13 +72,6 @@
     }

     /**
-     * Verifies whether any bit in the flags is set.
-     *
-     * @return True if any flag bit is set; false otherwise.
-     */
-    bool isSet() const { return _flags; }
-
-    /**
      * Verifies whether any bit matching the given mask is set.
      *
      * @param mask The mask containing the bits to verify.
@@ -87,13 +80,6 @@
     bool isSet(Type mask) const { return (_flags & mask); }

     /**
-     * Verifies whether all bits in the flags are set.
-     *
-     * @return True if all flag bits are set; false otherwise.
-     */
-    bool allSet() const { return !(~_flags); }
-
-    /**
      * Verifies whether no bits matching the given mask are set.
      *
      * @param mask The mask containing the bits to verify.
@@ -102,13 +88,6 @@
     bool allSet(Type mask) const { return (_flags & mask) == mask; }

     /**
-     * Verifies whether no bits in the flags are set.
-     *
-     * @return True if all flag bits are cleared; false otherwise.
-     */
-    bool noneSet() const { return _flags == 0; }
-
-    /**
      * Verifies whether no bits matching the given mask are set.
      *
      * @param mask The mask containing the bits to verify.
diff --git a/src/base/flags.test.cc b/src/base/flags.test.cc
index 297abc8..c45a7a8 100644
--- a/src/base/flags.test.cc
+++ b/src/base/flags.test.cc
@@ -99,16 +99,6 @@
     ASSERT_EQ(uint32_t(flags_a), uint32_t(flags_b));
 }

-/** Test isSet for any bit set. */
-TEST(FlagsTest, IsSetAny)
-{
-    const uint32_t value = (1 << 3);
-    const Flags<uint32_t> flags_a;
-    const Flags<uint32_t> flags_b(value);
-    ASSERT_FALSE(flags_a.isSet());
-    ASSERT_TRUE(flags_b.isSet());
-}
-
 /** Test isSet for multiple bits set. */
 TEST(FlagsTest, IsSetValue)
 {
@@ -131,19 +121,6 @@
     ASSERT_FALSE(flags.isSet(value_c));
 }

-/** Test if all bits are set with allSet. */
-TEST(FlagsTest, AllSet)
-{
-    const uint32_t value_b = (1 << 5) | (1 << 6);
-    const uint32_t value_c = std::numeric_limits<uint32_t>::max();
-    const Flags<uint32_t> flags_a;
-    const Flags<uint32_t> flags_b(value_b);
-    const Flags<uint32_t> flags_c(value_c);
-    ASSERT_FALSE(flags_a.allSet());
-    ASSERT_FALSE(flags_b.allSet());
-    ASSERT_TRUE(flags_c.allSet());
-}
-
 /** Test allSet comparing against another flag. */
 TEST(FlagsTest, AllSetMatch)
 {
@@ -154,16 +131,6 @@
     ASSERT_FALSE(flags.allSet(value_b));
 }

-/** Test if no bits are set with noneSet. */
-TEST(FlagsTest, NoneSet)
-{
-    const uint32_t value_b = (1 << 5) | (1 << 6);
-    const Flags<uint32_t> flags_a;
-    const Flags<uint32_t> flags_b(value_b);
-    ASSERT_TRUE(flags_a.noneSet());
-    ASSERT_FALSE(flags_b.noneSet());
-}
-
 /** Test noneSet comparing against another flag. */
 TEST(FlagsTest, NoneSetMatch)
 {
@@ -182,7 +149,7 @@
     const uint32_t value = (1 << 5) | (1 << 6);
     Flags<uint32_t> flags(value);
     flags.clear();
-    ASSERT_TRUE(flags.noneSet());
+    ASSERT_EQ(0, uint32_t(flags));
 }

 /** Test clearing specific bits. */

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38736
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: Ia7cbfd0726943506a3f04dc417e67a0b57cdbf95
Gerrit-Change-Number: 38736
Gerrit-PatchSet: 6
Gerrit-Owner: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Jason Lowe-Power <power...@gmail.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to