[Lldb-commits] [lldb] r283157 - Refactor the Args class.
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 3 15:51:09 PDT 2016
Author: zturner
Date: Mon Oct 3 17:51:09 2016
New Revision: 283157
URL: http://llvm.org/viewvc/llvm-project?rev=283157&view=rev
Log:
Refactor the Args class.
There were a number of issues with the Args class preventing
efficient use of strings and incoporating LLVM's StringRef class.
The two biggest were:
1. Backing memory stored in a std::string, so we would frequently
have to use const_cast to get a mutable buffer for passing to
various low level APIs.
2. backing std::strings stored in a std::list, which doesn't
provide random access.
I wanted to solve these two issues so that we could provide
StringRef access to the underlying arguments, and also a way
to provide range-based access to the underlying argument array
while still providing convenient c-style access via an argv style
const char**.
The solution here is to store arguments in a single "entry" class
which contains the backing memory, a StringRef with precomputed
length, and the quote char. The backing memory is a manually
allocated const char* so that it is not invalidated when the
container is resized, and there is a separate argv array provided
for c-style access.
Differential revision: https://reviews.llvm.org/D25099
Modified:
lldb/trunk/include/lldb/Interpreter/Args.h
lldb/trunk/source/Core/Logging.cpp
lldb/trunk/source/Core/StringList.cpp
lldb/trunk/source/Interpreter/Args.cpp
lldb/trunk/unittests/Interpreter/TestArgs.cpp
Modified: lldb/trunk/include/lldb/Interpreter/Args.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=283157&r1=283156&r2=283157&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Interpreter/Args.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Args.h Mon Oct 3 17:51:09 2016
@@ -58,6 +58,22 @@ typedef std::vector<OptionArgElement> Op
//----------------------------------------------------------------------
class Args {
public:
+ struct ArgEntry {
+ private:
+ friend class Args;
+ std::unique_ptr<char[]> ptr;
+
+ char *data() { return ptr.get(); }
+
+ public:
+ ArgEntry() = default;
+ ArgEntry(llvm::StringRef str, char quote);
+
+ llvm::StringRef ref;
+ char quote;
+ const char *c_str() const { return ptr.get(); }
+ };
+
//------------------------------------------------------------------
/// Construct with an option command string.
///
@@ -71,7 +87,7 @@ public:
Args(const Args &rhs);
- const Args &operator=(const Args &rhs);
+ Args &operator=(const Args &rhs);
//------------------------------------------------------------------
/// Destructor.
@@ -186,50 +202,6 @@ public:
void AppendArguments(const char **argv);
- // Delete const char* versions of StringRef functions. Normally this would
- // not be necessary, as const char * is implicitly convertible to StringRef.
- // However, since the use of const char* is so pervasive, and since StringRef
- // will assert if you try to construct one from nullptr, this allows the
- // compiler to catch instances of the function being invoked with a
- // const char *, allowing us to replace them with explicit conversions at each
- // call-site. This ensures that no callsites slip through the cracks where
- // we would be trying to implicitly convert from nullptr, since it will force
- // us to evaluate and explicitly convert each one.
- //
- // Once StringRef use becomes more pervasive, there will be fewer
- // implicit conversions because we will be using StringRefs across the whole
- // pipeline, so we won't have to have this "glue" that converts between the
- // two, and at that point it becomes easy to just make sure you don't pass
- // nullptr into the function on the odd occasion that you do pass a
- // const char *.
- // Call-site fixing methodology:
- // 1. If you know the string cannot be null (e.g. it's a const char[], or
- // it's been checked for null), use llvm::StringRef(ptr).
- // 2. If you don't know if it can be null (e.g. it's returned from a
- // function whose semantics are unclear), use
- // llvm::StringRef::withNullAsEmpty(ptr).
- // 3. If it's .c_str() of a std::string, just pass the std::string directly.
- // 4. If it's .str().c_str() of a StringRef, just pass the StringRef
- // directly.
- void ReplaceArgumentAtIndex(size_t, const char *, char = '\0') = delete;
- void AppendArgument(const char *arg_str, char quote_char = '\0') = delete;
- void InsertArgumentAtIndex(size_t, const char *, char = '\0') = delete;
- static bool StringToBoolean(const char *, bool, bool *) = delete;
- static lldb::ScriptLanguage
- StringToScriptLanguage(const char *, lldb::ScriptLanguage, bool *) = delete;
- static lldb::Encoding
- StringToEncoding(const char *,
- lldb::Encoding = lldb::eEncodingInvalid) = delete;
- static uint32_t StringToGenericRegister(const char *) = delete;
- static bool StringToVersion(const char *, uint32_t &, uint32_t &,
- uint32_t &) = delete;
- const char *Unshift(const char *, char = '\0') = delete;
- void AddOrReplaceEnvironmentVariable(const char *, const char *) = delete;
- bool ContainsEnvironmentVariable(const char *,
- size_t * = nullptr) const = delete;
- static int64_t StringToOptionEnum(const char *, OptionEnumValueElement *,
- int32_t, Error &) = delete;
-
//------------------------------------------------------------------
/// Insert the argument value at index \a idx to \a arg_cstr.
///
@@ -456,11 +428,6 @@ public:
static std::string EscapeLLDBCommandArgument(const std::string &arg,
char quote_char);
- // This one isn't really relevant to Arguments per se, but we're using the
- // Args as a
- // general strings container, so...
- void LongestCommonPrefix(std::string &common_prefix);
-
//------------------------------------------------------------------
/// Add or replace an environment variable with the given value.
///
@@ -493,22 +460,11 @@ public:
bool ContainsEnvironmentVariable(llvm::StringRef env_var_name,
size_t *argument_index = nullptr) const;
-protected:
- //------------------------------------------------------------------
- // Classes that inherit from Args can see and modify these
- //------------------------------------------------------------------
- typedef std::list<std::string> arg_sstr_collection;
- typedef std::vector<const char *> arg_cstr_collection;
- typedef std::vector<char> arg_quote_char_collection;
- arg_sstr_collection m_args;
- arg_cstr_collection m_argv; ///< The current argument vector.
- arg_quote_char_collection m_args_quote_char;
+private:
+ std::vector<ArgEntry> m_entries;
+ std::vector<char *> m_argv;
void UpdateArgsAfterOptionParsing();
-
- void UpdateArgvFromArgs();
-
- llvm::StringRef ParseSingleArgument(llvm::StringRef command);
};
} // namespace lldb_private
Modified: lldb/trunk/source/Core/Logging.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Logging.cpp?rev=283157&r1=283156&r2=283157&view=diff
==============================================================================
--- lldb/trunk/source/Core/Logging.cpp (original)
+++ lldb/trunk/source/Core/Logging.cpp Mon Oct 3 17:51:09 2016
@@ -93,7 +93,7 @@ void lldb_private::DisableLog(const char
if (log != nullptr) {
uint32_t flag_bits = 0;
- if (categories[0] != nullptr) {
+ if (categories && categories[0]) {
flag_bits = log->GetMask().Get();
for (size_t i = 0; categories[i] != nullptr; ++i) {
const char *arg = categories[i];
Modified: lldb/trunk/source/Core/StringList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/StringList.cpp?rev=283157&r1=283156&r2=283157&view=diff
==============================================================================
--- lldb/trunk/source/Core/StringList.cpp (original)
+++ lldb/trunk/source/Core/StringList.cpp Mon Oct 3 17:51:09 2016
@@ -103,34 +103,21 @@ void StringList::Join(const char *separa
void StringList::Clear() { m_strings.clear(); }
void StringList::LongestCommonPrefix(std::string &common_prefix) {
- const size_t num_strings = m_strings.size();
-
- if (num_strings == 0) {
- common_prefix.clear();
- } else {
- common_prefix = m_strings.front();
-
- for (size_t idx = 1; idx < num_strings; ++idx) {
- std::string &curr_string = m_strings[idx];
- size_t new_size = curr_string.size();
-
- // First trim common_prefix if it is longer than the current element:
- if (common_prefix.size() > new_size)
- common_prefix.erase(new_size);
-
- // Then trim it at the first disparity:
- for (size_t i = 0; i < common_prefix.size(); i++) {
- if (curr_string[i] != common_prefix[i]) {
- common_prefix.erase(i);
- break;
- }
- }
-
- // If we've emptied the common prefix, we're done.
- if (common_prefix.empty())
+ common_prefix.clear();
+ if (m_strings.empty())
+ return;
+
+ auto args = llvm::makeArrayRef(m_strings);
+ llvm::StringRef prefix = args.front();
+ for (auto arg : args.drop_front()) {
+ size_t count = 0;
+ for (count = 0; count < std::min(prefix.size(), arg.size()); ++count) {
+ if (prefix[count] != arg[count])
break;
}
+ prefix = prefix.take_front(count);
}
+ common_prefix = prefix;
}
void StringList::InsertStringAtIndex(size_t idx, const char *str) {
Modified: lldb/trunk/source/Interpreter/Args.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=283157&r1=283156&r2=283157&view=diff
==============================================================================
--- lldb/trunk/source/Interpreter/Args.cpp (original)
+++ lldb/trunk/source/Interpreter/Args.cpp Mon Oct 3 17:51:09 2016
@@ -25,95 +25,12 @@
#include "lldb/Target/StackFrame.h"
#include "lldb/Target/Target.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringSwitch.h"
using namespace lldb;
using namespace lldb_private;
-//----------------------------------------------------------------------
-// Args constructor
-//----------------------------------------------------------------------
-Args::Args(llvm::StringRef command) : m_args(), m_argv(), m_args_quote_char() {
- SetCommandString(command);
-}
-
-//----------------------------------------------------------------------
-// We have to be very careful on the copy constructor of this class
-// to make sure we copy all of the string values, but we can't copy the
-// rhs.m_argv into m_argv since it will point to the "const char *" c
-// strings in rhs.m_args. We need to copy the string list and update our
-// own m_argv appropriately.
-//----------------------------------------------------------------------
-Args::Args(const Args &rhs)
- : m_args(rhs.m_args), m_argv(), m_args_quote_char(rhs.m_args_quote_char) {
- UpdateArgvFromArgs();
-}
-
-//----------------------------------------------------------------------
-// We have to be very careful on the copy constructor of this class
-// to make sure we copy all of the string values, but we can't copy the
-// rhs.m_argv into m_argv since it will point to the "const char *" c
-// strings in rhs.m_args. We need to copy the string list and update our
-// own m_argv appropriately.
-//----------------------------------------------------------------------
-const Args &Args::operator=(const Args &rhs) {
- // Make sure we aren't assigning to self
- if (this != &rhs) {
- m_args = rhs.m_args;
- m_args_quote_char = rhs.m_args_quote_char;
- UpdateArgvFromArgs();
- }
- return *this;
-}
-
-//----------------------------------------------------------------------
-// Destructor
-//----------------------------------------------------------------------
-Args::~Args() {}
-
-void Args::Dump(Stream &s, const char *label_name) const {
- if (!label_name)
- return;
-
- const size_t argc = m_argv.size();
- for (size_t i = 0; i < argc; ++i) {
- s.Indent();
- const char *arg_cstr = m_argv[i];
- if (arg_cstr)
- s.Printf("%s[%zi]=\"%s\"\n", label_name, i, arg_cstr);
- else
- s.Printf("%s[%zi]=NULL\n", label_name, i);
- }
- s.EOL();
-}
-
-bool Args::GetCommandString(std::string &command) const {
- command.clear();
- const size_t argc = GetArgumentCount();
- for (size_t i = 0; i < argc; ++i) {
- if (i > 0)
- command += ' ';
- command += m_argv[i];
- }
- return argc > 0;
-}
-
-bool Args::GetQuotedCommandString(std::string &command) const {
- command.clear();
- const size_t argc = GetArgumentCount();
- for (size_t i = 0; i < argc; ++i) {
- if (i > 0)
- command.append(1, ' ');
- char quote_char = GetArgumentQuoteCharAtIndex(i);
- if (quote_char) {
- command.append(1, quote_char);
- command.append(m_argv[i]);
- command.append(1, quote_char);
- } else
- command.append(m_argv[i]);
- }
- return argc > 0;
-}
// A helper function for argument parsing.
// Parses the initial part of the first argument using normal double quote
@@ -159,22 +76,27 @@ static llvm::StringRef ParseDoubleQuotes
return quoted;
}
-// A helper function for SetCommandString.
-// Parses a single argument from the command string, processing quotes and
-// backslashes in a
-// shell-like manner. The parsed argument is appended to the m_args array. The
-// function returns
-// the unparsed portion of the string, starting at the first unqouted, unescaped
-// whitespace
-// character.
-llvm::StringRef Args::ParseSingleArgument(llvm::StringRef command) {
- // Argument can be split into multiple discontiguous pieces,
- // for example:
- // "Hello ""World"
- // this would result in a single argument "Hello World" (without/
- // the quotes) since the quotes would be removed and there is
- // not space between the strings.
+static size_t ArgvToArgc(const char **argv) {
+ if (!argv)
+ return 0;
+ size_t count = 0;
+ while (*argv++)
+ ++count;
+ return count;
+}
+// A helper function for SetCommandString. Parses a single argument from the
+// command string, processing quotes and backslashes in a shell-like manner.
+// The function returns a tuple consisting of the parsed argument, the quote
+// char used, and the unparsed portion of the string starting at the first
+// unqouted, unescaped whitespace character.
+static std::tuple<std::string, char, llvm::StringRef>
+ParseSingleArgument(llvm::StringRef command) {
+ // Argument can be split into multiple discontiguous pieces, for example:
+ // "Hello ""World"
+ // this would result in a single argument "Hello World" (without the quotes)
+ // since the quotes would be removed and there is not space between the
+ // strings.
std::string arg;
// Since we can have multiple quotes that form a single command
@@ -246,78 +168,133 @@ llvm::StringRef Args::ParseSingleArgumen
}
} while (!arg_complete);
- m_args.push_back(arg);
- m_args_quote_char.push_back(first_quote_char);
- return command;
+ return std::make_tuple(arg, first_quote_char, command);
}
-void Args::SetCommandString(llvm::StringRef command) {
- m_args.clear();
+Args::ArgEntry::ArgEntry(llvm::StringRef str, char quote) : quote(quote) {
+ size_t size = str.size();
+ ptr.reset(new char[size + 1]);
+
+ ::memcpy(data(), str.data() ? str.data() : "", size);
+ ptr[size] = 0;
+ ref = llvm::StringRef(c_str(), size);
+}
+
+//----------------------------------------------------------------------
+// Args constructor
+//----------------------------------------------------------------------
+Args::Args(llvm::StringRef command) { SetCommandString(command); }
+
+Args::Args(const Args &rhs) { *this = rhs; }
+
+Args &Args::operator=(const Args &rhs) {
+ Clear();
+
m_argv.clear();
- m_args_quote_char.clear();
+ m_entries.clear();
+ for (auto &entry : rhs.m_entries) {
+ m_entries.emplace_back(entry.ref, entry.quote);
+ m_argv.push_back(m_entries.back().data());
+ }
+ m_argv.push_back(nullptr);
+ return *this;
+}
- static const char *k_space_separators = " \t";
- command = command.ltrim(k_space_separators);
- while (!command.empty()) {
- command = ParseSingleArgument(command);
- command = command.ltrim(k_space_separators);
+//----------------------------------------------------------------------
+// Destructor
+//----------------------------------------------------------------------
+Args::~Args() {}
+
+void Args::Dump(Stream &s, const char *label_name) const {
+ if (!label_name)
+ return;
+
+ int i = 0;
+ for (auto &entry : m_entries) {
+ s.Indent();
+ s.Printf("%s[%zi]=\"%*s\"\n", label_name, i++, entry.ref.size(),
+ entry.ref.data());
+ }
+ s.Printf("%s[%zi]=NULL\n", label_name, i);
+ s.EOL();
+}
+
+bool Args::GetCommandString(std::string &command) const {
+ command.clear();
+
+ for (size_t i = 0; i < m_entries.size(); ++i) {
+ if (i > 0)
+ command += ' ';
+ command += m_entries[i].ref;
}
- UpdateArgvFromArgs();
+ return !m_entries.empty();
}
-void Args::UpdateArgsAfterOptionParsing() {
- // Now m_argv might be out of date with m_args, so we need to fix that
- arg_cstr_collection::const_iterator argv_pos, argv_end = m_argv.end();
- arg_sstr_collection::iterator args_pos;
- arg_quote_char_collection::iterator quotes_pos;
-
- for (argv_pos = m_argv.begin(), args_pos = m_args.begin(),
- quotes_pos = m_args_quote_char.begin();
- argv_pos != argv_end && args_pos != m_args.end(); ++argv_pos) {
- const char *argv_cstr = *argv_pos;
- if (argv_cstr == nullptr)
- break;
+bool Args::GetQuotedCommandString(std::string &command) const {
+ command.clear();
- while (args_pos != m_args.end()) {
- const char *args_cstr = args_pos->c_str();
- if (args_cstr == argv_cstr) {
- // We found the argument that matches the C string in the
- // vector, so we can now look for the next one
- ++args_pos;
- ++quotes_pos;
- break;
- } else {
- quotes_pos = m_args_quote_char.erase(quotes_pos);
- args_pos = m_args.erase(args_pos);
- }
+ for (size_t i = 0; i < m_entries.size(); ++i) {
+ if (i > 0)
+ command += ' ';
+
+ if (m_entries[i].quote) {
+ command += m_entries[i].quote;
+ command += m_entries[i].ref;
+ command += m_entries[i].quote;
+ } else {
+ command += m_entries[i].ref;
}
}
- if (args_pos != m_args.end())
- m_args.erase(args_pos, m_args.end());
-
- if (quotes_pos != m_args_quote_char.end())
- m_args_quote_char.erase(quotes_pos, m_args_quote_char.end());
+ return !m_entries.empty();
}
-void Args::UpdateArgvFromArgs() {
+void Args::SetCommandString(llvm::StringRef command) {
+ Clear();
m_argv.clear();
- arg_sstr_collection::const_iterator pos, end = m_args.end();
- for (pos = m_args.begin(); pos != end; ++pos)
- m_argv.push_back(pos->c_str());
+
+ static const char *k_space_separators = " \t";
+ command = command.ltrim(k_space_separators);
+ std::string arg;
+ char quote;
+ while (!command.empty()) {
+ std::tie(arg, quote, command) = ParseSingleArgument(command);
+ m_entries.emplace_back(arg, quote);
+ m_argv.push_back(m_entries.back().data());
+ command = command.ltrim(k_space_separators);
+ }
m_argv.push_back(nullptr);
- // Make sure we have enough arg quote chars in the array
- if (m_args_quote_char.size() < m_args.size())
- m_args_quote_char.resize(m_argv.size());
}
-size_t Args::GetArgumentCount() const {
- if (m_argv.empty())
- return 0;
- return m_argv.size() - 1;
+void Args::UpdateArgsAfterOptionParsing() {
+ assert(!m_argv.empty());
+ assert(m_argv.back() == nullptr);
+
+ // Now m_argv might be out of date with m_entries, so we need to fix that.
+ // This happens because getopt_long_only may permute the order of the
+ // arguments in argv, so we need to re-order the quotes and the refs array
+ // to match.
+ for (int i = 0; i < m_argv.size() - 1; ++i) {
+ const char *argv = m_argv[i];
+ auto pos =
+ std::find_if(m_entries.begin() + i, m_entries.end(),
+ [argv](const ArgEntry &D) { return D.c_str() == argv; });
+ assert(pos != m_entries.end());
+ size_t distance = std::distance(m_entries.begin(), pos);
+ if (i == distance)
+ continue;
+
+ assert(distance > i);
+
+ std::swap(m_entries[i], m_entries[distance]);
+ assert(m_entries[i].ref.data() == m_argv[i]);
+ }
+ m_entries.resize(m_argv.size() - 1);
}
+size_t Args::GetArgumentCount() const { return m_entries.size(); }
+
const char *Args::GetArgumentAtIndex(size_t idx) const {
if (idx < m_argv.size())
return m_argv[idx];
@@ -325,52 +302,62 @@ const char *Args::GetArgumentAtIndex(siz
}
char Args::GetArgumentQuoteCharAtIndex(size_t idx) const {
- if (idx < m_args_quote_char.size())
- return m_args_quote_char[idx];
+ if (idx < m_entries.size())
+ return m_entries[idx].quote;
return '\0';
}
char **Args::GetArgumentVector() {
- if (!m_argv.empty())
- return const_cast<char **>(&m_argv[0]);
- return nullptr;
+ assert(!m_argv.empty());
+ // TODO: functions like execve and posix_spawnp exhibit undefined behavior
+ // when argv or envp is null. So the code below is actually wrong. However,
+ // other code in LLDB depends on it being null. The code has been acting this
+ // way for some time, so it makes sense to leave it this way until someone
+ // has the time to come along and fix it.
+ return (m_argv.size() > 1) ? m_argv.data() : nullptr;
}
const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
- return const_cast<const char **>(&m_argv[0]);
- return nullptr;
+ assert(!m_argv.empty());
+ return (m_argv.size() > 1) ? const_cast<const char **>(m_argv.data())
+ : nullptr;
}
void Args::Shift() {
// Don't pop the last NULL terminator from the argv array
- if (m_argv.size() > 1) {
- m_argv.erase(m_argv.begin());
- m_args.pop_front();
- if (!m_args_quote_char.empty())
- m_args_quote_char.erase(m_args_quote_char.begin());
- }
+ if (m_entries.empty())
+ return;
+ m_argv.erase(m_argv.begin());
+ m_entries.erase(m_entries.begin());
}
llvm::StringRef Args::Unshift(llvm::StringRef arg_str, char quote_char) {
- m_args.push_front(arg_str);
- m_argv.insert(m_argv.begin(), m_args.front().c_str());
- m_args_quote_char.insert(m_args_quote_char.begin(), quote_char);
- return llvm::StringRef::withNullAsEmpty(GetArgumentAtIndex(0));
+ return InsertArgumentAtIndex(0, arg_str, quote_char);
}
void Args::AppendArguments(const Args &rhs) {
- const size_t rhs_argc = rhs.GetArgumentCount();
- for (size_t i = 0; i < rhs_argc; ++i)
- AppendArgument(llvm::StringRef(rhs.GetArgumentAtIndex(i)),
- rhs.GetArgumentQuoteCharAtIndex(i));
+ assert(m_argv.size() == m_entries.size() + 1);
+ assert(m_argv.back() == nullptr);
+ m_argv.pop_back();
+ for (auto &entry : rhs.m_entries) {
+ m_entries.emplace_back(entry.ref, entry.quote);
+ m_argv.push_back(m_entries.back().data());
+ }
+ m_argv.push_back(nullptr);
}
void Args::AppendArguments(const char **argv) {
- if (argv) {
- for (uint32_t i = 0; argv[i]; ++i)
- AppendArgument(llvm::StringRef::withNullAsEmpty(argv[i]));
+ size_t argc = ArgvToArgc(argv);
+
+ assert(m_argv.size() == m_entries.size() + 1);
+ assert(m_argv.back() == nullptr);
+ m_argv.pop_back();
+ for (int i = 0; i < argc; ++i) {
+ m_entries.emplace_back(argv[i], '\0');
+ m_argv.push_back(m_entries.back().data());
}
+
+ m_argv.push_back(nullptr);
}
llvm::StringRef Args::AppendArgument(llvm::StringRef arg_str, char quote_char) {
@@ -379,103 +366,65 @@ llvm::StringRef Args::AppendArgument(llv
llvm::StringRef Args::InsertArgumentAtIndex(size_t idx, llvm::StringRef arg_str,
char quote_char) {
- // Since we are using a std::list to hold onto the copied C string and
- // we don't have direct access to the elements, we have to iterate to
- // find the value.
- arg_sstr_collection::iterator pos, end = m_args.end();
- size_t i = idx;
- for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
- --i;
-
- pos = m_args.insert(pos, arg_str);
-
- if (idx >= m_args_quote_char.size()) {
- m_args_quote_char.resize(idx + 1);
- m_args_quote_char[idx] = quote_char;
- } else
- m_args_quote_char.insert(m_args_quote_char.begin() + idx, quote_char);
+ assert(m_argv.size() == m_entries.size() + 1);
+ assert(m_argv.back() == nullptr);
- UpdateArgvFromArgs();
- return GetArgumentAtIndex(idx);
+ if (idx > m_entries.size())
+ return llvm::StringRef();
+ m_entries.emplace(m_entries.begin() + idx, arg_str, quote_char);
+ m_argv.insert(m_argv.begin() + idx, m_entries[idx].data());
+ return m_entries[idx].ref;
}
llvm::StringRef Args::ReplaceArgumentAtIndex(size_t idx,
llvm::StringRef arg_str,
char quote_char) {
- // Since we are using a std::list to hold onto the copied C string and
- // we don't have direct access to the elements, we have to iterate to
- // find the value.
- arg_sstr_collection::iterator pos, end = m_args.end();
- size_t i = idx;
- for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
- --i;
-
- if (pos != end) {
- pos->assign(arg_str);
- assert(idx < m_argv.size() - 1);
- m_argv[idx] = pos->c_str();
- if (idx >= m_args_quote_char.size())
- m_args_quote_char.resize(idx + 1);
- m_args_quote_char[idx] = quote_char;
- return GetArgumentAtIndex(idx);
+ assert(m_argv.size() == m_entries.size() + 1);
+ assert(m_argv.back() == nullptr);
+
+ if (idx >= m_entries.size())
+ return llvm::StringRef();
+
+ if (arg_str.size() > m_entries[idx].ref.size()) {
+ m_entries[idx] = ArgEntry(arg_str, quote_char);
+ m_argv[idx] = m_entries[idx].data();
+ } else {
+ const char *src_data = arg_str.data() ? arg_str.data() : "";
+ ::memcpy(m_entries[idx].data(), src_data, arg_str.size());
+ m_entries[idx].ptr[arg_str.size()] = 0;
+ m_entries[idx].ref = m_entries[idx].ref.take_front(arg_str.size());
}
- return llvm::StringRef();
+
+ return m_entries[idx].ref;
}
void Args::DeleteArgumentAtIndex(size_t idx) {
- // Since we are using a std::list to hold onto the copied C string and
- // we don't have direct access to the elements, we have to iterate to
- // find the value.
- arg_sstr_collection::iterator pos, end = m_args.end();
- size_t i = idx;
- for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
- --i;
-
- if (pos != end) {
- m_args.erase(pos);
- assert(idx < m_argv.size() - 1);
- m_argv.erase(m_argv.begin() + idx);
- if (idx < m_args_quote_char.size())
- m_args_quote_char.erase(m_args_quote_char.begin() + idx);
- }
+ if (idx >= m_entries.size())
+ return;
+
+ m_argv.erase(m_argv.begin() + idx);
+ m_entries.erase(m_entries.begin() + idx);
}
void Args::SetArguments(size_t argc, const char **argv) {
- // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is
- // no need to clear it here.
- m_args.clear();
- m_args_quote_char.clear();
-
- // First copy each string
- for (size_t i = 0; i < argc; ++i) {
- m_args.push_back(argv[i]);
- if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`'))
- m_args_quote_char.push_back(argv[i][0]);
- else
- m_args_quote_char.push_back('\0');
- }
+ Clear();
- UpdateArgvFromArgs();
-}
+ auto args = llvm::makeArrayRef(argv, argc);
+ m_entries.resize(argc);
+ m_argv.resize(argc + 1);
+ for (int i = 0; i < args.size(); ++i) {
+ char quote =
+ ((args[i][0] == '\'') || (args[i][0] == '"') || (args[i][0] == '`'))
+ ? args[i][0]
+ : '\0';
-void Args::SetArguments(const char **argv) {
- // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is
- // no need to clear it here.
- m_args.clear();
- m_args_quote_char.clear();
-
- if (argv) {
- // First copy each string
- for (size_t i = 0; argv[i]; ++i) {
- m_args.push_back(argv[i]);
- if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`'))
- m_args_quote_char.push_back(argv[i][0]);
- else
- m_args_quote_char.push_back('\0');
- }
+ m_entries[i] = ArgEntry(args[i], quote);
+ m_argv[i] = m_entries[i].data();
}
+}
- UpdateArgvFromArgs();
+void Args::SetArguments(const char **argv) {
+ SetArguments(ArgvToArgc(argv), argv);
}
Error Args::ParseOptions(Options &options, ExecutionContext *execution_context,
@@ -598,9 +547,9 @@ Error Args::ParseOptions(Options &option
}
void Args::Clear() {
- m_args.clear();
+ m_entries.clear();
m_argv.clear();
- m_args_quote_char.clear();
+ m_argv.push_back(nullptr);
}
lldb::addr_t Args::StringToAddress(const ExecutionContext *exe_ctx,
@@ -947,36 +896,6 @@ uint32_t Args::StringToGenericRegister(l
return result;
}
-void Args::LongestCommonPrefix(std::string &common_prefix) {
- arg_sstr_collection::iterator pos, end = m_args.end();
- pos = m_args.begin();
- if (pos == end)
- common_prefix.clear();
- else
- common_prefix = (*pos);
-
- for (++pos; pos != end; ++pos) {
- size_t new_size = (*pos).size();
-
- // First trim common_prefix if it is longer than the current element:
- if (common_prefix.size() > new_size)
- common_prefix.erase(new_size);
-
- // Then trim it at the first disparity:
-
- for (size_t i = 0; i < common_prefix.size(); i++) {
- if ((*pos)[i] != common_prefix[i]) {
- common_prefix.erase(i);
- break;
- }
- }
-
- // If we've emptied the common prefix, we're done.
- if (common_prefix.empty())
- break;
- }
-}
-
void Args::AddOrReplaceEnvironmentVariable(llvm::StringRef env_var_name,
llvm::StringRef new_value) {
if (env_var_name.empty())
@@ -1415,10 +1334,10 @@ void Args::ParseArgsForCompletion(Option
// it is AT the cursor position.
// Note, a single quoted dash is not the same as a single dash...
+ const ArgEntry &cursor = m_entries[cursor_index];
if ((static_cast<int32_t>(dash_dash_pos) == -1 ||
cursor_index < dash_dash_pos) &&
- m_args_quote_char[cursor_index] == '\0' &&
- strcmp(GetArgumentAtIndex(cursor_index), "-") == 0) {
+ cursor.quote == '\0' && cursor.ref == "-") {
option_element_vector.push_back(
OptionArgElement(OptionArgElement::eBareDash, cursor_index,
OptionArgElement::eBareDash));
Modified: lldb/trunk/unittests/Interpreter/TestArgs.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/TestArgs.cpp?rev=283157&r1=283156&r2=283157&view=diff
==============================================================================
--- lldb/trunk/unittests/Interpreter/TestArgs.cpp (original)
+++ lldb/trunk/unittests/Interpreter/TestArgs.cpp Mon Oct 3 17:51:09 2016
@@ -66,6 +66,109 @@ TEST(ArgsTest, TestAppendArg) {
EXPECT_STREQ(args.GetArgumentAtIndex(1), "second_arg");
}
+TEST(ArgsTest, TestInsertArg) {
+ Args args;
+ args.AppendArgument("1");
+ args.AppendArgument("2");
+ args.AppendArgument("3");
+ args.InsertArgumentAtIndex(1, "1.5");
+ args.InsertArgumentAtIndex(4, "3.5");
+
+ ASSERT_EQ(5u, args.GetArgumentCount());
+ EXPECT_STREQ("1", args.GetArgumentAtIndex(0));
+ EXPECT_STREQ("1.5", args.GetArgumentAtIndex(1));
+ EXPECT_STREQ("2", args.GetArgumentAtIndex(2));
+ EXPECT_STREQ("3", args.GetArgumentAtIndex(3));
+ EXPECT_STREQ("3.5", args.GetArgumentAtIndex(4));
+}
+
+TEST(ArgsTest, TestArgv) {
+ Args args;
+ EXPECT_EQ(nullptr, args.GetArgumentVector());
+
+ args.AppendArgument("1");
+ EXPECT_NE(nullptr, args.GetArgumentVector()[0]);
+ EXPECT_EQ(nullptr, args.GetArgumentVector()[1]);
+
+ args.AppendArgument("2");
+ EXPECT_NE(nullptr, args.GetArgumentVector()[0]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[1]);
+ EXPECT_EQ(nullptr, args.GetArgumentVector()[2]);
+
+ args.AppendArgument("3");
+ EXPECT_NE(nullptr, args.GetArgumentVector()[0]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[1]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[2]);
+ EXPECT_EQ(nullptr, args.GetArgumentVector()[3]);
+
+ args.InsertArgumentAtIndex(1, "1.5");
+ EXPECT_NE(nullptr, args.GetArgumentVector()[0]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[1]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[2]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[3]);
+ EXPECT_EQ(nullptr, args.GetArgumentVector()[4]);
+
+ args.InsertArgumentAtIndex(4, "3.5");
+ EXPECT_NE(nullptr, args.GetArgumentVector()[0]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[1]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[2]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[3]);
+ EXPECT_NE(nullptr, args.GetArgumentVector()[4]);
+ EXPECT_EQ(nullptr, args.GetArgumentVector()[5]);
+}
+
+TEST(ArgsTest, GetQuotedCommandString) {
+ Args args;
+ const char *str = "process launch -o stdout.txt -- \"a b c\"";
+ args.SetCommandString(str);
+
+ std::string stdstr;
+ ASSERT_TRUE(args.GetQuotedCommandString(stdstr));
+ EXPECT_EQ(str, stdstr);
+}
+
+TEST(ArgsTest, BareSingleQuote) {
+ Args args;
+ args.SetCommandString("a\\'b");
+ EXPECT_EQ(1u, args.GetArgumentCount());
+
+ EXPECT_STREQ("a'b", args.GetArgumentAtIndex(0));
+}
+
+TEST(ArgsTest, DoubleQuotedItem) {
+ Args args;
+ args.SetCommandString("\"a b c\"");
+ EXPECT_EQ(1u, args.GetArgumentCount());
+
+ EXPECT_STREQ("a b c", args.GetArgumentAtIndex(0));
+}
+
+TEST(ArgsTest, AppendArguments) {
+ Args args;
+ const char *argv[] = {"1", "2", nullptr};
+ const char *argv2[] = {"3", "4", nullptr};
+
+ args.AppendArguments(argv);
+ ASSERT_EQ(2u, args.GetArgumentCount());
+ EXPECT_STREQ("1", args.GetArgumentVector()[0]);
+ EXPECT_STREQ("2", args.GetArgumentVector()[1]);
+ EXPECT_EQ(nullptr, args.GetArgumentVector()[2]);
+ EXPECT_STREQ("1", args.GetArgumentAtIndex(0));
+ EXPECT_STREQ("2", args.GetArgumentAtIndex(1));
+
+ args.AppendArguments(argv2);
+ ASSERT_EQ(4u, args.GetArgumentCount());
+ EXPECT_STREQ("1", args.GetArgumentVector()[0]);
+ EXPECT_STREQ("2", args.GetArgumentVector()[1]);
+ EXPECT_STREQ("3", args.GetArgumentVector()[2]);
+ EXPECT_STREQ("4", args.GetArgumentVector()[3]);
+ EXPECT_EQ(nullptr, args.GetArgumentVector()[4]);
+ EXPECT_STREQ("1", args.GetArgumentAtIndex(0));
+ EXPECT_STREQ("2", args.GetArgumentAtIndex(1));
+ EXPECT_STREQ("3", args.GetArgumentAtIndex(2));
+ EXPECT_STREQ("4", args.GetArgumentAtIndex(3));
+}
+
TEST(ArgsTest, StringToBoolean) {
bool success = false;
EXPECT_TRUE(Args::StringToBoolean(llvm::StringRef("true"), false, nullptr));
More information about the lldb-commits
mailing list