[flags] Refactor MaybeBoolFlag to use base::Optional

Use the existing {base::Optional} instead of the extra {MaybeBoolFlag}
struct. This makes writing to a maybe-flag simpler because you just
write a boolean value and that automatically initializes the optional.

R=cbruni@chromium.org

Bug: v8:12887
Change-Id: I940d20286d65ba4355dc04b4b6068a306706f295
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3686412
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80915}
This commit is contained in:
Clemens Backes 2022-06-02 14:12:07 +02:00 committed by V8 LUCI CQ
parent f363be9c66
commit 22b4ab86ef
6 changed files with 42 additions and 61 deletions

View File

@ -2685,7 +2685,10 @@ v8_header_set("v8_flags") {
"src/flags/flags.h",
]
deps = [ ":v8_shared_internal_headers" ]
deps = [
":v8_libbase",
":v8_shared_internal_headers",
]
}
v8_header_set("v8_internal_headers") {

View File

@ -81,9 +81,9 @@ static unsigned CpuFeaturesFromCommandLine() {
// If any of the old (deprecated) flags are specified, print a warning, but
// otherwise try to respect them for now.
// TODO(jbramley): When all the old bots have been updated, remove this.
if (FLAG_enable_armv7.has_value || FLAG_enable_vfp3.has_value ||
FLAG_enable_32dregs.has_value || FLAG_enable_neon.has_value ||
FLAG_enable_sudiv.has_value || FLAG_enable_armv8.has_value) {
if (FLAG_enable_armv7.has_value() || FLAG_enable_vfp3.has_value() ||
FLAG_enable_32dregs.has_value() || FLAG_enable_neon.has_value() ||
FLAG_enable_sudiv.has_value() || FLAG_enable_armv8.has_value()) {
// As an approximation of the old behaviour, set the default values from the
// arm_arch setting, then apply the flags over the top.
bool enable_armv7 = (result & (1u << ARMv7)) != 0;
@ -92,41 +92,41 @@ static unsigned CpuFeaturesFromCommandLine() {
bool enable_neon = (result & (1u << ARMv7)) != 0;
bool enable_sudiv = (result & (1u << ARMv7_SUDIV)) != 0;
bool enable_armv8 = (result & (1u << ARMv8)) != 0;
if (FLAG_enable_armv7.has_value) {
if (FLAG_enable_armv7.has_value()) {
fprintf(stderr,
"Warning: --enable_armv7 is deprecated. "
"Use --arm_arch instead.\n");
enable_armv7 = FLAG_enable_armv7.value;
enable_armv7 = FLAG_enable_armv7.value();
}
if (FLAG_enable_vfp3.has_value) {
if (FLAG_enable_vfp3.has_value()) {
fprintf(stderr,
"Warning: --enable_vfp3 is deprecated. "
"Use --arm_arch instead.\n");
enable_vfp3 = FLAG_enable_vfp3.value;
enable_vfp3 = FLAG_enable_vfp3.value();
}
if (FLAG_enable_32dregs.has_value) {
if (FLAG_enable_32dregs.has_value()) {
fprintf(stderr,
"Warning: --enable_32dregs is deprecated. "
"Use --arm_arch instead.\n");
enable_32dregs = FLAG_enable_32dregs.value;
enable_32dregs = FLAG_enable_32dregs.value();
}
if (FLAG_enable_neon.has_value) {
if (FLAG_enable_neon.has_value()) {
fprintf(stderr,
"Warning: --enable_neon is deprecated. "
"Use --arm_arch instead.\n");
enable_neon = FLAG_enable_neon.value;
enable_neon = FLAG_enable_neon.value();
}
if (FLAG_enable_sudiv.has_value) {
if (FLAG_enable_sudiv.has_value()) {
fprintf(stderr,
"Warning: --enable_sudiv is deprecated. "
"Use --arm_arch instead.\n");
enable_sudiv = FLAG_enable_sudiv.value;
enable_sudiv = FLAG_enable_sudiv.value();
}
if (FLAG_enable_armv8.has_value) {
if (FLAG_enable_armv8.has_value()) {
fprintf(stderr,
"Warning: --enable_armv8 is deprecated. "
"Use --arm_arch instead.\n");
enable_armv8 = FLAG_enable_armv8.value;
enable_armv8 = FLAG_enable_armv8.value();
}
// Emulate the old implications.
if (enable_armv8) {

View File

@ -122,26 +122,6 @@
#define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value)
#endif
#define COMMA ,
#ifdef FLAG_MODE_DECLARE
struct MaybeBoolFlag {
static MaybeBoolFlag Create(bool has_value, bool value) {
MaybeBoolFlag flag;
flag.has_value = has_value;
flag.value = value;
return flag;
}
bool has_value;
bool value;
bool operator!=(const MaybeBoolFlag& other) const {
return has_value != other.has_value || value != other.value;
}
};
#endif
#ifdef DEBUG
#define DEBUG_BOOL true
#else
@ -243,7 +223,7 @@ struct MaybeBoolFlag {
#define DEFINE_BOOL_READONLY(nam, def, cmt) \
FLAG_READONLY(BOOL, bool, nam, def, cmt)
#define DEFINE_MAYBE_BOOL(nam, cmt) \
FLAG(MAYBE_BOOL, MaybeBoolFlag, nam, {false COMMA false}, cmt)
FLAG(MAYBE_BOOL, base::Optional<bool>, nam, base::nullopt, cmt)
#define DEFINE_INT(nam, def, cmt) FLAG(INT, int, nam, def, cmt)
#define DEFINE_UINT(nam, def, cmt) FLAG(UINT, unsigned int, nam, def, cmt)
#define DEFINE_UINT_READONLY(nam, def, cmt) \
@ -2316,5 +2296,3 @@ DEFINE_BOOL(enable_embedded_constant_pool, V8_EMBEDDED_CONSTANT_POOL,
#undef FLAG_MODE_META
#undef FLAG_MODE_DEFINE_IMPLICATIONS
#undef FLAG_MODE_APPLY
#undef COMMA

View File

@ -93,16 +93,17 @@ struct Flag {
if (change_flag) *reinterpret_cast<bool*>(valptr_) = value;
}
MaybeBoolFlag maybe_bool_variable() const {
base::Optional<bool> maybe_bool_variable() const {
DCHECK(type_ == TYPE_MAYBE_BOOL);
return *reinterpret_cast<MaybeBoolFlag*>(valptr_);
return *reinterpret_cast<base::Optional<bool>*>(valptr_);
}
void set_maybe_bool_variable(MaybeBoolFlag value, SetBy set_by) {
void set_maybe_bool_variable(base::Optional<bool> value, SetBy set_by) {
DCHECK(type_ == TYPE_MAYBE_BOOL);
bool change_flag = *reinterpret_cast<MaybeBoolFlag*>(valptr_) != value;
bool change_flag =
*reinterpret_cast<base::Optional<bool>*>(valptr_) != value;
change_flag = CheckFlagChange(set_by, change_flag);
if (change_flag) *reinterpret_cast<MaybeBoolFlag*>(valptr_) = value;
if (change_flag) *reinterpret_cast<base::Optional<bool>*>(valptr_) = value;
}
int int_variable() const {
@ -324,7 +325,7 @@ struct Flag {
case TYPE_BOOL:
return bool_variable() == bool_default();
case TYPE_MAYBE_BOOL:
return maybe_bool_variable().has_value == false;
return maybe_bool_variable().has_value() == false;
case TYPE_INT:
return int_variable() == int_default();
case TYPE_UINT:
@ -353,8 +354,7 @@ struct Flag {
set_bool_variable(bool_default(), SetBy::kDefault);
break;
case TYPE_MAYBE_BOOL:
set_maybe_bool_variable(MaybeBoolFlag::Create(false, false),
SetBy::kDefault);
set_maybe_bool_variable(base::nullopt, SetBy::kDefault);
break;
case TYPE_INT:
set_int_variable(int_default(), SetBy::kDefault);
@ -460,8 +460,8 @@ std::ostream& operator<<(std::ostream& os, const FlagValue& flag_value) {
os << (flag.bool_variable() ? "true" : "false");
break;
case Flag::TYPE_MAYBE_BOOL:
os << (flag.maybe_bool_variable().has_value
? (flag.maybe_bool_variable().value ? "true" : "false")
os << (flag.maybe_bool_variable().has_value()
? (flag.maybe_bool_variable().value() ? "true" : "false")
: "unset");
break;
case Flag::TYPE_INT:
@ -643,8 +643,7 @@ int FlagList::SetFlagsFromCommandLine(int* argc, char** argv, bool remove_flags,
flag->set_bool_variable(!negated, Flag::SetBy::kCommandLine);
break;
case Flag::TYPE_MAYBE_BOOL:
flag->set_maybe_bool_variable(MaybeBoolFlag::Create(true, !negated),
Flag::SetBy::kCommandLine);
flag->set_maybe_bool_variable(!negated, Flag::SetBy::kCommandLine);
break;
case Flag::TYPE_INT:
flag->set_int_variable(static_cast<int>(strtol(value, &endp, 10)),

View File

@ -5,6 +5,7 @@
#ifndef V8_FLAGS_FLAGS_H_
#define V8_FLAGS_FLAGS_H_
#include "src/base/optional.h"
#include "src/common/globals.h"
namespace v8 {

View File

@ -69,8 +69,8 @@ TEST_F(FlagsTest, Flags2) {
false));
CHECK_EQ(8, argc);
CHECK(!FLAG_testing_bool_flag);
CHECK(FLAG_testing_maybe_bool_flag.has_value);
CHECK(!FLAG_testing_maybe_bool_flag.value);
CHECK(FLAG_testing_maybe_bool_flag.has_value());
CHECK(!FLAG_testing_maybe_bool_flag.value());
CHECK_EQ(77, FLAG_testing_int_flag);
CHECK_EQ(.25, FLAG_testing_float_flag);
CHECK_EQ(0, strcmp(FLAG_testing_string_flag, "no way!"));
@ -85,8 +85,8 @@ TEST_F(FlagsTest, Flags2b) {
"--testing_string_flag no_way! ";
CHECK_EQ(0, FlagList::SetFlagsFromString(str, strlen(str)));
CHECK(!FLAG_testing_bool_flag);
CHECK(FLAG_testing_maybe_bool_flag.has_value);
CHECK(!FLAG_testing_maybe_bool_flag.value);
CHECK(FLAG_testing_maybe_bool_flag.has_value());
CHECK(!FLAG_testing_maybe_bool_flag.value());
CHECK_EQ(77, FLAG_testing_int_flag);
CHECK_EQ(.25, FLAG_testing_float_flag);
CHECK_EQ(0, strcmp(FLAG_testing_string_flag, "no_way!"));
@ -108,8 +108,8 @@ TEST_F(FlagsTest, Flags3) {
true));
CHECK_EQ(2, argc);
CHECK(FLAG_testing_bool_flag);
CHECK(FLAG_testing_maybe_bool_flag.has_value);
CHECK(FLAG_testing_maybe_bool_flag.value);
CHECK(FLAG_testing_maybe_bool_flag.has_value());
CHECK(FLAG_testing_maybe_bool_flag.value());
CHECK_EQ(-666, FLAG_testing_int_flag);
CHECK_EQ(-12E10, FLAG_testing_float_flag);
CHECK_EQ(0, strcmp(FLAG_testing_string_flag, "foo-bar"));
@ -124,8 +124,8 @@ TEST_F(FlagsTest, Flags3b) {
"-testing-string-flag=foo-bar";
CHECK_EQ(0, FlagList::SetFlagsFromString(str, strlen(str)));
CHECK(FLAG_testing_bool_flag);
CHECK(FLAG_testing_maybe_bool_flag.has_value);
CHECK(FLAG_testing_maybe_bool_flag.value);
CHECK(FLAG_testing_maybe_bool_flag.has_value());
CHECK(FLAG_testing_maybe_bool_flag.value());
CHECK_EQ(-666, FLAG_testing_int_flag);
CHECK_EQ(-12E10, FLAG_testing_float_flag);
CHECK_EQ(0, strcmp(FLAG_testing_string_flag, "foo-bar"));
@ -138,14 +138,14 @@ TEST_F(FlagsTest, Flags4) {
CHECK_EQ(0, FlagList::SetFlagsFromCommandLine(&argc, const_cast<char**>(argv),
true));
CHECK_EQ(2, argc);
CHECK(!FLAG_testing_maybe_bool_flag.has_value);
CHECK(!FLAG_testing_maybe_bool_flag.has_value());
}
TEST_F(FlagsTest, Flags4b) {
SetFlagsToDefault();
const char* str = "--testing_bool_flag --foo";
CHECK_EQ(2, FlagList::SetFlagsFromString(str, strlen(str)));
CHECK(!FLAG_testing_maybe_bool_flag.has_value);
CHECK(!FLAG_testing_maybe_bool_flag.has_value());
}
TEST_F(FlagsTest, Flags5) {