Thanks for laying this out. I don't think any of us have a brief for getopt,
it was the tool at hand. Getting some fuzzy match suggestions would be nice.
Your comment about getopts reminds me, a more fundamental problem with lldb's
command interpreter is that the CommandObject is stateful.
labath added a comment.
There are several components in the "command line parsing"
1. argparse: parsing a single string into the individual words (processing
quotes, backslashes, etc.)
2. resolving the "command" part of the command line (the "frame variable" part
of "frame variable
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324775: Make LLDBs clang module cache path
customizable (authored by adrian, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
jingham added a comment.
To me, the determinant here is whether the llvm project sees many more
interactive tools in its future. If lldb is the only one, then moving all this
functionality to llvm seems like effort better spent elsewhere. But if there
are other interactive tools in the
zturner added a comment.
It's probably possible to make it work, but as Jim said, there's no drop in
replacement currently. There's bits and pieces of stuff that, with a dedicated
effort, could be improved to the point of being sufficient, though.
https://reviews.llvm.org/D43099
jingham added a comment.
I don't think there's any battle. I wouldn't mind moving the command parsing
code into llvm if there's a suitable replacement.
But so far as I can see llvm's command line parsing was primarily intended for
parsing args for shell tools. So it lacks features required
davide added inline comments.
Comment at: source/Target/Target.cpp:3949
+ idx);
+ assert(option_value);
+ return option_value->GetCurrentValue();
aprantl wrote:
> davide wrote:
> > add an assertion
aprantl added inline comments.
Comment at: source/Target/Target.cpp:3949
+ idx);
+ assert(option_value);
+ return option_value->GetCurrentValue();
davide wrote:
> add an assertion message if you
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
This looks fine to me.
https://reviews.llvm.org/D43099
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
davide accepted this revision.
davide added a comment.
LGTM modulo minor.
Comment at: source/Plugins/ExpressionParser/Clang/CMakeLists.txt:26
clangCodeGen
+clangDriver
clangEdit
aprantl wrote:
> I checked and this does not affects LLDB's binary
aprantl added inline comments.
Comment at: source/Plugins/ExpressionParser/Clang/CMakeLists.txt:26
clangCodeGen
+clangDriver
clangEdit
I checked and this does not affects LLDB's binary size in any measurable way.
https://reviews.llvm.org/D43099
aprantl added inline comments.
Comment at:
source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:595-601
+if (Path.empty()) {
+ // This code is copied from the Clang driver.
+ const bool erased_on_reboot = false;
+
aprantl updated this revision to Diff 133636.
aprantl added a comment.
Herald added subscribers: hintonda, mgorny.
Address most review feedback.
https://reviews.llvm.org/D43099
Files:
include/lldb/Target/Target.h
packages/Python/lldbsuite/test/lldbtest.py
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
Think about whether it would be better to have GetClangModulesCachePath
calculate the fallback modules path rather than having the client do it?
Comment at:
aprantl created this revision.
aprantl added a reviewer: jingham.
This patch makes LLDB's clang module cache path customizable via `settings set
target.clang-modules-cache-path ` and uses it in the LLDB testsuite to
reuse the same location inside the build directory for LLDB and clang.
15 matches
Mail list logo
