[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
This revision was automatically updated to reflect the committed changes.
Closed by commit rG182b4652e542: [StringRef] Add enable-if to StringLiteral.
(authored by zturner).
Herald added subscribers: llvm-commits, dexonsmith.
Herald added a project: LLVM.
Changed prior to commit:
https://reviews.llvm.org/D27780?vs=81625=223552#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D27780/new/
https://reviews.llvm.org/D27780
Files:
llvm/include/llvm/ADT/StringRef.h
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -838,22 +838,21 @@
/// A wrapper around a string literal that serves as a proxy for constructing
/// global tables of StringRefs with the length computed at compile time.
- /// Using this class with a non-literal char array is considered undefined
- /// behavior. To prevent this, it is recommended that StringLiteral *only*
- /// be used in a constexpr context, as such:
+ /// In order to avoid the invocation of a global constructor, StringLiteral
+ /// should *only* be used in a constexpr context, as such:
///
/// constexpr StringLiteral S("test");
///
- /// Note: There is a subtle behavioral difference in the constructor of
- /// StringRef and StringLiteral, as illustrated below:
- ///
- /// constexpr StringLiteral S("a\0b"); // S.size() == 3
- /// StringRef S("a\0b"); // S.size() == 1
- ///
class StringLiteral : public StringRef {
public:
template
-constexpr StringLiteral(const char ()[N]) : StringRef(Str, N - 1) {}
+constexpr StringLiteral(const char ()[N])
+#if __has_attribute(enable_if)
+__attribute((enable_if(__builtin_strlen(Str) == N - 1,
+ "invalid string literal")))
+#endif
+: StringRef(Str, N - 1) {
+}
};
/// @name StringRef Comparison Operators
@@ -865,9 +864,7 @@
}
LLVM_ATTRIBUTE_ALWAYS_INLINE
- inline bool operator!=(StringRef LHS, StringRef RHS) {
-return !(LHS == RHS);
- }
+ inline bool operator!=(StringRef LHS, StringRef RHS) { return !(LHS == RHS);
}
inline bool operator<(StringRef LHS, StringRef RHS) {
return LHS.compare(RHS) == -1;
Index: llvm/include/llvm/ADT/StringRef.h
===
--- llvm/include/llvm/ADT/StringRef.h
+++ llvm/include/llvm/ADT/StringRef.h
@@ -838,22 +838,21 @@
/// A wrapper around a string literal that serves as a proxy for constructing
/// global tables of StringRefs with the length computed at compile time.
- /// Using this class with a non-literal char array is considered undefined
- /// behavior. To prevent this, it is recommended that StringLiteral *only*
- /// be used in a constexpr context, as such:
+ /// In order to avoid the invocation of a global constructor, StringLiteral
+ /// should *only* be used in a constexpr context, as such:
///
/// constexpr StringLiteral S("test");
///
- /// Note: There is a subtle behavioral difference in the constructor of
- /// StringRef and StringLiteral, as illustrated below:
- ///
- /// constexpr StringLiteral S("a\0b"); // S.size() == 3
- /// StringRef S("a\0b"); // S.size() == 1
- ///
class StringLiteral : public StringRef {
public:
template
-constexpr StringLiteral(const char ()[N]) : StringRef(Str, N - 1) {}
+constexpr StringLiteral(const char ()[N])
+#if __has_attribute(enable_if)
+__attribute((enable_if(__builtin_strlen(Str) == N - 1,
+ "invalid string literal")))
+#endif
+: StringRef(Str, N - 1) {
+}
};
/// @name StringRef Comparison Operators
@@ -865,9 +864,7 @@
}
LLVM_ATTRIBUTE_ALWAYS_INLINE
- inline bool operator!=(StringRef LHS, StringRef RHS) {
-return !(LHS == RHS);
- }
+ inline bool operator!=(StringRef LHS, StringRef RHS) { return !(LHS == RHS); }
inline bool operator<(StringRef LHS, StringRef RHS) {
return LHS.compare(RHS) == -1;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
zturner added a comment. Yea as I mentioned this whole plan might be killed by a blocker I ran into last night. I'm still trying to figure out if there's a workaround. https://reviews.llvm.org/D27780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Ok, as long as the StringRef constructors are quick and efficient and not running strlen() each time I am good. https://reviews.llvm.org/D27780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
So I ran into a nasty problem doing the rest of the conversions and at the moment I'm not sure if there's even a workaround. So this is on hold while I think about it. On Thu, Dec 15, 2016 at 8:06 PM Zachary Turner via Phabricator < revi...@reviews.llvm.org> wrote: > zturner added a comment. > > Greg, did the comments about implicit construction of a `StringRef` from a > char literal being zero overhead make sense? If so, are there any other > concerns? > > > https://reviews.llvm.org/D27780 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
zturner added a comment. Greg, did the comments about implicit construction of a `StringRef` from a char literal being zero overhead make sense? If so, are there any other concerns? https://reviews.llvm.org/D27780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
zturner removed rL LLVM as the repository for this revision.
zturner updated this revision to Diff 81625.
zturner added a comment.
Re-upload the correct diff.
https://reviews.llvm.org/D27780
Files:
lldb/include/lldb/Interpreter/Options.h
lldb/include/lldb/lldb-private-types.h
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Host/common/OptionParser.cpp
lldb/source/Interpreter/Args.cpp
lldb/source/Interpreter/OptionGroupOutputFile.cpp
lldb/source/Interpreter/Options.cpp
Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -333,27 +333,14 @@
}
}
-bool Options::SupportsLongOption(const char *long_option) {
- if (!long_option || !long_option[0])
+bool Options::SupportsLongOption(llvm::StringRef long_option) {
+ long_option.consume_front("--");
+ if (long_option.empty())
return false;
- auto opt_defs = GetDefinitions();
- if (opt_defs.empty())
-return false;
-
- const char *long_option_name = long_option;
- if (long_option[0] == '-' && long_option[1] == '-')
-long_option_name += 2;
-
- for (auto : opt_defs) {
-if (!def.long_option)
- continue;
-
-if (strcmp(def.long_option, long_option_name) == 0)
- return true;
- }
-
- return false;
+ return llvm::any_of(GetDefinitions(), [=](const OptionDefinition ) {
+return D.long_option == long_option;
+ });
}
enum OptionDisplayType {
@@ -603,9 +590,10 @@
strm.IndentMore(5);
- if (opt_defs[i].usage_text)
+ if (!opt_defs[i].usage_text.empty())
OutputFormattedUsageText(strm, opt_defs[i], screen_width);
- if (opt_defs[i].enum_values != nullptr) {
+
+ if (opt_defs[i].enum_values == nullptr) {
strm.Indent();
strm.Printf("Values: ");
for (int k = 0; opt_defs[i].enum_values[k].string_value != nullptr;
@@ -670,10 +658,6 @@
auto opt_defs = GetDefinitions();
- std::string cur_opt_std_str(input.GetArgumentAtIndex(cursor_index));
- cur_opt_std_str.erase(char_pos);
- const char *cur_opt_str = cur_opt_std_str.c_str();
-
for (size_t i = 0; i < opt_element_vector.size(); i++) {
int opt_pos = opt_element_vector[i].opt_pos;
int opt_arg_pos = opt_element_vector[i].opt_arg_pos;
@@ -708,18 +692,17 @@
}
return true;
} else if (opt_defs_index != OptionArgElement::eUnrecognizedArg) {
+auto cur_opt_str = input[cursor_index].ref.take_front(char_pos);
+
// We recognized it, if it an incomplete long option, complete it anyway
-// (getopt_long_only is
-// happy with shortest unique string, but it's still a nice thing to
-// do.) Otherwise return
-// The string so the upper level code will know this is a full match and
-// add the " ".
-if (cur_opt_str && strlen(cur_opt_str) > 2 && cur_opt_str[0] == '-' &&
-cur_opt_str[1] == '-' &&
-strcmp(opt_defs[opt_defs_index].long_option, cur_opt_str) != 0) {
+// (getopt_long_only is happy with shortest unique string, but it's
+// still a nice thing to do.) Otherwise return the string so the upper
+// level code will know this is a full match and add the " ".
+if (cur_opt_str.startswith("--") &&
+cur_opt_str.drop_front(2) != opt_defs[opt_defs_index].long_option) {
std::string full_name("--");
full_name.append(opt_defs[opt_defs_index].long_option);
- matches.AppendString(full_name.c_str());
+ matches.AppendString(full_name);
return true;
} else {
matches.AppendString(input.GetArgumentAtIndex(cursor_index));
@@ -729,33 +712,32 @@
// FIXME - not handling wrong options yet:
// Check to see if they are writing a long option & complete it.
// I think we will only get in here if the long option table has two
-// elements
-// that are not unique up to this point. getopt_long_only does shortest
-// unique match
-// for long options already.
-
-if (cur_opt_str && strlen(cur_opt_str) > 2 && cur_opt_str[0] == '-' &&
-cur_opt_str[1] == '-') {
- for (auto : opt_defs) {
-if (!def.long_option)
- continue;
+// elements that are not unique up to this point. getopt_long_only
+// does shortest unique match for long options already.
+auto cur_opt_str = input[cursor_index].ref.take_front(char_pos);
-if (strstr(def.long_option, cur_opt_str + 2) == def.long_option) {
- std::string full_name("--");
- full_name.append(def.long_option);
- // The options definitions table has duplicates because of the
- // way the grouping information is stored, so only add once.
- bool duplicate = false;
- for (size_t k = 0; k <
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
zturner reopened this revision. zturner added a comment. My bad, this revision was not actually closed, I attached the wrong diff to an unrelated commit. I will need to re-upload this one. Repository: rL LLVM https://reviews.llvm.org/D27780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289853: [StringRef] Add enable-if to StringLiteral.
(authored by zturner).
Changed prior to commit:
https://reviews.llvm.org/D27780?vs=81487=81624#toc
Repository:
rL LLVM
https://reviews.llvm.org/D27780
Files:
llvm/trunk/include/llvm/ADT/StringRef.h
Index: llvm/trunk/include/llvm/ADT/StringRef.h
===
--- llvm/trunk/include/llvm/ADT/StringRef.h
+++ llvm/trunk/include/llvm/ADT/StringRef.h
@@ -838,22 +838,21 @@
/// A wrapper around a string literal that serves as a proxy for constructing
/// global tables of StringRefs with the length computed at compile time.
- /// Using this class with a non-literal char array is considered undefined
- /// behavior. To prevent this, it is recommended that StringLiteral *only*
- /// be used in a constexpr context, as such:
+ /// In order to avoid the invocation of a global constructor, StringLiteral
+ /// should *only* be used in a constexpr context, as such:
///
/// constexpr StringLiteral S("test");
///
- /// Note: There is a subtle behavioral difference in the constructor of
- /// StringRef and StringLiteral, as illustrated below:
- ///
- /// constexpr StringLiteral S("a\0b"); // S.size() == 3
- /// StringRef S("a\0b"); // S.size() == 1
- ///
class StringLiteral : public StringRef {
public:
template
-constexpr StringLiteral(const char ()[N]) : StringRef(Str, N - 1) {}
+constexpr StringLiteral(const char ()[N])
+#if __has_attribute(enable_if)
+__attribute((enable_if(__builtin_strlen(Str) == N - 1,
+ "invalid string literal")))
+#endif
+: StringRef(Str, N - 1) {
+}
};
/// @name StringRef Comparison Operators
@@ -865,9 +864,7 @@
}
LLVM_ATTRIBUTE_ALWAYS_INLINE
- inline bool operator!=(StringRef LHS, StringRef RHS) {
-return !(LHS == RHS);
- }
+ inline bool operator!=(StringRef LHS, StringRef RHS) { return !(LHS == RHS);
}
inline bool operator<(StringRef LHS, StringRef RHS) {
return LHS.compare(RHS) == -1;
Index: llvm/trunk/include/llvm/ADT/StringRef.h
===
--- llvm/trunk/include/llvm/ADT/StringRef.h
+++ llvm/trunk/include/llvm/ADT/StringRef.h
@@ -838,22 +838,21 @@
/// A wrapper around a string literal that serves as a proxy for constructing
/// global tables of StringRefs with the length computed at compile time.
- /// Using this class with a non-literal char array is considered undefined
- /// behavior. To prevent this, it is recommended that StringLiteral *only*
- /// be used in a constexpr context, as such:
+ /// In order to avoid the invocation of a global constructor, StringLiteral
+ /// should *only* be used in a constexpr context, as such:
///
/// constexpr StringLiteral S("test");
///
- /// Note: There is a subtle behavioral difference in the constructor of
- /// StringRef and StringLiteral, as illustrated below:
- ///
- /// constexpr StringLiteral S("a\0b"); // S.size() == 3
- /// StringRef S("a\0b"); // S.size() == 1
- ///
class StringLiteral : public StringRef {
public:
template
-constexpr StringLiteral(const char ()[N]) : StringRef(Str, N - 1) {}
+constexpr StringLiteral(const char ()[N])
+#if __has_attribute(enable_if)
+__attribute((enable_if(__builtin_strlen(Str) == N - 1,
+ "invalid string literal")))
+#endif
+: StringRef(Str, N - 1) {
+}
};
/// @name StringRef Comparison Operators
@@ -865,9 +864,7 @@
}
LLVM_ATTRIBUTE_ALWAYS_INLINE
- inline bool operator!=(StringRef LHS, StringRef RHS) {
-return !(LHS == RHS);
- }
+ inline bool operator!=(StringRef LHS, StringRef RHS) { return !(LHS == RHS); }
inline bool operator<(StringRef LHS, StringRef RHS) {
return LHS.compare(RHS) == -1;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
labath added a comment. Looks reasonable. I look forward to using StringRef in more places. https://reviews.llvm.org/D27780 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
zturner added inline comments.
Comment at: lldb/source/Interpreter/Options.cpp:728
+for (auto : range) {
+ std::string full_name("--");
+ full_name.append(def.long_option);
clayborg wrote:
> Do we still need std::string here for full_name? We might be able to do
> smarter things with StringRef for all uses of full_name below, including the
> matches.GetStringAtIndex() by seeing if the string at index starts with "--",
> and then just comparing the remainder to "def.long_option"?
Yes, I tried that at first, but soon after noticed that `matches` is an output
parameter, so we would have to fix up the caller to stop making assumptions
that the `--` is tacked onto the beginning. Certainly doable, but it has
potential for growing into a large CL depending on how far down the rabbit hole
we'd have to go to fix everything.
https://reviews.llvm.org/D27780
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
zturner added a comment.
In response to all the questions about "Will a StringRef constructor be called
on XXX", the answer is usually yes, but the constructor will be inlined and
invoke `__builtin_strlen` (which is constexpr on GCC and Clang) when used on a
string literal. In other words, there is zero overhead.
So the code generated for `S.startswith("--")` will end up just being `(2 >=
S.Length) && (::memcmp(S.Data, "--", 2) == 0)`.
The only time we have to worry about constexprness and make sure we use
`StringLiteral` is with static variables (global or local). With static
variables, the compiler has to decide between static link-time initialization
or runtime initialization, and only if you declare the variable as `constexpr`
will it choose link-time initialization. So the generated code for these two
might look something like this:
// No generated code, S is initialized at link-time.
constexpr StringLiteral S("foo");
// mov ,
// mov , 3
StringLiteral T("foo");
https://reviews.llvm.org/D27780
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
clayborg added a comment.
There seems to be a bunch of places that we are passing a string literal to
functions that take a StringRef. Can we pass L("--") instead of "--" to
functions that take a string ref and have it be more efficient so a StringRef
isn't implicitly constructed for us each time? See inlined comments for places.
Comment at: lldb/source/Interpreter/Args.cpp:954
+ llvm::SmallString<3> short_buffer =
+ llvm::formatv("-{0}", (char)long_options[long_options_index].val);
+ llvm::SmallString<255> long_buffer = llvm::formatv(
Will this line cause a StringRef constructor to be called on "-{0}"? If so, is
there a way to avoid this with some constexpr magic?
Comment at: lldb/source/Interpreter/Args.cpp:956
+ llvm::SmallString<255> long_buffer = llvm::formatv(
+ "--{0}", long_options[long_options_index].definition->long_option);
Ditto
Comment at: lldb/source/Interpreter/Options.cpp:337
+bool Options::SupportsLongOption(llvm::StringRef long_option) {
+ long_option.consume_front("--");
+ if (long_option.empty())
Will this line cause a StringRef constructor to be called on "--"?
Comment at: lldb/source/Interpreter/Options.cpp:701
+// level code will know this is a full match and add the " ".
+if (cur_opt_str.startswith("--") &&
+cur_opt_str.drop_front(2) != opt_defs[opt_defs_index].long_option)
{
Will this line cause a StringRef constructor to be called on "--"?
Comment at: lldb/source/Interpreter/Options.cpp:719
-if (strstr(def.long_option, cur_opt_str + 2) == def.long_option) {
- std::string full_name("--");
- full_name.append(def.long_option);
- // The options definitions table has duplicates because of the
- // way the grouping information is stored, so only add once.
- bool duplicate = false;
- for (size_t k = 0; k < matches.GetSize(); k++) {
-if (matches.GetStringAtIndex(k) == full_name) {
- duplicate = true;
- break;
-}
- }
- if (!duplicate)
-matches.AppendString(full_name.c_str());
+if (!cur_opt_str.consume_front("--"))
+ return true;
Will this line cause a StringRef constructor to be called on "--"?
Comment at: lldb/source/Interpreter/Options.cpp:728
+for (auto : range) {
+ std::string full_name("--");
+ full_name.append(def.long_option);
Do we still need std::string here for full_name? We might be able to do smarter
things with StringRef for all uses of full_name below, including the
matches.GetStringAtIndex() by seeing if the string at index starts with "--",
and then just comparing the remainder to "def.long_option"?
Comment at: lldb/source/Interpreter/Options.cpp:834
// restrict it to that shared library.
- if (cur_opt_name && strcmp(cur_opt_name, "shlib") == 0 &&
- cur_arg_pos != -1) {
-const char *module_name = input.GetArgumentAtIndex(cur_arg_pos);
-if (module_name) {
+ if (cur_opt_name == "shlib" && cur_arg_pos != -1) {
+auto module_name = input[cur_arg_pos].ref;
Will this line cause a StringRef constructor to be called on "shlib"?
https://reviews.llvm.org/D27780
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef
zturner created this revision.
zturner added reviewers: clayborg, labath.
zturner added a subscriber: lldb-commits.
The major blocker to this until now has been that we can't have global
constructors, no matter how trivial.
Recently LLVM obtained the `StringLiteral` class which addresses this. This
class has a `constexpr` constructor, so as long as a global `StringRef` (or
class/struct containing a `StringRef` is declared `constexpr`, the `StringRef`
can be initialized with a `StringLiteral` and not invoke a global constructor.
This probably sounds confusing, but in practice it's simple. It looks like
this:
// ok
constexpr StringRef S = llvm::StringLiteral("foo");
// ok
constexpr StringLiteral T("foo");
// not ok, global constructor invoked.
StringLiteral U("foo");
struct Foo {
constexpr Foo(StringRef S) : S(S) {}
StringRef S;
};
// ok
constexpr Foo F(llvm::StringLiteral("foo"));
Admittedly, it's pretty verbose to type out `StringLiteral` everywhere. To get
around this, I alias it to `L` in the individual TUs where we use it. This way
we can write:
using L = llvm::StringLiteral;
static constexpr OptionDefinition[] g_defs = { ..., ..., L("opt"), ..., ...,
L("help text") };
Again, note that no global constructor is invoked here, even though this is an
array.
The advantage of doing all of this is that we can now have access to all the
wonderful member functions of `StringRef`. Upon fixing up all the compilation
failures (there were very few however) triggered by the change, we can already
see some code being simplified, and in the future I would like to extend this
to other classes like `RegisterInfo` etc as well.
Note that this patch is incomplete as is. Before submitting, I would need to
go change EVERY global definition of `OptionDefinition` to be marked
`constexpr`, otherwise a global constructor would be invoked. I intentionally
did it in only 2 places here, just to illustrate the idea, before changing the
code everywhere.
https://reviews.llvm.org/D27780
Files:
lldb/include/lldb/Interpreter/Options.h
lldb/include/lldb/lldb-private-types.h
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Host/common/OptionParser.cpp
lldb/source/Interpreter/Args.cpp
lldb/source/Interpreter/OptionGroupOutputFile.cpp
lldb/source/Interpreter/Options.cpp
Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -333,27 +333,14 @@
}
}
-bool Options::SupportsLongOption(const char *long_option) {
- if (!long_option || !long_option[0])
+bool Options::SupportsLongOption(llvm::StringRef long_option) {
+ long_option.consume_front("--");
+ if (long_option.empty())
return false;
- auto opt_defs = GetDefinitions();
- if (opt_defs.empty())
-return false;
-
- const char *long_option_name = long_option;
- if (long_option[0] == '-' && long_option[1] == '-')
-long_option_name += 2;
-
- for (auto : opt_defs) {
-if (!def.long_option)
- continue;
-
-if (strcmp(def.long_option, long_option_name) == 0)
- return true;
- }
-
- return false;
+ return llvm::any_of(GetDefinitions(), [=](const OptionDefinition ) {
+return D.long_option == long_option;
+ });
}
enum OptionDisplayType {
@@ -603,9 +590,10 @@
strm.IndentMore(5);
- if (opt_defs[i].usage_text)
+ if (!opt_defs[i].usage_text.empty())
OutputFormattedUsageText(strm, opt_defs[i], screen_width);
- if (opt_defs[i].enum_values != nullptr) {
+
+ if (opt_defs[i].enum_values == nullptr) {
strm.Indent();
strm.Printf("Values: ");
for (int k = 0; opt_defs[i].enum_values[k].string_value != nullptr;
@@ -670,10 +658,6 @@
auto opt_defs = GetDefinitions();
- std::string cur_opt_std_str(input.GetArgumentAtIndex(cursor_index));
- cur_opt_std_str.erase(char_pos);
- const char *cur_opt_str = cur_opt_std_str.c_str();
-
for (size_t i = 0; i < opt_element_vector.size(); i++) {
int opt_pos = opt_element_vector[i].opt_pos;
int opt_arg_pos = opt_element_vector[i].opt_arg_pos;
@@ -708,18 +692,17 @@
}
return true;
} else if (opt_defs_index != OptionArgElement::eUnrecognizedArg) {
+auto cur_opt_str = input[cursor_index].ref.take_front(char_pos);
+
// We recognized it, if it an incomplete long option, complete it anyway
-// (getopt_long_only is
-// happy with shortest unique string, but it's still a nice thing to
-// do.) Otherwise return
-// The string so the upper level code will know this is a full match and
-// add the " ".
-if (cur_opt_str && strlen(cur_opt_str) > 2 && cur_opt_str[0] == '-' &&
-cur_opt_str[1] == '-' &&
-strcmp(opt_defs[opt_defs_index].long_option, cur_opt_str) != 0) {
+// (getopt_long_only is
