aprantl added a comment.
Does that mean we can now also remove the #ifdef __APPLE__ from the objectfile
unit tests?
https://reviews.llvm.org/D46934
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
aprantl added a comment.
> The advantage of the second one is that we will have the ability to inject
> commands which depend on the results of previous commands (something that I
> think we will need, sooner or later).
That is worth considering. To write good tests that depend on previous re
aprantl added a comment.
My overall mental image of lldb-mi is very incomplete, but I'm imagining
lldb-mi as having one thread that wakes up every n milliseconds checks for
command input and then calls into the SBAPI to handle the commands. If that is
how it works, then one very simple thing we
aprantl added inline comments.
Comment at: source/Plugins/Process/Utility/RegisterContextDarwinConstants.h:18
+ KERNEL_SUCCESS = 0,
+ KERNEL_INVALID_ARGUMENT = 4,
+};
I think I would prefer
#ifndef KERN_INVALID_ARGUMENT
#define KERN_INVALID_ARGUMENT 4
#endif
aprantl added a comment.
In https://reviews.llvm.org/D46934#1102867, @labath wrote:
> In https://reviews.llvm.org/D46934#1101963, @aprantl wrote:
>
> > Does that mean we can now also remove the #ifdef __APPLE__ from the
> > objectfile unit tests?
>
>
> Which ones do you mean? I wasn't aware we h
aprantl added a comment.
For the experiment you can probably just stick it into
`CMICmnLLDBDebugger::InitSBDebugger()`.
Repository:
rL LLVM
https://reviews.llvm.org/D46588
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.ll
aprantl added a comment.
Okay, that sounds promising! Then let's proceed this way:
- Add a new command line option to lldb-mi that is called `--synchronous` with
a help text "Block until each command has finished executing. Used for testing
only." and use it in this test
- continue writing as m
aprantl added inline comments.
Comment at: packages/Python/lldbsuite/test/dosep.py:122
print("[%s FAILED]%s" % (name, timeout_str), file=sys.stderr)
-print("Command invoked: %s" % ' '.join(command), file=sys.stderr)
+print("Reproduce with: lld
aprantl added inline comments.
Comment at: packages/Python/lldbsuite/test/dosep.py:122
print("[%s FAILED]%s" % (name, timeout_str), file=sys.stderr)
-print("Command invoked: %s" % ' '.join(command), file=sys.stderr)
+print("Reproduce with: lld
aprantl added subscribers: jingham, jasonmolenda, labath.
aprantl added a comment.
Thanks for jumping on this Amara — I just wanted to point out that we ususally
don't revert lldb changes that only break the lldb-xcode bot if they pass on
the lldb-cmake bot at the same time. When this happens it
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
Looks good!
Repository:
rL LLVM
https://reviews.llvm.org/D47110
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.
aprantl accepted this revision.
aprantl added a comment.
LGTM!
Repository:
rL LLVM
https://reviews.llvm.org/D46588
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl created this revision.
aprantl added reviewers: zturner, jingham.
Herald added subscribers: jkorous, MaskRay, ioeric, ilya-biryukov, mgorny.
@zturner wrote:
> This change has introduced a dependency from Core -> clang Driver (due to
> #include "clang/Driver/Driver.h" in ModuleList.cpp).
aprantl updated this revision to Diff 148241.
aprantl added a comment.
Updated patch according to Zachary's suggestion.
https://reviews.llvm.org/D47235
Files:
include/lldb/Core/ModuleList.h
include/lldb/Host/HostInfoBase.h
source/API/SystemInitializerFull.cpp
source/Core/ModuleList.cpp
aprantl updated this revision to Diff 148248.
aprantl added a comment.
Forgot to update unit tests.
https://reviews.llvm.org/D47235
Files:
include/lldb/API/SystemInitializerFull.h
include/lldb/Core/ModuleList.h
include/lldb/Host/HostInfoBase.h
source/API/SystemInitializerFull.cpp
sour
aprantl added a comment.
In https://reviews.llvm.org/D46005#1109817, @zturner wrote:
> In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
>
> > In https://reviews.llvm.org/D46005#1109693, @zturner wrote:
> >
> > > FWIW, I think the single biggest improvement one could make to the LL
aprantl updated this revision to Diff 148264.
aprantl added a comment.
Remove whitespace changes.
https://reviews.llvm.org/D47235
Files:
include/lldb/API/SystemInitializerFull.h
include/lldb/Core/ModuleList.h
source/API/SystemInitializerFull.cpp
source/Core/ModuleList.cpp
tools/lldb-t
aprantl updated this revision to Diff 148265.
aprantl added a comment.
Remove unused include.
https://reviews.llvm.org/D47235
Files:
include/lldb/API/SystemInitializerFull.h
include/lldb/Core/ModuleList.h
source/API/SystemInitializerFull.cpp
source/Core/ModuleList.cpp
tools/lldb-test/
aprantl added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
aprantl added a comment.
In https://reviews.llvm.org/D46005#1109920, @zturner wrote:
> In https://reviews.llvm.org/D46005#1109911, @aprantl wrote:
>
> > In https://reviews.llvm.org/D46005#1109817, @zturner wrote:
> >
> > > In https://reviews.llvm.org/D46005#1109761, @JDevlieghere wrote:
> > >
> >
aprantl added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
aprantl added inline comments.
Comment at: source/Core/ModuleList.cpp:94
- llvm::SmallString<128> path;
- clang::driver::Driver::getDefaultModuleCachePath(path);
- SetClangModulesCachePath(path);
+ assert(!g_default_clang_modules_cache_path.empty());
+ SetClangModulesCache
aprantl added a comment.
The missing context here is that the lldb-mi -target-select command currently
calls `HandleCommand("target modules search-paths add", ...)`.
Is adding a new SBAPI the right approach to implementing this functionality
without going through HandleCommand? Or is HandleComma
aprantl added inline comments.
Comment at: lit/lit.cfg:61
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
It looks like "lldb-mi" may not be a valid substitution. On Darwin the command
`# RUN: %lldb_mi `
is expanded to
bin/lldb -S .../llvm/tools/lldb/
aprantl added inline comments.
Comment at: lit/lit.cfg:61
+lldb_mi = lit.util.which('lldb-mi', lldb_tools_dir)
+
aprantl wrote:
> It looks like "lldb-mi" may not be a valid substitution. On Darwin the command
>
> `# RUN: %lldb_mi `
>
> is expanded to
>
> bin
aprantl added a comment.
Generally I'm fine with improving the layering. I just wanted to point out that
the Swift language plugin also wants know the clang resource directory (it
calls `HostInfo::GetLLDBPath(ePathTypeClangDir, clang_dir_spec)`) since Swift
embeds Clang. That said it makes no s
aprantl added a comment.
Nice. It should be easy to also create a test for this that just specifies an
invalid filename and verifies that lldb-mi returns a failure?
Repository:
rL LLVM
https://reviews.llvm.org/D47678
___
lldb-commits mailing lis
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
thanks.
https://reviews.llvm.org/D47678
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
aprantl added a comment.
Stella, is there a public bot that runs the tests on Windows that we could
watch? I found
http://lab.llvm.org:8011/builders/lldb-x86-windows-msvc2015?numbuilds=1000 but
it doesn't look like that one actually failed with the error you saw.
Comment at:
aprantl added inline comments.
Comment at: lit/tools/lldb-mi/breakpoint/break-insert.test:1
-# REQUIRES: nowindows
+# XFAIL: windows
# -> llvm.org/pr24452
aprantl wrote:
> Do we still expect the test to fail after this change?
Nevermind.. phabricator folded your
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.
Comment at: lit/tools/lldb-mi/exec/exec-next.test:2
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
@stella.stemanova: Would be interesting to understand w
aprantl accepted this revision.
aprantl added inline comments.
Comment at: lit/tools/lldb-mi/exec/exec-next.test:19
+
+-exec-next --thread 0
+# Check that exec-next can process the case of invalid thread ID.
0 feels like it might actually be a valid thread id on
aprantl added a comment.
Did the old implementation come with a testcase? Perhaps I'm misunderstanding
the question, but it would probably be best to preserve the old behavior.
Comment at: tools/lldb-mi/MICmdCmdExec.cpp:515
+ lldb::SBError error;
+ if (nThreadId != UINT64_MA
aprantl added a comment.
That's awesome! May I ask why you chose to use lldb_Component modules instead
of doing submodules? Is this for build performance?
https://reviews.llvm.org/D47929
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
htt
aprantl added a comment.
There is no testcase... is this used in a subsequent commit?
Comment at: lldb/trunk/tools/lldb-mi/MICmdBase.cpp:223
+// Throws: None.
+//--
+void CMICmdBase::SetError(const lldb::SBError &error) {
At some point we should convert the en
aprantl added a comment.
Okay.. reverting is cheap, so please go ahead.
Repository:
rL LLVM
https://reviews.llvm.org/D47914
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl added a comment.
This looks reasonable to me, but I'd like to also hear from Greg and Jim since
SBAPI changes are kind of final.
Comment at: include/lldb/API/SBThread.h:96
+ void StepOver(SBError &error,
+lldb::RunMode stop_other_threads = lldb::eOnl
aprantl added a comment.
Hmm.. it looks like most C++ API calls don't have any documentation.
http://lldb.llvm.org/cpp_reference/html/classlldb_1_1SBThread.html#a42755a170e127881a5dd65162217f68b
It does look like the python API has more documentation.. where does that come
from?
http://lldb.llv
aprantl added a comment.
It would be good to add documentation for the new API call there, then.
https://reviews.llvm.org/D47991
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl added a comment.
No, but I found this: http://www.swig.org/Doc1.3/Python.html#Python_nn67
https://reviews.llvm.org/D47991
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl added a comment.
Have you looked into making the error the first vs the last argument? If the
majority of all SBAPI calls put the error last, we should do this here, too.
https://reviews.llvm.org/D47991
___
lldb-commits mailing list
lldb-co
aprantl accepted this revision.
aprantl added a comment.
Ah I see. That's because the last argument is a C++ default argument. It looks
like the convention in this file is that the error argument should be the last
non-defaulted argument.
https://reviews.llvm.org/D47991
aprantl added a comment.
Hmm.. @lemo 's reasoning
Comment at: scripts/interface/SBThread.i:257
+%feature("autodoc",
+"Do a instruction level single step in the currently selected thread.
+") StepInstruction;
a -> an
Comment at: so
aprantl added a comment.
> I agree with Leonard, for the StepOver overload that returns errors, just
> make the mode parameter required. That will reduce confusion.
Works for me, too.
Comment at: source/API/SBThread.cpp:1136
bool result = false;
if (exe_ctx.HasThreadSco
aprantl accepted this revision.
aprantl added inline comments.
Comment at: source/Host/CMakeLists.txt:7
+# Removes all module flags from the current CMAKE_CXX_FLAGS. Used for
+# the Obj-C++ code in lldb which we don't want to build with modules.
+# Reasons for this are that modul
aprantl added inline comments.
Comment at: source/Host/CMakeLists.txt:7
+# Removes all module flags from the current CMAKE_CXX_FLAGS. Used for
+# the Obj-C++ code in lldb which we don't want to build with modules.
+# Reasons for this are that modules with Obj-C++ would require th
aprantl added inline comments.
Comment at: source/Plugins/Language/ObjC/Cocoa.cpp:462
+ if (!strcmp(class_name, "NSDecimalNumber"))
+return NSDecimalNumberSummaryProvider(valobj, stream, options);
Side note: It would be slightly faster/elegant to use a Str
aprantl added a comment.
I'm getting a new warning now, can you also reproduce this?
In file included from :21:
In file included from ../tools/lldb/include/lldb/Host/MainLoop.h:13:
tools/lldb/include/lldb/Host/Config.h:33:9: warning: 'HAVE_LIBCOMPRESSION'
macro redefined [-Wmacro-redefined
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.
Comment at: tools/lldb-mi/MICmdBase.cpp:221
+// Args:error - (R) Error description object.
+// Return: None.
+// Throws: None.
It returns a bool,
aprantl added a comment.
> Also, I've been thinking about another approach with having a method in
> CMICmdBase that takes two parameters: pointers to a functions in which user
> could specify needed actions. But the main problem is that we don't have a
> knowledge about these functions, they m
aprantl added inline comments.
Comment at: tools/lldb-mi/MICmdBase.cpp:221
+// Args:error - (R) Error description object.
+// Return: None.
+// Throws: None.
apolyakov wrote:
> aprantl wrote:
> > It returns a bool, right?
> >
> > At some point it sure woul
aprantl added a comment.
Can you post a more concrete example? I think this sounds like a good idea.
https://reviews.llvm.org/D48295
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commit
aprantl added a comment.
That sounds like a good overall direction, though I probably wouldn't
communicate the function pointers via member variables but rather prefer them
to be passed as function arguments or return values. This makes the flow a
little more obvious for readers.
https://revi
aprantl added a comment.
In this design the success_handlers return an exit status *and* set a string
error. We could unify this by having the handler return an llvm::Error
(https://llvm.org/doxygen/classllvm_1_1Error.html). When it is successful, it
returns Error::success, otherwise it returns
aprantl added a comment.
In https://reviews.llvm.org/D48295#1136692, @apolyakov wrote:
> I don't completely understand what you mean. First of all, what do you mean
> when talking about success_handlers? 'cause if you mean success_handler from
> `Execute` function, then I should say that it doe
aprantl added a comment.
I think I misread your patch. Now the naming of success_handler makes much more
sense, too.
What do you think about defining a function that takes an SBError result, and a
function that converts this error into a string? This would allow chaining more
than one SBAPI co
aprantl added a comment.
Ok, then let's continue this way.
Comment at: tools/lldb-mi/MICmdBase.h:89
+ [] { return MIstatus::failure; },
+ const lldb::SBError error = lldb::SBError());
template T *GetOption(const CMIUtilString &vStrO
aprantl added a comment.
In https://reviews.llvm.org/D48295#1137595, @apolyakov wrote:
> Changed method's name from `ReturnMIStatus` to `HandleSBError`. Also added
> one more use case(see -exec-step's Execute function).
>
> The only thing that worries me is that if we want to specify handler for
aprantl added a comment.
This patch is adding new overloads to SBAPI calls that don't return an SBError,
such as:
// Old:
void StepOutOfFrame(SBFrame &frame);
// New:
void StepOutOfFrame(SBFrame &frame, SBError &error);
I wonder if it would be easier to use and more consistent with the
aprantl accepted this revision.
aprantl added a comment.
Okay.. then let's go with this version.
https://reviews.llvm.org/D47991
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl added a comment.
In https://reviews.llvm.org/D47991#1138029, @jingham wrote:
> Won't this break client code that was calling StepOver? We are pretty
> serious about maintaining binary compatibility with the SB API's.
Yeah, we can't replace existing function calls: The C++ name manglin
aprantl added a comment.
I guess you *could* use different function names, such as HandleSBError,
HandleSBErrorWithSuccess, HandleSBErrorWithSuccessAndFailure, ...
https://reviews.llvm.org/D48295
___
lldb-commits mailing list
lldb-commits@lists.ll
aprantl added a comment.
In https://reviews.llvm.org/D47991#1139249, @clayborg wrote:
> that is the right way to do it, but we can't overload on return type only. We
> will need the old version of the code to be in the API for compatibility.
> Overloading by return type will result is two symbo
aprantl accepted this revision.
aprantl added a comment.
I think that works. Assuming that we can use these new helpers in lots of other
places :-)
https://reviews.llvm.org/D48295
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lis
aprantl added a comment.
Perhaps it's supposed to mark a return argument? The lldb-mi tool uses a
(compared to the rest of the project) very odd Windows-like coding style that
I'm not particularly familiar with. It would be nice to convert all of it over
to just use the plain LLVM style so it i
aprantl added a comment.
Very nice!
Comment at: include/lldb/Expression/ExpressionParser.h:52
+ virtual bool Complete(StringList &matches, unsigned line, unsigned pos,
+unsigned typed_pos) = 0;
Could you add a Doxygen comment?
=
aprantl added a comment.
I don't have a problem with dropping compatibility with llvm-gcc in LLDB, but I
should point out that LLDB generally wants to be able to debug code produced by
a wide range of compilers, including old ones.
https://reviews.llvm.org/D48500
___
aprantl added a comment.
People might have a legitimate reason to debug very old code, e.g., for
backporting security fixes or similar. On the other hand one might argue that
they could just do this with a debugger from the same era.
https://reviews.llvm.org/D48500
_
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.
Comment at: tools/lldb-mi/MICmdCmdExec.cpp:137
+ auto successHandler = [this] {
+// CODETAG_DEBUG_SESSION_RUNNING_PROG_RECEIVED_SIGINT_PAUSE_PROGRAM
+if (!CMID
aprantl added a comment.
Nice. Since you are adding new lit-based tests for these commands, does that
mean that the old python tests become redundant and could we remove them? IIRC
the python tests don't set synchronous mode so they are prone to fail under
load.
https://reviews.llvm.org/D4852
aprantl added inline comments.
Comment at:
packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py:228
-# Test that --thread is optional
-self.runCmd("-exec-next-instruction --frame 0")
-self.expect("\^running")
Is this combina
aprantl added inline comments.
Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:157
+if (entry.tag() != DW_TAG_structure_type &&
+entry.tag() != DW_TAG_class_type)
+ continue;
Wait.. we accept both structs and classes in LLDB?
aprantl added a comment.
SGTM
https://reviews.llvm.org/D48646
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl added a comment.
Is the dummy target something we need to expose over the SBAPI?
I see that other places in lldb-mi query `if (sbTarget ==
rSessionInfo.GetDebugger().GetDummyTarget())`. Would that be sufficient?
https://reviews.llvm.org/D48775
___
aprantl added a comment.
This seems like a reasonable addition. Could you also add documentation for the
new API?
https://reviews.llvm.org/D48801
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
aprantl added a comment.
This is going to be really nice!
Comment at: tools/lldb-mi/MICmdCmdSymbol.cpp:24
+namespace {
+inline const CMICmnMIValueTuple
+CreateMITuplePCLine(const uint32_t addr, const uint32_t line_number) {
Please remove the `inline` keyword. L
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
LGTM with the inline keyword removed.
Comment at: tools/lldb-mi/MICmdCmdSymbol.cpp:24
+namespace {
+inline const CMICmnMIValueTuple
+CreateMITuplePCLine(const uint32_t addr
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.
Comment at: include/lldb/API/SBModule.h:136
+ ///
+ /// @param[in] sb_file_spec
+ /// A lldb::SBFileSpec object that contains source file
We typ
aprantl added a comment.
Fre
Comment at: packages/Python/lldbsuite/test/python_api/target/TestTargetAPI.py:54
+self.setTearDownCleanup(dictionary=d)
+self.find_compile_units('b.out')
+
apolyakov wrote:
> aprantl wrote:
> > shouldn't this be `sel
aprantl added a comment.
Have you seen my earlier question:
> Is the dummy target something we need to expose over the SBAPI?
> I see that other places in lldb-mi query if (sbTarget ==
> rSessionInfo.GetDebugger().GetDummyTarget()).
> Would that be sufficient?
https://reviews.llvm.org/D4877
aprantl added a comment.
Okay then let's not do this for now. It's fine to revisit this later if there
turns out to be a good use-case for it, but every SBAPI call we introduce has
to be supported indefinitely and can therefore be quite expensive to maintain.
https://reviews.llvm.org/D48775
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
That looks like a safe change to make.
https://reviews.llvm.org/D48977
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.l
aprantl added inline comments.
Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py:376
self.expect(
-"\^error,msg=\"Command 'data-info-line'\. Error: The LineEntry is
absent or has an unknown format\.\"")
+"\^error,msg=\"C
aprantl added inline comments.
Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py:22
@skipIfDarwin # pexpect is known to be unreliable on Darwin
@skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread
races
def test_ll
aprantl added inline comments.
Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/data/TestMiData.py:22
@skipIfDarwin # pexpect is known to be unreliable on Darwin
@skipIfFreeBSD # llvm.org/pr22411: Failure presumably due to known thread
races
def test_ll
aprantl added a comment.
@jingham: do you have any opinion about the right SBAPI for manipulating
settings like Alexander outlined?
https://reviews.llvm.org/D47302
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-
aprantl added a comment.
Seems good otherwise.
Comment at: source/API/SBTarget.cpp:1468
+ }
+ if (from[0] && to[0]) {
+Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
apolyakov wrote:
> I didn't find nullptr check in other API functions
aprantl added inline comments.
Comment at: source/API/SBTarget.cpp:1468
+ }
+ if (from[0] && to[0]) {
+Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
apolyakov wrote:
> aprantl wrote:
> > apolyakov wrote:
> > > I didn't find nullptr chec
aprantl added inline comments.
Comment at: source/API/SBTarget.cpp:1467
+ }
+ const ConstString csFrom(from), csTo(to);
+ if (csFrom && csTo) {
personally I would write this as:
```
if (!csFrom)
return error.SetErrorString(" path is empty");
if (!csTo)
ret
aprantl added inline comments.
Comment at: source/API/SBTarget.cpp:1467
+ }
+ const ConstString csFrom(from), csTo(to);
+ if (csFrom && csTo) {
apolyakov wrote:
> aprantl wrote:
> > personally I would write this as:
> > ```
> > if (!csFrom)
> > return error.
aprantl added inline comments.
Comment at: source/API/SBTarget.cpp:1467
+ }
+ const ConstString csFrom(from), csTo(to);
+ if (csFrom && csTo) {
apolyakov wrote:
> aprantl wrote:
> > apolyakov wrote:
> > > aprantl wrote:
> > > > personally I would write this as
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
sgtm.
https://reviews.llvm.org/D49739
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
aprantl added a comment.
Deleting dead code is always good; I'll let Jason sign this off though.
https://reviews.llvm.org/D50155
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
Hmm.. yeah, this looks more like a side-channel than a proper part of the MI
protocol. That said, this is also what the original code was doing, so we can
investigate the proper protocol sep
aprantl added inline comments.
Comment at: lit/tools/lldb-mi/interpreter/cli-support/target-list.test:5
+# We should hardcode executable name since at the moment of running of
+# lldb-mi the name must be known.
+# RUN: %cxx -o a.exe %p/inputs/main.cpp -g
That's t
aprantl added inline comments.
Comment at: lit/tools/lldb-mi/interpreter/cli-support/breakpoint-set.test:4
+#
+# RUN: %cxx -o %t %p/inputs/main.cpp -g
+# RUN: %lldbmi %t < %s | FileCheck %s
stella.stamenova wrote:
> apolyakov wrote:
> > stella.stamenova wrote:
>
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
Thanks, let's just give it try!
Repository:
rLLDB LLDB
https://reviews.llvm.org/D50722
___
lldb-commits mailing list
lldb-commits@lists.llvm
aprantl created this revision.
aprantl added a reviewer: vsk.
Automatically set path to sanitizer runtime when running tests on macOS.
rdar://problem/42984739
https://reviews.llvm.org/D50997
Files:
lit/Suite/lit.cfg
lit/Suite/lit.site.cfg.in
Index: lit/Suite/lit.site.cfg.in
=
aprantl added a comment.
Thanks, that was quite helpful!
https://reviews.llvm.org/D50997
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
aprantl added inline comments.
Comment at: lldb/include/lldb/Symbol/Function.h:331
+ /// \ref resolved.
+ union {
+const char *mangled_name;
`llvm::PointerUnion` ?
https://reviews.llvm.org/D50478
___
lldb-com
101 - 200 of 1517 matches
Mail list logo
