Summary:
This patch removes File::SetStream() and File::SetDescriptor(),
and replaces most direct uses of File with pointers to File.
Instead of calling SetStream() on a file, we make a new file and
replace it.
My ultimate goal here is to introduce a new API class SBFile, which
has full support for python io.IOStream file objects. These can
redirect read() and write() to python code, so lldb::Files will
need a way to dispatch those methods. Additionally it will need some
form of sharing and assigning files, as a SBFile will be passed in and
assigned to the main IO streams of the debugger.
In my prototype patch queue, I make File itself copyable and add a
secondary class FileOps to manage the sharing and dispatch. In that
case SBFile was a unique_ptr<File>.
(here: https://github.com/smoofra/llvm-project/tree/files)
However in review, Pavel Labath suggested that it be shared_ptr instead.
(here: https://reviews.llvm.org/D67793)
In order for SBFile to use shared_ptr<File>, everything else should
as well.
If this patch is accepted, I will make SBFile use a shared_ptr
I will remove FileOps from future patches and use subclasses of File
instead.
Reviewers: JDevlieghere, jasonmolenda, zturner, jingham, labath
Reviewed By: labath
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D67891
llvm-svn: 373090
The StringRef should always be identical to the C string, so we
might as well just create the StringRef from the C-string. This
might be slightly slower until we implement the storage of ArgEntry
with a string instead of a std::unique_ptr<char[]>. Until then we
have to do the additional strlen on the C string to construct the
StringRef.
llvm-svn: 371842
Summary:
We currently have a bunch of code that is supposed to handle invalid command options, but
all this code is unreachable because invalid options are already handled in `Options::Parse`.
The only way we can reach this code is when we declare but then not implement an option
(which will be made impossible with D65386, which is also when we can completely remove
the `default` cases).
This patch replaces all this code with `llvm_unreachable` to make clear this is dead code
that can't be reached.
Reviewers: JDevlieghere
Reviewed By: JDevlieghere
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D66522
llvm-svn: 369625
Summary:
We still have some leftovers of the old completion API in the internals of
LLDB that haven't been replaced by the new CompletionRequest. These leftovers
are:
* The return values (int/size_t) in all completion functions.
* Our result array that starts indexing at 1.
* `WordComplete` mode.
I didn't replace them back then because it's tricky to figure out what exactly they
are used for and the completion code is relatively untested. I finally got around
to writing more tests for the API and understanding the semantics, so I think it's
a good time to get rid of them.
A few words why those things should be removed/replaced:
* The return values are really cryptic, partly redundant and rarely documented.
They are also completely ignored by Xcode, so whatever information they contain will end up
breaking Xcode's completion mechanism. They are also partly impossible to even implement
as we assign negative values special meaning and our completion API sometimes returns size_t.
Completion functions are supposed to return -2 to rewrite the current line. We seem to use this
in some untested code path to expand the history repeat character to the full command, but
I haven't figured out why that doesn't work at the moment.
Completion functions return -1 to 'insert the completion character', but that isn't implemented
(even though we seem to activate this feature in LLDB sometimes).
All positive values have to match the number of results. This is obviously just redundant information
as the user can just look at the result list to get that information (which is what Xcode does).
* The result array that starts indexing at 1 is obviously unexpected. The first element of the array is
reserved for the common prefix of all completions (e.g. "foobar" and "footar" -> "foo"). The idea is
that we calculate this to make the life of the API caller easier, but obviously forcing people to have
1-based indices is not helpful (or even worse, forces them to manually copy the results to make it
0-based like Xcode has to do).
* The `WordComplete` mode indicates that LLDB should enter a space behind the completion. The
idea is that we let the top-level API know that we just provided a full completion. Interestingly we
`WordComplete` is just a single bool that somehow represents all N completions. And we always
provide full completions in LLDB, so in theory it should always be true.
The only use it currently serves is providing redundant information about whether we have a single
definitive completion or not (which we already know from the number of results we get).
This patch essentially removes `WordComplete` mode and makes the result array indexed from 0.
It also removes all return values from all internal completion functions. The only non-redundant information
they contain is about rewriting the current line (which is broken), so that functionality was moved
to the CompletionRequest API. So you can now do `addCompletion("blub", "description", CompletionMode::RewriteLine)`
to do the same.
For the SB API we emulate the old behaviour by making the array indexed from 1 again with the common
prefix at index 0. I didn't keep the special negative return codes as we either never sent them before (e.g. -2) or we
didn't even implement them in the Editline handler (e.g. -1).
I tried to keep this patch minimal and I'm aware we can probably now even further simplify a bunch of related code,
but I would prefer doing this in follow-up NFC commits
Reviewers: JDevlieghere
Reviewed By: JDevlieghere
Subscribers: arphaman, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D66536
llvm-svn: 369624
Now that we've moved to C++14, we no longer need the llvm::make_unique
implementation from STLExtras.h. This patch is a mechanical replacement
of (hopefully) all the llvm::make_unique instances across the monorepo.
Differential revision: https://reviews.llvm.org/D66259
llvm-svn: 368933
Reformat OptionEnumValueElement to make it easier to distinguish between
its fields. This also removes the need to disable clang-format for these
arrays.
Differential revision: https://reviews.llvm.org/D65489
llvm-svn: 367638
Summary:
Right now our CommandOptions.inc only generates the initializer for the options list but
not the array declaration boilerplate around it. As the array definition is identical for all arrays,
we might as well also let the CommandOptions.inc generate it alongside the initializers.
This patch will also allow us to generate additional declarations related to that option list in
the future (e.g. a enum class representing the specific options which would make our
handling code less prone).
This patch also fixes a few option tables that didn't follow our naming style.
Reviewers: JDevlieghere
Reviewed By: JDevlieghere
Subscribers: abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D65331
llvm-svn: 367186
This patch ensures that we propagate errors coming from the lldbinit
file trough the command/script interpreter. Before, if you did something
like command script import syntax_error.py, and the python file
contained a syntax error, lldb wouldn't tell you about it. This changes
with the current patch: errors are now propagated by default.
PS: Jim authored this change and I added testing.
Differential revision: https://reviews.llvm.org/D61579
llvm-svn: 360216
This patch ensures that we honor the stop-command-source-on-error
setting from `command source`. The problem is that we didn't
differentiate between the boolean value being true or false, or not
being set. For the latter scenario, we should calculate the value in the
command interpreter based on the global options.
Differential revision: https://reviews.llvm.org/D61406
llvm-svn: 359750
In r359354 a GetDebugger() method was added to the CommandObject class,
so that we didn't have to go through the command interpreter to obtain
the script interpreter. This patch simplifies other call sites where
m_interpreter.GetDebugger() was used, and replaces them with a shorter
call to the new method.
llvm-svn: 359373
This is part two of the change started in r359330. This patch moves the
ownership of the script interpreter from the command interpreter into
the debugger. I would've preferred to remove the lazy initialization,
however the fact that the scripting language is set after the debugger
is created makes that tricky. So for now this does exactly the same
thing as when it was under the command interpreter. The result is that
this patch is fully NFC.
Differential revision: https://reviews.llvm.org/D61211
llvm-svn: 359354
A lot of comments in LLDB are surrounded by an ASCII line to delimit the
begging and end of the comment.
Its use is not really consistent across the code base, sometimes the
lines are longer, sometimes they are shorter and sometimes they are
omitted. Furthermore, it looks kind of weird with the 80 column limit,
where the comment actually extends past the line, but not by much.
Furthermore, when /// is used for Doxygen comments, it looks
particularly odd. And when // is used, it incorrectly gives the
impression that it's actually a Doxygen comment.
I assume these lines were added to improve distinguishing between
comments and code. However, given that todays editors and IDEs do a
great job at highlighting comments, I think it's worth to drop this for
the sake of consistency. The alternative is fixing all the
inconsistencies, which would create a lot more churn.
Differential revision: https://reviews.llvm.org/D60508
llvm-svn: 358135
Summary:
Within .lldbinit, regex commands can be structured as a list of substitutions over
multiple lines. It's possible that this is uninentional, but it works and has
benefits.
For example:
command regex <command-name>
s/pat1/repl1/
s/pat2/repl2/
...
I use this form of `command regex` in my `~/.lldbinit`, because it makes it
clearer to write and read compared to a single line definition, because
multiline substitutions don't need to be quoted, and are broken up one per line.
However, multiline definitions result in usage instructions being printed for
each use. The result is that every time I run `lldb`, I get a dozen or more
lines of noise. With this change, the instructions are only printed when
`command regex` is invoked interactively, or from a terminal, neither of which
are true when lldb is sourcing `~/.lldbinit`.
Reviewers: clayborg, jingham
Reviewed By: clayborg
Subscribers: jdoerfert, kastiglione, xiaobai, keith, lldb-commits
Differential Revision: https://reviews.llvm.org/D48752
llvm-svn: 355793
This patch adds the necessary logic to capture and replay commands
entered into the command interpreter. A DataRecorder shadows the input
and writes its data to a know file. During replay this file is used as
the command interpreter's input.
It's possible to the command interpreter more than once, with a
different input source. We support this scenario by using multiple
buffers. The synchronization for this takes place at the SB layer, where
we create a new recorder every time the debugger input is changed.
During replay we use the corresponding buffer as input.
Differential revision: https://reviews.llvm.org/D58564
llvm-svn: 355249
The `ap` suffix is a remnant of lldb's former use of auto pointers,
before they got deprecated. Although all their uses were replaced by
unique pointers, some variables still carried the suffix.
In r353795 I removed another auto_ptr remnant, namely redundant calls to
::get for unique_pointers. Jim justly noted that this is a good
opportunity to clean up the variable names as well.
I went over all the changes to ensure my find-and-replace didn't have
any undesired side-effects. I hope I didn't miss any, but if you end up
at this commit doing a git blame on a weirdly named variable, please
know that the change was unintentional.
llvm-svn: 353912
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
This patch removes the comments grouping header includes. They were
added after running IWYU over the LLDB codebase. However they add little
value, are often outdates and burdensome to maintain.
llvm-svn: 346626
This patch removes the logic for resolving paths out of FileSpec and
updates call sites to rely on the FileSystem class instead.
Differential revision: https://reviews.llvm.org/D53915
llvm-svn: 345890
Summary:
Add settings to control command echoing:
```
(lldb) settings set interpreter.echo-commands true
(lldb) settings set interpreter.echo-comment-commands true
```
Both settings default to true, which keeps LLDB's existing behavior in non-interactive mode (echo all command inputs to the output).
So far the only way to change this behavior was the `--source-quietly` flag, which disables all output including evaluation results.
Now `echo-commands` allows to turn off echoing for commands, while evaluation results are still printed. No effect if `--source-quietly` was present.
`echo-comment-commands` allows to turn off echoing for commands in case they are pure comment lines. No effect if `echo-commands` is false.
Note that the behavior does not change immediately! The new settings take effect only with the next command source.
LLDB lit test are the main motivation for this feature. So far incoming `#CHECK` line have always been echoed to the output and so they could never fail. Now we can disable it in lit-lldb-init.
Todos: Finish test for this feature. Add to lit-lldb-init. Check for failing lit tests.
Reviewers: aprantl, jasonmolenda, JDevlieghere
Subscribers: friss, lldb-commits
Differential Revision: https://reviews.llvm.org/D52788
llvm-svn: 343859
Summary:
We currently allow any completion handler to read and manipulate the list of matches we
calculated so far. This leads to a few problems:
Firstly, a completion handler's logic can now depend on previously calculated results
by another handlers. No completion handler should have such an implicit dependency,
but the current API makes it likely that this could happen (or already happens). Especially
the fact that some completion handler deleted all previously calculated results can mess
things up right now.
Secondly, all completion handlers have knowledge about our internal data structures with
this API. This makes refactoring this internal data structure much harder than it should be.
Especially planned changes like the support of descriptions for completions are currently
giant patches because we have to refactor every single completion handler.
This patch narrows the contract the CompletionRequest has with the different handlers to:
1. A handler can suggest a completion.
2. A handler can ask how many suggestions we already have.
Point 2 obviously means we still have a dependency left between the different handlers, but
getting rid of this is too large to just append it to this patch.
Otherwise this patch just completely hides the internal StringList to the different handlers.
The CompletionRequest API now also ensures that the list of completions is unique and we
don't suggest the same value multiple times to the user. This property has been so far only
been ensured by the `Option` handler, but is now applied globally. This is part of this patch
as the OptionHandler is no longer able to implement this functionality itself.
Reviewers: jingham, davide, labath
Reviewed By: davide
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D49322
llvm-svn: 338151
Summary:
As suggested in D48796, this patch replaces even more internal calls that were using the old
completion API style with a single CompletionRequest. In some cases we also pass an option
vector/index, but as we don't always have this information, it currently is not part of the
CompletionRequest class.
The constructor of the CompletionRequest is now also more sensible. You only pass the
user input, cursor position and your list of matches to the request and the rest will be
inferred (using the same code we used before to calculate this). You also have to pass these
match window parameters to it, even though they are unused right now.
The patch shouldn't change any behavior.
Reviewers: jingham
Reviewed By: jingham
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D48976
llvm-svn: 337031
Summary:
This patch gets rid of the C-string parameter in the RawCommandObject::DoExecute function,
making the code simpler and less memory unsafe.
There seems to be a assumption in some command objects that this parameter could be a nullptr,
but from what I can see the rest of the API doesn't actually allow this (and other command
objects and related code pieces dereference this parameter without any checks).
Especially CommandObjectRegexCommand has error handling code for a nullptr that is now gone.
Reviewers: davide, jingham, teemperor
Reviewed By: teemperor
Subscribers: jingham, lldb-commits
Differential Revision: https://reviews.llvm.org/D49207
llvm-svn: 336955
Summary:
A subset of the LLDB commands follows this command line interface style:
<command name> [arguments] -- <string suffix>
The parsing code for this interface has been so far been duplicated into the different
command objects which makes it hard to maintain and reuse elsewhere.
This patches improves the situation by adding a OptionsWithRaw class that centralizes
the parsing logic and allows easier testing. The different commands now just call this class to
extract the arguments and the raw suffix from the provided user input.
Reviewers: jingham
Reviewed By: jingham
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D49106
llvm-svn: 336723
Summary:
This patch refactors the internal completion API. It now takes (as far as possible) a single
CompletionRequest object instead o half a dozen in/out/in-out parameters. The CompletionRequest
contains a common superset of the different parameters as far as it makes sense. This includes
the raw command line string and raw cursor position, which should make the `expr` command
possible to implement (at least without hacks that reconstruct the command line from the args).
This patch is not intended to change the observable behavior of lldb in any way. It's also as
minimal as possible and doesn't attempt to fix all the problems the API has.
Some Q&A:
Q: Why is this not fixing all the problems in the completion API?
A: Because is a blocker for the expr command completion which I want to get in ASAP. This is the
smallest patch that unblocks the expr completion patch and which allows trivial refactoring in the future.
The patch also doesn't really change the internal information flow in the API, so that hopefully
saves us from ever having to revert and resubmit this humongous patch.
Q: Can we merge all the copy-pasted code in the completion methods
(like computing the current incomplete arg) into CompletionRequest class?
A: Yes, but it's out of scope for this patch.
Q: Why the `word_complete = request.GetWordComplete(); ... ` pattern?
A: I don't want to add a getter that returns a reference to the internal integer. So we have
to use a temporary variable and the Getter/Setter instead. We don't throw exceptions
from what I can tell, so the behavior doesn't change.
Q: Why are we not owning the list of matches?
A: Because that's how the previous API works. But that should be fixed too (in another patch).
Q: Can we make the constructor simpler and compute some of the values from the plain command?
A: I think this works, but I rather want to have this in a follow up commit. Especially when making nested
request it's a bit awkward that the parsed arguments behave as both input/output (as we should in theory
propagate the changes on the nested request back to the parent request if we don't want to change the
behavior too much).
Q: Can't we pass one const request object and then just return another result object instead of mixing
them together in one in/out parameter?
A: It's hard to get keep the same behavior with that pattern, but I think we can also get a nice API with just
a single request object. If we make all input parameters read-only, we have a clear separation between what
is actually an input and what an output parameter (and hopefully we get rid of the in-out parameters).
Q: Can we throw out the 'match' variables that are not implemented according to the comment?
A: We currently just forward them as in the old code to the different methods, even though I think
they are really not used. We can easily remove and readd them once every single completion method just
takes a CompletionRequest, but for now I prefer NFC behavior from the perspective of the API user.
Reviewers: davide, jingham, labath
Reviewed By: jingham
Subscribers: mgorny, friss, lldb-commits
Differential Revision: https://reviews.llvm.org/D48796
llvm-svn: 336146
This is intended as a clean up after the big clang-format commit
(r280751), which unfortunately resulted in many of the comment
paragraphs in LLDB being very hard to read.
FYI, the script I used was:
import textwrap
import commands
import os
import sys
import re
tmp = "%s.tmp"%sys.argv[1]
out = open(tmp, "w+")
with open(sys.argv[1], "r") as f:
header = ""
text = ""
comment = re.compile(r'^( *//) ([^ ].*)$')
special = re.compile(r'^((([A-Z]+[: ])|([0-9]+ )).*)|(.*;)$')
for line in f:
match = comment.match(line)
if match and not special.match(match.group(2)):
# skip intentionally short comments.
if not text and len(match.group(2)) < 40:
out.write(line)
continue
if text:
text += " " + match.group(2)
else:
header = match.group(1)
text = match.group(2)
continue
if text:
filled = textwrap.wrap(text, width=(78-len(header)),
break_long_words=False)
for l in filled:
out.write(header+" "+l+'\n')
text = ""
out.write(line)
os.rename(tmp, sys.argv[1])
Differential Revision: https://reviews.llvm.org/D46144
llvm-svn: 331197
Summary:
The Args class is used in plenty of places besides the command
interpreter (e.g., anything requiring an argc+argv combo, such as when
launching a process), so it needs to be in a lower layer. Now that the
class has no external dependencies, it can be moved down to the Utility
module.
This removes the last (direct) dependency from the Host module to
Interpreter, so I remove the Interpreter module from Host's dependency
list.
Reviewers: zturner, jingham, davide
Subscribers: mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D45480
llvm-svn: 330200
Summary:
The idea behind this is to move the functionality which depend on other lldb
classes into a separate class. This way, the Args class can be turned
into a lightweight arc+argv wrapper and moved into the lower lldb
layers.
Reviewers: jingham, zturner
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D44306
llvm-svn: 329677
This renames the LLDB error class to Status, as discussed
on the lldb-dev mailing list.
A change of this magnitude cannot easily be done without
find and replace, but that has potential to catch unwanted
occurrences of common strings such as "Error". Every effort
was made to find all the obvious things such as the word "Error"
appearing in a string, etc, but it's possible there are still
some lingering occurences left around. Hopefully nothing too
serious.
llvm-svn: 302872
The long-term goal here is to get rid of the functions
GetArgumentAtIndex() and GetQuoteCharAtIndex(), instead
replacing them with operator based access and range-based for
enumeration. There are a lot of callsites, though, so the
changes will be done incrementally, starting with this one.
Differential Revision: https://reviews.llvm.org/D26883
llvm-svn: 287597
This is a large API change that removes the two functions from
StreamString that return a std::string& and a const std::string&,
and instead provide one function which returns a StringRef.
Direct access to the underlying buffer violates the concept of
a "stream" which is intended to provide forward only access,
and makes porting to llvm::raw_ostream more difficult in the
future.
Differential Revision: https://reviews.llvm.org/D26698
llvm-svn: 287152
This is better for a number of reasons. Mostly style, but also:
1) Signed-unsigned comparison warnings disappear since there is
no loop index.
2) Iterating with the range-for style gives you back an entry
that has more than just a const char*, so it's more efficient
and more useful.
3) Makes code safter since the type system enforces that it's
impossible to index out of bounds.
llvm-svn: 283413
Lawrence D'Anna