Skip to content

Commit

Permalink
src: reduce to simple const char* in OptionsParser
Browse files Browse the repository at this point in the history 
> A lot of the `std::string` usage here could be reduced
  to simple `const char*`s if it's reasonable to expect the values to be
  known at compile-time.

So this commit uses `const char*` to replace most of `std::string` in
`OptionsParser`.

PR-URL: #26297
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
@ZYSzys @BethGriggs
ZYSzys authored and BethGriggs committed Apr 16, 2019
1 parent 6d4731e commit 0fa3a51
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 49 deletions.
48 changes: 24 additions & 24 deletions src/node_options-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ EnvironmentOptions* PerIsolateOptions::get_per_env_options() {
namespace options_parser {

template <typename Options>
void OptionsParser<Options>::AddOption(const std::string& name,
const std::string& help_text,
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
bool Options::* field,
OptionEnvvarSettings env_setting) {
options_.emplace(name,
Expand All @@ -36,8 +36,8 @@ void OptionsParser<Options>::AddOption(const std::string& name,
}

template <typename Options>
void OptionsParser<Options>::AddOption(const std::string& name,
const std::string& help_text,
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
uint64_t Options::* field,
OptionEnvvarSettings env_setting) {
options_.emplace(
Expand All @@ -49,8 +49,8 @@ void OptionsParser<Options>::AddOption(const std::string& name,
}

template <typename Options>
void OptionsParser<Options>::AddOption(const std::string& name,
const std::string& help_text,
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
int64_t Options::* field,
OptionEnvvarSettings env_setting) {
options_.emplace(
Expand All @@ -62,8 +62,8 @@ void OptionsParser<Options>::AddOption(const std::string& name,
}

template <typename Options>
void OptionsParser<Options>::AddOption(const std::string& name,
const std::string& help_text,
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
std::string Options::* field,
OptionEnvvarSettings env_setting) {
options_.emplace(
Expand All @@ -76,8 +76,8 @@ void OptionsParser<Options>::AddOption(const std::string& name,

template <typename Options>
void OptionsParser<Options>::AddOption(
const std::string& name,
const std::string& help_text,
const char* name,
const char* help_text,
std::vector<std::string> Options::* field,
OptionEnvvarSettings env_setting) {
options_.emplace(name, OptionInfo {
Expand All @@ -89,8 +89,8 @@ void OptionsParser<Options>::AddOption(
}

template <typename Options>
void OptionsParser<Options>::AddOption(const std::string& name,
const std::string& help_text,
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
HostPort Options::* field,
OptionEnvvarSettings env_setting) {
options_.emplace(
Expand All @@ -102,44 +102,44 @@ void OptionsParser<Options>::AddOption(const std::string& name,
}

template <typename Options>
void OptionsParser<Options>::AddOption(const std::string& name,
const std::string& help_text,
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
NoOp no_op_tag,
OptionEnvvarSettings env_setting) {
options_.emplace(name, OptionInfo{kNoOp, nullptr, env_setting, help_text});
}

template <typename Options>
void OptionsParser<Options>::AddOption(const std::string& name,
const std::string& help_text,
void OptionsParser<Options>::AddOption(const char* name,
const char* help_text,
V8Option v8_option_tag,
OptionEnvvarSettings env_setting) {
options_.emplace(name,
OptionInfo{kV8Option, nullptr, env_setting, help_text});
}

template <typename Options>
void OptionsParser<Options>::AddAlias(const std::string& from,
const std::string& to) {
void OptionsParser<Options>::AddAlias(const char* from,
const char* to) {
aliases_[from] = { to };
}

template <typename Options>
void OptionsParser<Options>::AddAlias(const std::string& from,
void OptionsParser<Options>::AddAlias(const char* from,
const std::vector<std::string>& to) {
aliases_[from] = to;
}

template <typename Options>
void OptionsParser<Options>::AddAlias(
const std::string& from,
const char* from,
const std::initializer_list<std::string>& to) {
AddAlias(from, std::vector<std::string>(to));
}

template <typename Options>
void OptionsParser<Options>::Implies(const std::string& from,
const std::string& to) {
void OptionsParser<Options>::Implies(const char* from,
const char* to) {
auto it = options_.find(to);
CHECK_NE(it, options_.end());
CHECK_EQ(it->second.type, kBoolean);
Expand All @@ -149,8 +149,8 @@ void OptionsParser<Options>::Implies(const std::string& from,
}

template <typename Options>
void OptionsParser<Options>::ImpliesNot(const std::string& from,
const std::string& to) {
void OptionsParser<Options>::ImpliesNot(const char* from,
const char* to) {
auto it = options_.find(to);
CHECK_NE(it, options_.end());
CHECK_EQ(it->second.type, kBoolean);
Expand Down
46 changes: 21 additions & 25 deletions src/node_options.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,43 +185,39 @@ class OptionsParser {
struct NoOp {};
struct V8Option {};

// TODO(addaleax): A lot of the `std::string` usage here could be reduced
// to simple `const char*`s if it's reasonable to expect the values to be
// known at compile-time.

// These methods add a single option to the parser. Optionally, it can be
// specified whether the option should be allowed from environment variable
// sources (i.e. NODE_OPTIONS).
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
bool Options::* field,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
uint64_t Options::* field,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
int64_t Options::* field,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
std::string Options::* field,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
std::vector<std::string> Options::* field,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
HostPort Options::* field,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
NoOp no_op_tag,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);
void AddOption(const std::string& name,
const std::string& help_text,
void AddOption(const char* name,
const char* help_text,
V8Option v8_option_tag,
OptionEnvvarSettings env_setting = kDisallowedInEnvironment);

Expand All @@ -232,15 +228,15 @@ class OptionsParser {
// the option is presented in that form (i.e. with a '=').
// If `from` has the form "--option-a <arg>", the alias will only be expanded
// if the option has a non-option argument (not starting with -) following it.
void AddAlias(const std::string& from, const std::string& to);
void AddAlias(const std::string& from, const std::vector<std::string>& to);
void AddAlias(const std::string& from,
void AddAlias(const char* from, const char* to);
void AddAlias(const char* from, const std::vector<std::string>& to);
void AddAlias(const char* from,
const std::initializer_list<std::string>& to);

// Add implications from some arbitrary option to a boolean one, either
// in a way that makes `from` set `to` to true or to false.
void Implies(const std::string& from, const std::string& to);
void ImpliesNot(const std::string& from, const std::string& to);
void Implies(const char* from, const char* to);
void ImpliesNot(const char* from, const char* to);

// Insert options from another options parser into this one, along with
// a method that yields the target options type from this parser's options
Expand Down

0 comments on commit 0fa3a51

Please sign in to comment.