Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner added inline comments.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
zturner wrote:
> clayborg wrote:
> > zturner wrote:
> > > clayborg wrote:
> > > > Remove this comment. The whole point off this function is to get an
> > > > argument vector that is NULL terminated. Feel free to rename the
> > > > function as desired to indicate this, but that is the whole point of
> > > > this function. Feel free to document it if needed as well.
> > > There are two ways we use these argument vectors. One is in a call to a
> > > function which takes an argc and an argv. In that case the null
> > > terminator is unnecessary because you're specifying the argc explicitly.
> > > Another is in a call to a function which you do not specify an argc, and
> > > in tht case it requires it to be null terminated.
> > >
> > > The point of this function isn't "return me something that's null
> > > terminated", it's "return me something I can use like an argv". That
> > > doesn't require null termination. And in fact, if you look at the
> > > callsites I fixed up, not a single one depends on the null termination.
> > > Everyone just needs to know the size. And for that you can call
> > > `result.size()`. If you add the null terminator, then you have to write
> > > `result.size()-1`, which is unnecessarily confusing. I believe it
> > > provides the most logical behavior as it is currently written.
> > Remove the comment. Even in "main(int argc, const char **argv)" the "argv"
> > is null terminated. So all unix variants expect the null termination. The
> > correct solution is to add a boolean argument:
> >
> > ```
> > std::vector Args::GetArgumentVector(bool null_terminate =
> > true) const {
> > ```
> >
> > Then it is clear.
> Even in main though, the null is not counted in the argc, right? So as
> written it will conform to the semantics of main. `argc` is the number of
> non-null pointers, but the null is still there anyway. If you add the
> boolean argument and pass true, it will give you back something whose `argc`
> (i.e. `result.size()`) is one too high.
BTW, `llvm::SmallString` uses this same trick to return a null terminated
string without modifying the size. Look at the implementation of
`SmallString::c_str()` in `SmallString.h`
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner added inline comments.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
clayborg wrote:
> zturner wrote:
> > clayborg wrote:
> > > Remove this comment. The whole point off this function is to get an
> > > argument vector that is NULL terminated. Feel free to rename the function
> > > as desired to indicate this, but that is the whole point of this
> > > function. Feel free to document it if needed as well.
> > There are two ways we use these argument vectors. One is in a call to a
> > function which takes an argc and an argv. In that case the null terminator
> > is unnecessary because you're specifying the argc explicitly. Another is
> > in a call to a function which you do not specify an argc, and in tht case
> > it requires it to be null terminated.
> >
> > The point of this function isn't "return me something that's null
> > terminated", it's "return me something I can use like an argv". That
> > doesn't require null termination. And in fact, if you look at the
> > callsites I fixed up, not a single one depends on the null termination.
> > Everyone just needs to know the size. And for that you can call
> > `result.size()`. If you add the null terminator, then you have to write
> > `result.size()-1`, which is unnecessarily confusing. I believe it provides
> > the most logical behavior as it is currently written.
> Remove the comment. Even in "main(int argc, const char **argv)" the "argv" is
> null terminated. So all unix variants expect the null termination. The
> correct solution is to add a boolean argument:
>
> ```
> std::vector Args::GetArgumentVector(bool null_terminate = true)
> const {
> ```
>
> Then it is clear.
Even in main though, the null is not counted in the argc, right? So as written
it will conform to the semantics of main. `argc` is the number of non-null
pointers, but the null is still there anyway. If you add the boolean argument
and pass true, it will give you back something whose `argc` (i.e.
`result.size()`) is one too high.
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
clayborg added a comment.
Ok, so just add the boolean arg to Args::GetArgumentVector and we are good to
go.
Comment at: source/Interpreter/Args.cpp:270-272
@@ -321,5 +269,5 @@
const char *Args::GetArgumentAtIndex(size_t idx) const {
- if (idx < m_argv.size())
-return m_argv[idx];
+ if (idx < m_args.size())
+return m_args[idx].c_str();
return nullptr;
}
Ok, sounds good, for a later CL then.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
zturner wrote:
> clayborg wrote:
> > Remove this comment. The whole point off this function is to get an
> > argument vector that is NULL terminated. Feel free to rename the function
> > as desired to indicate this, but that is the whole point of this function.
> > Feel free to document it if needed as well.
> There are two ways we use these argument vectors. One is in a call to a
> function which takes an argc and an argv. In that case the null terminator
> is unnecessary because you're specifying the argc explicitly. Another is in
> a call to a function which you do not specify an argc, and in tht case it
> requires it to be null terminated.
>
> The point of this function isn't "return me something that's null
> terminated", it's "return me something I can use like an argv". That doesn't
> require null termination. And in fact, if you look at the callsites I fixed
> up, not a single one depends on the null termination. Everyone just needs to
> know the size. And for that you can call `result.size()`. If you add the
> null terminator, then you have to write `result.size()-1`, which is
> unnecessarily confusing. I believe it provides the most logical behavior as
> it is currently written.
Remove the comment. Even in "main(int argc, const char **argv)" the "argv" is
null terminated. So all unix variants expect the null termination. The correct
solution is to add a boolean argument:
```
std::vector Args::GetArgumentVector(bool null_terminate = true)
const {
```
Then it is clear.
Comment at: source/Interpreter/CommandObject.cpp:119
@@ -118,3 +118,2 @@
// so we need to push a dummy value into position zero.
-args.Unshift(llvm::StringRef("dummy_string"));
const bool require_validation = true;
zturner wrote:
> clayborg wrote:
> > Why was this removed?
> Because it's a little weird to do it at this high of a level. This requires
> internal knowledge of the workings of `Args::ParseOptions()`, which itself
> calls `OptionParser::Parse()`, It's strange for a function this high up to
> require such low-level knowledge of the workings of a function. So instead,
> I moved this behavior into `Args::ParseOptions`. If you check that function,
> you will see that it injects this argument into the beginning.
>
> This is nice because now it never modifies the internal state of the `Args`
> itself, but only the copy that gets passed to `getopt_long_only`.
Sounds good. I hate the fact that we have to do this. We might be able to play
with the getopt_long globals to work around this and not have to add it, but as
long as you took it into account I am fine.
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner added inline comments.
Comment at: source/Interpreter/Args.cpp:269-272
@@ -320,6 +268,6 @@
const char *Args::GetArgumentAtIndex(size_t idx) const {
- if (idx < m_argv.size())
-return m_argv[idx];
+ if (idx < m_args.size())
+return m_args[idx].c_str();
return nullptr;
}
clayborg wrote:
> Convert this over to return StringRef?
Yes, that is the next item on my list. But this function is used EVERYWHERE,
so that will be a very large CL and should be done orthogonally.
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
clayborg wrote:
> Remove this comment. The whole point off this function is to get an argument
> vector that is NULL terminated. Feel free to rename the function as desired
> to indicate this, but that is the whole point of this function. Feel free to
> document it if needed as well.
There are two ways we use these argument vectors. One is in a call to a
function which takes an argc and an argv. In that case the null terminator is
unnecessary because you're specifying the argc explicitly. Another is in a
call to a function which you do not specify an argc, and in tht case it
requires it to be null terminated.
The point of this function isn't "return me something that's null terminated",
it's "return me something I can use like an argv". That doesn't require null
termination. And in fact, if you look at the callsites I fixed up, not a
single one depends on the null termination. Everyone just needs to know the
size. And for that you can call `result.size()`. If you add the null
terminator, then you have to write `result.size()-1`, which is unnecessarily
confusing. I believe it provides the most logical behavior as it is currently
written.
Comment at: source/Interpreter/Args.cpp:327
@@ -381,11 +326,3 @@
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);
+ m_args.insert(m_args.begin() + idx, arg_str);
clayborg wrote:
> Is there a conversion operator that converts StringRef to std::string
> somewhere? How is this working? Iterators being detected?
Yes, StringRef has an `operator std::string()`.
Comment at: source/Interpreter/Args.cpp:373-376
@@ -460,21 +372,6 @@
-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');
-}
- }
-
- UpdateArgvFromArgs();
+void Args::SetArguments(const Args ) {
+ m_args = other.m_args;
+ m_args_quote_char = other.m_args_quote_char;
}
clayborg wrote:
> Should we change this to an assignment operator? If so, rename "other" to
> "rhs".
I thought about it, but it seems like a minor stylistic issue. I think it
could go either way. I'm fine adding it or not.
Comment at: source/Interpreter/CommandObject.cpp:119
@@ -118,3 +118,2 @@
// so we need to push a dummy value into position zero.
-args.Unshift(llvm::StringRef("dummy_string"));
const bool require_validation = true;
clayborg wrote:
> Why was this removed?
Because it's a little weird to do it at this high of a level. This requires
internal knowledge of the workings of `Args::ParseOptions()`, which itself
calls `OptionParser::Parse()`, It's strange for a function this high up to
require such low-level knowledge of the workings of a function. So instead, I
moved this behavior into `Args::ParseOptions`. If you check that function, you
will see that it injects this argument into the beginning.
This is nice because now it never modifies the internal state of the `Args`
itself, but only the copy that gets passed to `getopt_long_only`.
Comment at: source/Target/ProcessInfo.cpp:94
@@ -93,3 +93,3 @@
bool first_arg_is_executable) {
- m_arguments.SetArguments(argv);
+
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
See inlined comments.
Comment at: source/Interpreter/Args.cpp:264-265
@@ -315,3 +263,4 @@
size_t Args::GetArgumentCount() const {
- if (m_argv.empty())
+ if (m_args.empty())
return 0;
+ return m_args.size();
Remove this if, it is no longer needed. Just return m_args.size()
Comment at: source/Interpreter/Args.cpp:269-272
@@ -320,6 +268,6 @@
const char *Args::GetArgumentAtIndex(size_t idx) const {
- if (idx < m_argv.size())
-return m_argv[idx];
+ if (idx < m_args.size())
+return m_args[idx].c_str();
return nullptr;
}
Convert this over to return StringRef?
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
-return const_cast(_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
Remove this comment. The whole point off this function is to get an argument
vector that is NULL terminated. Feel free to rename the function as desired to
indicate this, but that is the whole point of this function. Feel free to
document it if needed as well.
Comment at: source/Interpreter/Args.cpp:327
@@ -381,11 +326,3 @@
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);
+ m_args.insert(m_args.begin() + idx, arg_str);
Is there a conversion operator that converts StringRef to std::string
somewhere? How is this working? Iterators being detected?
Comment at: source/Interpreter/Args.cpp:373-376
@@ -460,21 +372,6 @@
-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');
-}
- }
-
- UpdateArgvFromArgs();
+void Args::SetArguments(const Args ) {
+ m_args = other.m_args;
+ m_args_quote_char = other.m_args_quote_char;
}
Should we change this to an assignment operator? If so, rename "other" to "rhs".
Comment at: source/Interpreter/CommandInterpreter.cpp:2049-2050
@@ -2048,5 +2048,4 @@
cmd_args.Clear();
-cmd_args.SetArguments(new_args.GetArgumentCount(),
- new_args.GetConstArgumentVector());
+cmd_args.SetArguments(new_args);
} else {
If we do the operator = above, then:
```
cmd_args = new_args;
```
Comment at: source/Interpreter/CommandInterpreter.cpp:2059-2060
@@ -2059,5 +2058,4 @@
if (wants_raw_input) {
cmd_args.Clear();
- cmd_args.SetArguments(new_args.GetArgumentCount(),
-new_args.GetConstArgumentVector());
+ cmd_args.SetArguments(new_args);
}
If we do the operator = above, then:
```
cmd_args = new_args;
```
Comment at: source/Interpreter/CommandObject.cpp:119
@@ -118,3 +118,2 @@
// so we need to push a dummy value into position zero.
-args.Unshift(llvm::StringRef("dummy_string"));
const bool require_validation = true;
Why was this removed?
Comment at: source/Interpreter/OptionValueArgs.cpp:33
@@ -32,3 +32,3 @@
else
-args.SetArguments(argv.size(), [0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
args = argv;
Comment at: source/Interpreter/OptionValueArray.cpp:151
@@ -150,3 +150,3 @@
else
-args.SetArguments(argv.size(), [0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
args = argv;
Comment at: source/Target/ProcessInfo.cpp:94
@@ -93,3 +93,3 @@
bool first_arg_is_executable) {
- m_arguments.SetArguments(argv);
+ m_arguments.SetArguments(Args::ArgvToArrayRef(argv));
What not have a SetArguments that takes a "const char **" and does this
internally? It is no less safe and makes the API more usable
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner updated this revision to Diff 72657.
zturner added a comment.
Updated
https://reviews.llvm.org/D24952
Files:
examples/plugins/commands/fooplugin.cpp
include/lldb/API/SBCommandInterpreter.h
include/lldb/Host/OptionParser.h
include/lldb/Interpreter/Args.h
source/API/SBCommandInterpreter.cpp
source/API/SBLaunchInfo.cpp
source/API/SBProcess.cpp
source/API/SBTarget.cpp
source/Commands/CommandObjectBreakpoint.cpp
source/Commands/CommandObjectLog.cpp
source/Host/common/OptionParser.cpp
source/Interpreter/Args.cpp
source/Interpreter/CommandInterpreter.cpp
source/Interpreter/CommandObject.cpp
source/Interpreter/OptionValueArgs.cpp
source/Interpreter/OptionValueArray.cpp
source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
source/Target/ProcessInfo.cpp
source/Target/ProcessLaunchInfo.cpp
Index: source/Target/ProcessLaunchInfo.cpp
===
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -339,8 +339,8 @@
if (m_shell) {
std::string shell_executable = m_shell.GetPath();
- const char **argv = GetArguments().GetConstArgumentVector();
- if (argv == nullptr || argv[0] == nullptr)
+ const auto argv = GetArguments().GetArgumentVector();
+ if (!argv[0])
return false;
Args shell_arguments;
std::string safe_arg;
Index: source/Target/ProcessInfo.cpp
===
--- source/Target/ProcessInfo.cpp
+++ source/Target/ProcessInfo.cpp
@@ -91,7 +91,7 @@
void ProcessInfo::SetArguments(char const **argv,
bool first_arg_is_executable) {
- m_arguments.SetArguments(argv);
+ m_arguments.SetArguments(Args::ArgvToArrayRef(argv));
// Is the first argument the executable?
if (first_arg_is_executable) {
Index: source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -421,15 +421,11 @@
}
// Send the environment and the program + arguments after we connect
- const char **envp =
- launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-
- if (envp) {
-const char *env_entry;
-for (int i = 0; (env_entry = envp[i]); ++i) {
- if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0)
-break;
-}
+ const auto argv = launch_info.GetEnvironmentEntries().GetArgumentVector();
+
+ for (auto env_entry : argv) {
+if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0)
+ break;
}
ArchSpec arch_spec = launch_info.GetArchitecture();
Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1391,6 +1391,7 @@
if (!shell)
return 1;
+ // CLEANUP: This function could be made nicer / safer with StringRef.
std::string shell_string = shell.GetPath();
const char *shell_name = strrchr(shell_string.c_str(), '/');
if (shell_name == NULL)
@@ -1402,13 +1403,10 @@
// /bin/sh re-exec's itself as /bin/bash requiring another resume.
// But it only does this if the COMMAND_MODE environment variable
// is set to "legacy".
-const char **envp =
-launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-if (envp != NULL) {
- for (int i = 0; envp[i] != NULL; i++) {
-if (strcmp(envp[i], "COMMAND_MODE=legacy") == 0)
- return 2;
- }
+const auto argv = launch_info.GetEnvironmentEntries().GetArgumentVector();
+for (auto env_entry : argv) {
+ if (strcmp(env_entry, "COMMAND_MODE=legacy") == 0)
+return 2;
}
return 1;
} else if (strcmp(shell_name, "csh") == 0 ||
Index: source/Interpreter/OptionValueArray.cpp
===
--- source/Interpreter/OptionValueArray.cpp
+++ source/Interpreter/OptionValueArray.cpp
@@ -148,7 +148,7 @@
if (argv.empty())
args.Clear();
else
-args.SetArguments(argv.size(), [0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
}
Index: source/Interpreter/OptionValueArgs.cpp
===
--- source/Interpreter/OptionValueArgs.cpp
+++ source/Interpreter/OptionValueArgs.cpp
@@ -30,6 +30,6 @@
if (argv.empty())
args.Clear();
else
-args.SetArguments(argv.size(), [0]);
+args.SetArguments(argv);
return args.GetArgumentCount();
}
Index: source/Interpreter/CommandObject.cpp
===
--- source/Interpreter/CommandObject.cpp
+++
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
amccarth added a subscriber: amccarth.
Comment at: include/lldb/Interpreter/Args.h:154
@@ -167,3 +153,3 @@
//--
- const char **GetConstArgumentVector() const;
+ void GetArgumentVector(std::vector ) const;
Perhaps this should actually return the vector instead of void, and rely on the
return value optimization or, at worst, vector move. It would be easier to use
and it would let you initialize variables as you declare them, (which means you
could use it in constructor intializers and you could declare the vector as
const). It also would eliminate the ambiguity of what happens if the `args`
already contains some values (e.g., the caller might expect it to append rather
than replace).
Comment at: source/API/SBCommandInterpreter.cpp:119
@@ +118,3 @@
+std::vector argv;
+command.GetArgumentVector(argv);
+bool ret = m_backend->DoExecute(debugger_sb, [0], sb_return);
Here's an example where, if GetArgumentVector actually returned the result, you
could simplify the delcaration as:
const auto argv = command.GetArgumentVector();
Comment at: source/Host/common/FileSpec.cpp:251
@@ +250,3 @@
+
+ for (auto : name_list) {
+matches.AppendString(name);
Should `name` be `const`? As in:
for (const auto : name_list) { ... }
Comment at: source/Interpreter/Args.cpp:46
@@ -44,3 +45,3 @@
// strings in rhs.m_args. We need to copy the string list and update our
// own m_argv appropriately.
//--
I think your change eliminated the need for this comment. If not, it at least
needs an update.
Comment at: source/Interpreter/Args.cpp:275
@@ +274,3 @@
+ // new_argv.
+ // So make a copy and then swap it in.
+ std::vector new_args(new_argv.begin(), new_argv.end());
clang-format didn't do a great job wrapping the long lines in this comment.
Can you fix it up?
Comment at: source/Interpreter/Args.cpp:427
@@ -512,1 +426,3 @@
+ GetArgumentVector(args);
+ args.insert(args.begin(), "dummy");
while (1) {
This is an example where the ambiguity of whether GetArgumentVector appends or
not could mislead the caller into doing the wrong this. This looks plausible
(and more efficient):
std::vector args;
args.push_back("dummy");
GetArgumentVector(args);
If GetArgumentVector returned the vector, then this ambiguity wouldn't exist.
https://reviews.llvm.org/D24952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
labath added a subscriber: labath. Comment at: include/lldb/Interpreter/Args.h:150 @@ -147,19 +149,3 @@ /// /// @return /// An array of NULL terminate C string argument pointers that I think doxygen will complain about `@return` in a function returning void. BTW, with the NRVO, move semantics and everything, is there any reason to keep using the return-through-reference-argument pattern? It would be a lot more natural if this indeed _was_ a return value. https://reviews.llvm.org/D24952 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D24952: Remove Args::m_argv
zturner added a comment. Ignore the changes to `FileSpec`, it seems the two CLs I was working on got mixed together. https://reviews.llvm.org/D24952 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
