> On May 23, 2018, at 7:21 PM, Zachary Turner wrote:
>
>
> On Wed, May 23, 2018 at 7:04 PM Jim Ingham via Phabricator
> wrote:
> jingham added a comment.
>
> I worry when concerns of layering which seem a little artificial to me would
> make us add a shadow class for something that is alre
On Wed, May 23, 2018 at 7:04 PM Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:
> jingham added a comment.
>
> I worry when concerns of layering which seem a little artificial to me
> would make us add a shadow class for something that is already well
> expressed as it is. If you pa
jingham added a comment.
I worry when concerns of layering which seem a little artificial to me would
make us add a shadow class for something that is already well expressed as it
is. If you pass them as arguments, and then I add another useful one and want
to use it in both the regular and th
zturner added a comment.
FWIW `REPL` only uses 4 of the 10+ fields of
`CommandObjectExpression::Options`. If you don't like the separate struct
idea, I can just pass them directly as as independent arguments to
`REPL::SetCommandOptions`.
https://reviews.llvm.org/D47232
___
zturner added a comment.
In https://reviews.llvm.org/D47232#1108865, @jingham wrote:
> Perhaps a better way to handle this is to think of REPL.cpp/h as adjuncts of
> CommandObjectExpression.cpp/h - they mostly define the fancy IOHandler that
> gets pushed when you run the command in this mode.
jingham added a subscriber: zturner.
jingham added a comment.
The expression command had two modes before introducing the REPL. You can do:
(lldb) expr -- some C expression
or you can do:
(lldb) expr
Enter expressions, then terminate with an empty line to evaluate:
1: first C expression
2
The expression command had two modes before introducing the REPL. You can do:
(lldb) expr -- some C expression
or you can do:
(lldb) expr
Enter expressions, then terminate with an empty line to evaluate:
1: first C expression
2: Second C Expression
3:
The second only differs from the fi
labath added a comment.
Both of the solutions sound plausible to me (extra struct vs. moving REPL to
`Command`). Maybe that's because I don't know enough about the REPL to have
formed an opinion on it.
Comment at: lldb/include/lldb/Expression/REPL.h:31
+bool TryAllThreads
jingham added a comment.
Perhaps a better way to handle this is to think of REPL.cpp/h as adjuncts of
CommandObjectExpression.cpp/h - they mostly define the fancy IOHandler that
gets pushed when you run the command in this mode. I think it would be fine to
move them to Command, at which point
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
The REPL is just a mode of the expression command. You invoke it by saying:
(lldb) expr --repl --
or you can add any other options to it that you want, including flags like -i
or
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
Looks fine to me, but let's wait for Pavel. Are you back working on lldb ? :)
https://reviews.llvm.org/D47232
___
lldb-commits mailing list
lldb-
zturner created this revision.
zturner added reviewers: labath, davide.
The REPL (which lives in Expression) was making use of the command options for
the expression command. It's arguable whether `REPL` should even live in
`Expression` to begin with, but it makes more sense for Command to depe
12 matches
Mail list logo