Skip to content

cmdlinet: add value_opt methods #8525

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 10, 2024
Merged

cmdlinet: add value_opt methods #8525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/goto-cc/armcc_mode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,8 @@
}

// armcc's default is .o
if(cmdline.isset("default_extension="))
compiler.object_file_extension=
cmdline.get_value("default_extension=");
else
compiler.object_file_extension="o";
compiler.object_file_extension =
cmdline.value_opt("default_extension=").value_or("o");

Check warning on line 109 in src/goto-cc/armcc_mode.cpp

View check run for this annotation

Codecov / codecov/patch

src/goto-cc/armcc_mode.cpp#L108-L109 

Added lines #L108 - L109 were not covered by tests

// note that ARM's default is "unsigned_chars",
// in contrast to gcc's default!
Expand Down
5 changes: 1 addition & 4 deletions src/goto-cc/gcc_mode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,10 +982,7 @@ int gcc_modet::gcc_hybrid_binary(compilet &compiler)
else
{
// -c is not given
if(cmdline.isset('o'))
output_files.push_back(cmdline.get_value('o'));
else
output_files.push_back("a.out");
output_files.push_back(cmdline.value_opt('o').value_or("a.out"));
}

if(output_files.empty() ||
Expand Down
8 changes: 4 additions & 4 deletions src/goto-instrument/goto_instrument_parse_options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ int goto_instrument_parse_optionst::doit()

if(cmdline.isset("log"))
{
std::string filename=cmdline.get_value("log");
bool have_file=!filename.empty() && filename!="-";
std::string filename = cmdline.value_opt("log").value_or("-");
bool have_file = filename != "-";

jsont result=goto_unwind.output_log_json();

Expand Down Expand Up @@ -1319,8 +1319,8 @@ void goto_instrument_parse_optionst::instrument_goto_program()
}
else
{
std::string filename=cmdline.get_value("log");
bool have_file=!filename.empty() && filename!="-";
std::string filename = cmdline.value_opt("log").value_or("-");
bool have_file = filename != "-";

jsont result = goto_function_inline_and_log(
goto_model, function, ui_message_handler, true, caching);
Expand Down
14 changes: 12 additions & 2 deletions src/util/cmdline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,18 @@ bool cmdlinet::isset(const char *option) const
}

std::string cmdlinet::get_value(char option) const
{
return value_opt(option).value_or("");
}

std::optional<std::string> cmdlinet::value_opt(char option) const
{
auto i=getoptnr(option);

if(i.has_value() && !options[*i].values.empty())
return options[*i].values.front();
else
return "";
return {};
}

void cmdlinet::set(const std::string &option, bool value)
Expand Down Expand Up @@ -97,13 +102,18 @@ const std::list<std::string> &cmdlinet::get_values(char option) const
}

std::string cmdlinet::get_value(const char *option) const
{
return value_opt(option).value_or("");
}

std::optional<std::string> cmdlinet::value_opt(const char *option) const
{
auto i=getoptnr(option);

if(i.has_value() && !options[*i].values.empty())
return options[*i].values.front();
else
return "";
return {};
}

const std::list<std::string> &cmdlinet::get_values(
Expand Down
3 changes: 3 additions & 0 deletions src/util/cmdline.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ class cmdlinet
std::string get_value(char option) const;
std::string get_value(const char *option) const;

std::optional<std::string> value_opt(char option) const;
std::optional<std::string> value_opt(const char *option) const;

Comment on lines +79 to +81
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about changing this API to become value_or? So that it would be used like this: cmdline.value_or("log", "-")?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be confusing given that it is the same method name as std::optional::value_or. Right now what you do is only a little bit longer: cmdline.value_opt("log").value_or("-").

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider operator()? Then it would be cmdline("log").value_or("-")

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps value_or isn't the best name to choose, but it still seems like a nuisance when the only thing we really want is value_opt immediately followed by value_or.

Copy link
Member Author

@kroening kroening Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some (very modest) benefit in doing

auto foo_opt = cmdline.value_opt("foo");
if(foo_opt) ....  foo_opt.value()

The benefit is that you avoid the double lookup in the argument map that you'd do with cmdline.isset("foo") followed by cmdline.get_value("foo"). But of course not measurable.

const std::list<std::string> &get_values(const std::string &option) const;
const std::list<std::string> &get_values(char option) const;

Expand Down
Loading