apolyakov updated this revision to Diff 150913.
apolyakov added a comment.
Some readability improvements(more comments). I think that we should keep
`StepOver` signature as
void StepOver(SBError , lldb::RunMode stop_other_threads);
since it seems that in SBThread methods SBError paramater is
Author: labath
Date: Tue Jun 12 05:43:55 2018
New Revision: 334496
URL: http://llvm.org/viewvc/llvm-project?rev=334496=rev
Log:
lit/SymbolFile/DWARF: Simplify test RUN lines
Use -mllvm compiler argument to enable DWARF v5 accelerator tables
instead of piping the IR through llc.
Modified:
labath added a comment.
I like the idea of centralizing the framework code as much as possible.
However, I am not sure how the dependency management fits into this. More
details in inline comments.
Comment at: CMakeLists.txt:46
+ if (NOT APPLE)
+message(FATAL_ERROR
On Mon, 11 Jun 2018 at 22:00, Leonard Mosescu via lldb-commits
wrote:
>>
>> remove space before (
>
>
> I'd be happy to make the change, but looking at the rest of the file it seems
> that almost everything uses the opposite convention: Foo (...).
>
I guess that's because the .i files escaped
> On Jun 12, 2018, at 9:08 AM, Adrian Prantl via Phabricator
> wrote:
>
> aprantl added a comment.
>
> Hmm.. @lemo 's reasoning
What I was going to say here: Just from looking at the API documentation, it
looks like default arguments are mapped into Python by SWIG, so there doesn't
seem
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:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334557: Disable warnings for the generated LLDB wrapper
source (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: teemperor
Date: Tue Jun 12 15:51:20 2018
New Revision: 334557
URL: http://llvm.org/viewvc/llvm-project?rev=334557=rev
Log:
Disable warnings for the generated LLDB wrapper source
Summary:
This source files emits all kind of compiler warnings on different platforms.
As the source code
in
compnerd added inline comments.
Comment at: CMakeLists.txt:145
+ add_dependencies(lldb-suite lldb-framework)
+elseif()
+ if (LLDB_CAN_USE_LLDB_SERVER)
Shouldn't this be `else()`?
Comment at: CMakeLists.txt:176
+if (LLDB_BUILD_FRAMEWORK)
clayborg resigned from this revision.
clayborg added a comment.
I'll defer to the cmake experts. I hope we can produce a LLDB.framework that is
identical to the Xcode produced version, or at least as close.
https://reviews.llvm.org/D48060
___
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
teemperor updated this revision to Diff 151077.
teemperor added a comment.
- The regex that removes the modules cache now actually removes the whole flag
+ path.
- Fixed more typos.
https://reviews.llvm.org/D47929
Files:
include/lldb/module.modulemap
source/Host/CMakeLists.txt
xiaobai updated this revision to Diff 151067.
xiaobai added a comment.
Minor fixups pointed out by compnerd
https://reviews.llvm.org/D48060
Files:
CMakeLists.txt
cmake/modules/LLDBFramework.cmake
source/API/CMakeLists.txt
tools/driver/CMakeLists.txt
Index: tools/driver/CMakeLists.txt
xiaobai added inline comments.
Comment at: CMakeLists.txt:145
+ add_dependencies(lldb-suite lldb-framework)
+elseif()
+ if (LLDB_CAN_USE_LLDB_SERVER)
compnerd wrote:
> Shouldn't this be `else()`?
Yup
Comment at: CMakeLists.txt:176
+if
JDevlieghere created this revision.
JDevlieghere added reviewers: aprantl, jingham, jasonmolenda.
This patch adds a data formatter for NSDecimalNumber. The latter is a
Foundation object used for representing and performing arithmetic on base-10
numbers that bridges to Decimal.
Repository:
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
I didn't check whether the representation was correct (although it looks lo).
The structure of the patch looks fine to me, thanks for adding the comments :)
Repository:
rL LLVM
jingham added a comment.
I agree with Leonard, for the StepOver overload that returns errors, just make
the mode parameter required. That will reduce confusion.
https://reviews.llvm.org/D47991
___
lldb-commits mailing list
apolyakov updated this revision to Diff 150996.
apolyakov added a comment.
Added `StepOver` overload without optional parameters.
https://reviews.llvm.org/D47991
Files:
include/lldb/API/SBThread.h
scripts/interface/SBThread.i
source/API/SBThread.cpp
Index: source/API/SBThread.cpp
apolyakov added inline comments.
Comment at: source/API/SBThread.cpp:1136
bool result = false;
if (exe_ctx.HasThreadScope()) {
Process::StopLocker stop_locker;
@aprantl do you think that we should use early exit here? There is printing to
log at the
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.
Comment at: source/Utility/FileSpec.cpp:244-246
+ // Only update style if explicitly requested.
+ if (style)
+m_style = (*style == Style::native) ? GetNativeStyle() : *style;
JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.
Comment at: source/Utility/FileSpec.cpp:244-246
+ // Only update style if explicitly requested.
+ if (style)
+m_style = (*style == Style::native) ? GetNativeStyle() : *style;
apolyakov updated this revision to Diff 151000.
apolyakov added a comment.
Minor update: moved `error.SetErrorString` to the top of `if` statement to
increase readability.
https://reviews.llvm.org/D47991
Files:
include/lldb/API/SBThread.h
scripts/interface/SBThread.i
teemperor updated this revision to Diff 151004.
teemperor retitled this revision from "Add modulemap to lldb include directory"
to "Add modules support for lldb headers in include/".
teemperor edited the summary of this revision.
teemperor added a comment.
This revision is now accepted and ready
teemperor added inline comments.
Comment at: include/lldb/module.modulemap:66
+// This big module is necessary to work around the cyclic dependencies
+// between its submodules.
+module lldb {
teemperor wrote:
> bruno wrote:
> > Will this trick be enough for
labath accepted this revision.
labath added inline comments.
Comment at: source/Utility/FileSpec.cpp:244-246
+ // Only update style if explicitly requested.
+ if (style)
+m_style = (*style == Style::native) ? GetNativeStyle() : *style;
JDevlieghere wrote:
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
Unfortunate, but this seems reasonable. lg.
https://reviews.llvm.org/D48096
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
xiaobai updated this revision to Diff 151049.
xiaobai added a comment.
Minor change I forgot to make
https://reviews.llvm.org/D48060
Files:
CMakeLists.txt
cmake/modules/LLDBFramework.cmake
source/API/CMakeLists.txt
tools/driver/CMakeLists.txt
Index: tools/driver/CMakeLists.txt
xiaobai updated this revision to Diff 151048.
xiaobai added a comment.
Updated based on feedback. I would like to get more feedback before I'm
comfortable changing it further and/or committing this.
https://reviews.llvm.org/D48060
Files:
CMakeLists.txt
cmake/modules/LLDBFramework.cmake
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
teemperor updated this revision to Diff 151023.
teemperor added a comment.
- Obj-C -> Obj-C++
https://reviews.llvm.org/D47929
Files:
include/lldb/module.modulemap
source/Host/CMakeLists.txt
source/Host/common/Terminal.cpp
source/Host/macosx/Host.mm
teemperor abandoned this revision.
teemperor added a comment.
It seems people are also in favor of completely disabling the warnings, so that
patch will replace this one.
https://reviews.llvm.org/D47411
___
lldb-commits mailing list
teemperor edited subscribers, added: lldb-commits; removed: cfe-commits.
teemperor added a comment.
- Fixed mailing list subscriber.
https://reviews.llvm.org/D47996
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
teemperor created this revision.
teemperor added a reviewer: aprantl.
Herald added a subscriber: mgorny.
This source files emits all kind of compiler warnings on different platforms.
As the source code
in the file is generated and we therefore can't actually fix the warnings, we
might as well
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
xiaobai added inline comments.
Comment at: cmake/modules/LLDBFramework.cmake:45
+
+ add_dependencies(lldb-framework liblldb lldb-argdumper lldb-server
lldb-framework-headers)
+ add_dependencies(finish_swig lldb-framework)
labath wrote:
> xiaobai wrote:
> >
apolyakov added inline comments.
Comment at: source/API/SBThread.cpp:816
+error.SetErrorString("passed a frame from another thread");
+return;
}
I found this place and added `return`.
https://reviews.llvm.org/D47991
teemperor updated this revision to Diff 151025.
teemperor marked an inline comment as done.
teemperor added a comment.
- Fixed a typo.
https://reviews.llvm.org/D47929
Files:
include/lldb/module.modulemap
source/Host/CMakeLists.txt
source/Host/common/Terminal.cpp
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
labath added inline comments.
Comment at: cmake/modules/LLDBFramework.cmake:45
+
+ add_dependencies(lldb-framework liblldb lldb-argdumper lldb-server
lldb-framework-headers)
+ add_dependencies(finish_swig lldb-framework)
xiaobai wrote:
> labath wrote:
> >
Author: teemperor
Date: Tue Jun 12 14:22:52 2018
New Revision: 334549
URL: http://llvm.org/viewvc/llvm-project?rev=334549=rev
Log:
Added modulemap for lldb-mi
Summary: This patch allows building a C++ module for the lldb-mi headers.
Reviewers: bruno, aprantl
Reviewed By: aprantl
Subscribers:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL334549: Added modulemap for lldb-mi (authored by teemperor,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47996?vs=150663=151033#toc
davide accepted this revision.
davide added a comment.
LGTM with a nit.
Comment at: source/Utility/FileSpec.cpp:18-21
+#include "llvm/ADT/Triple.h" // for Triple
+#include "llvm/ADT/Twine.h" // for Twine
+#include "llvm/Support/ErrorOr.h" // for ErrorOr
#include
xiaobai added inline comments.
Comment at: cmake/modules/LLDBFramework.cmake:1
+if (LLDB_BUILD_FRAMEWORK)
+ add_custom_target(lldb-framework)
labath wrote:
> Maybe use early exit here?
Sounds good
Comment at:
labath added inline comments.
Comment at: source/Utility/FileSpec.cpp:244-246
+ // Only update style if explicitly requested.
+ if (style)
+m_style = (*style == Style::native) ? GetNativeStyle() : *style;
I don't really have a clear opinion on whether the
Author: labath
Date: Tue Jun 12 06:11:25 2018
New Revision: 334500
URL: http://llvm.org/viewvc/llvm-project?rev=334500=rev
Log:
DWARFDebugNames: Implement last GetGlobalVariables overload
This function implements the search for all global variables within a
given compilation unit.
Modified:
Author: zturner
Date: Tue Jun 12 10:43:52 2018
New Revision: 334518
URL: http://llvm.org/viewvc/llvm-project?rev=334518=rev
Log:
Refactor ExecuteAndWait to take StringRefs.
This simplifies some code which had StringRefs to begin with, and
makes other code more complicated which had const char*
apolyakov updated this revision to Diff 150983.
apolyakov added a comment.
Added early exits.
https://reviews.llvm.org/D47991
Files:
include/lldb/API/SBThread.h
scripts/interface/SBThread.i
source/API/SBThread.cpp
Index: source/API/SBThread.cpp
Author: labath
Date: Tue Jun 12 05:57:36 2018
New Revision: 334498
URL: http://llvm.org/viewvc/llvm-project?rev=334498=rev
Log:
lldb-test symbols: Add -file argument and the ability to dump global variables
in a file
The motivation for this is to be able to Dwarf index ability to look up
teemperor planned changes to this revision.
teemperor added a comment.
It seems the compilation fails on OS X with this modulemap (or any modulemap).
The reason seems to be that on OS X we compile Obj-C++ as part of lldb which
assumes that Clang/LLVM is valid Obj-C++. However, the Obj-C parsing
Author: labath
Date: Tue Jun 12 06:26:43 2018
New Revision: 334501
URL: http://llvm.org/viewvc/llvm-project?rev=334501=rev
Log:
Fix build error introduced in r334498
Some gcc versions (circa 4.9) do not accept initializing Expected
objects containing a reference to a function from a function.
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, zturner, clayborg, davide.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.
With the recent changes in FileSpec to use LLVM's path style, it is
possible to delegate a bunch of common path
Author: labath
Date: Tue Jun 12 09:50:01 2018
New Revision: 334516
URL: http://llvm.org/viewvc/llvm-project?rev=334516=rev
Log:
DebugNamesDWARFIndex: Implement DWARFDeclContext variant of GetTypes method
This method is used to find complete definitions of a type when one
parses a compile unit
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
Looks like good cleanup to me, thanks!
Comment at:
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:2593-2596
+if
53 matches
Mail list logo
