ki.stfu accepted this revision.
ki.stfu added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: tools/lldb-mi/MICmdCmdGdbShow.cpp:350
lldb::SBDebugger &rDbgr = m_rLLDBDebugSessionInfo.GetDebugger();
- m_strValue =
lldb::SBDebugger::GetInternalVar
Yes, a centralized list would be nice
On Fri, Feb 3, 2017 at 5:27 PM Jim Ingham wrote:
> That doesn't help me if I don't know that Format takes formatters and that
> various lldb objects take formatters and here are all the formatters we
> have. And it would be kinder to have the list of availab
That doesn't help me if I don't know that Format takes formatters and that
various lldb objects take formatters and here are all the formatters we have.
And it would be kinder to have the list of available formatters centralized
somewhere so folks don't have to go scouring through the code to s
Right, one of the suggestions I mentioned first was to have comments in the
headers that mention the ability to format the class, or to have the
formatters defined in the same header file as the object. So you look at
the definition of FileSpec, and you learn about the formatter for FileSpec.
On
zturner added a comment.
FWIW, long term I would like to get rid of ALL printf style formatting. So
right now, yes, they are only used in a few places. But it would be nice if it
were the only type of formatting used anywhere. At which point I think most of
your questions would be resolved o
Not to be snarky, but that's why we put comments in headers...
Jim
> On Feb 3, 2017, at 4:47 PM, Zachary Turner via Phabricator
> wrote:
>
> zturner added a comment.
>
> I guess the same way you would know how to use any part of a library or API.
> The first time you've ever used an API, yo
zturner added a comment.
I guess the same way you would know how to use any part of a library or API.
The first time you've ever used an API, you don't know how it works, so you
don't know it's capabilities. So you fiddle around, read the source code,
trudge through some compiler errors, then
Author: kamil
Date: Fri Feb 3 18:20:24 2017
New Revision: 294071
URL: http://llvm.org/viewvc/llvm-project?rev=294071&view=rev
Log:
Install six.py conditionally
Summary:
The current version of LLDB installs six.py into global python library
directory. This approach produces conflicts downstream
jingham added a comment.
That's kind of a Meno response... How would I know to look for them if I don't
know they are there. These format strings are only used in a couple of places.
Those calls should tell me that I CAN use these format strings and where/how
to find them.
https://reviews.
labath updated this revision to Diff 87046.
labath added a comment.
Herald added a subscriber: mgorny.
Add ConstString formatter and use it.
https://reviews.llvm.org/D29510
Files:
include/lldb/Core/Log.h
include/lldb/Core/Logging.h
include/lldb/Utility/ConstString.h
source/API/SBBroadca
zturner added a comment.
In https://reviews.llvm.org/D29510#666443, @jingham wrote:
> This is sort of a side question, but Pavel's comment brought it up. If I
> were new to this stuff, and wanted to know which entities had formatters and
> which didn't, how would I find that out easily? There
jingham accepted this revision.
jingham added a comment.
Oh, yeah, and I should remember to check okay...
https://reviews.llvm.org/D29510
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-c
jingham added a comment.
This is sort of a side question, but Pavel's comment brought it up. If I were
new to this stuff, and wanted to know which entities had formatters and which
didn't, how would I find that out easily? There are a couple of Format calls
that use it, maybe they should have
clayborg added a comment.
Much better, just clean up the unnecessary includes and we are good to go.
https://reviews.llvm.org/D29514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commit
clayborg added inline comments.
Comment at: source/Target/SectionLoadList.cpp:70
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- L
labath added a comment.
Comment at: source/Target/SectionLoadList.cpp:70
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LIBLLD
Compiles on Windows, I will watch the bots to see if anything breaks on
Linux/ OSX.
On Fri, Feb 3, 2017 at 3:03 PM Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:
> labath accepted this revision.
> labath added a comment.
> This revision is now accepted and ready to land.
>
> look
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
looks good, just make sure it compiles.
Comment at: lldb/include/lldb/Core/Log.h:18
#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/Error.h"
#include "lldb/ll
jingham added a comment.
Thanks for taking the trouble to clean that up, that's lovely!
https://reviews.llvm.org/D29510
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner updated this revision to Diff 87036.
https://reviews.llvm.org/D29514
Files:
lldb/include/lldb/Core/Log.h
lldb/include/lldb/Utility/Error.h
lldb/source/Core/Communication.cpp
lldb/source/Host/common/Host.cpp
lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
lldb/sour
zturner added inline comments.
Comment at: source/Target/SectionLoadList.cpp:70
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LI
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Looks good.
https://reviews.llvm.org/D29510
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
labath added a reviewer: clayborg.
labath added a comment.
Adding greg in case he has some thoughts on this as well.
https://reviews.llvm.org/D29510
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
zturner added a comment.
Fair enough, I can do that.
https://reviews.llvm.org/D29514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
labath added a comment.
In https://reviews.llvm.org/D29514#666341, @zturner wrote:
> If we do what you suggest, then we are relying on the `lldb::Error` format
> provider, which does not have the ability to accept this kind of "contextual
> reason" string (i.e. the string "While reading the %d'
zturner added a comment.
In https://reviews.llvm.org/D29514#666285, @labath wrote:
> I'm not opposed to this patch, if people really want it, but I don't really
> see the value of this macro.
> Why couldn't I write
> `LLDB_LOG(log, "foo: {0}", error);`
> instead of
> `LLDB_LOG_ERROR(log, err
Sorry for the noise, I must have gotten the numbers wrong somehow.
On Fri, Feb 3, 2017 at 1:59 PM, Geoff Berry wrote:
> Hans,
>
> Sorry for the confusion, but this isn't the change I was referring to. I
> included the phabricator review numbers in my original message, not svn
> commit numbers.
>
clayborg added a comment.
So I agree with Pavel. Let us know what you think
https://reviews.llvm.org/D29514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
clayborg added a comment.
I realize the functionality would add a "error: " prefix if it wasn't there,
but it seems like we could add this as a formatting option of the
lldb_private::Error class? Then we can just switch the logging code to put the
error in as a log item and add the extra flag t
labath added a comment.
As for the modules part, I assumed the Log class will end up in the lowest
layer also. I intended to move it there after I am done with it, but we can do
it sooner if that is stopping you from doing anything. ( I do agree that it
makes sense to reverse the dependency tho
Yes, this is a cleanup commit. I don't see a reason for cherry-picking it.
Geoff, why do you need this cherry-picked?
cheers,
pl
On 3 February 2017 at 13:54, Hans Wennborg wrote:
> Geoff asked for this to be merged to 4.0. It looks like a nice change,
> but I'm a little hesitant since it doesn'
Geoff asked for this to be merged to 4.0. It looks like a nice change,
but I'm a little hesitant since it doesn't look like it's fixing a
regression. Pavel, what do you think?
On Fri, Feb 3, 2017 at 9:42 AM, Pavel Labath via lldb-commits
wrote:
> Author: labath
> Date: Fri Feb 3 11:42:04 2017
>
labath added a comment.
I'm not opposed to this patch, if people really want it, but I don't really see
the value of this macro.
Why couldn't I write
`LLDB_LOG(log, "foo: {0}", error);`
instead of
`LLDB_LOG_ERROR(log, error, "foo");`
Am I missing something?
https://reviews.llvm.org/D29514
__
zturner created this revision.
Instead of having the Error class (which is in Utility) know about logging
(which is in Core), it seems more appropriate to put all logging code together,
and teach Log about errors rather than teaching Error about logs. This patch
does so.
https://reviews.llvm
labath created this revision.
Per discussion in https://reviews.llvm.org/D28616, having two ways two request
logging (log
enable lldb XXX verbose && log enable -v lldb XXX) is confusing. This
removes the first option and standardizes all code to use the second
one.
I've added a LLDB_LOGV macro a
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
looks good, thank you.
Repository:
rL LLVM
https://reviews.llvm.org/D29405
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://li
Author: labath
Date: Fri Feb 3 14:26:57 2017
New Revision: 294036
URL: http://llvm.org/viewvc/llvm-project?rev=294036&view=rev
Log:
Add a missing break statement
Modified:
lldb/trunk/source/Commands/CommandObjectLog.cpp
Modified: lldb/trunk/source/Commands/CommandObjectLog.cpp
URL:
http://
krytarowski updated this revision to Diff 87000.
krytarowski edited the summary of this revision.
krytarowski added a comment.
Pass option to `--useSystemSix` to `finishSwigWrapperClasses.py`
Repository:
rL LLVM
https://reviews.llvm.org/D29405
Files:
CMakeLists.txt
cmake/modules/LLDBConf
Author: labath
Date: Fri Feb 3 12:50:41 2017
New Revision: 294023
URL: http://llvm.org/viewvc/llvm-project?rev=294023&view=rev
Log:
Use LLDB_LOG in NativeRegisterContextLinux*** files
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
lldb/trunk/source/Plugi
Author: labath
Date: Fri Feb 3 12:50:45 2017
New Revision: 294024
URL: http://llvm.org/viewvc/llvm-project?rev=294024&view=rev
Log:
Fix incorrect logging in ThreadPlan::ShouldReportStop
it used printf with formatv style specifications. Also, switch to
LLDB_LOG.
Modified:
lldb/trunk/source/T
Author: labath
Date: Fri Feb 3 11:42:04 2017
New Revision: 294019
URL: http://llvm.org/viewvc/llvm-project?rev=294019&view=rev
Log:
Push down more common code into PlatformPOSIX
Summary:
- GetFileWithUUID: All platforms except PlatformDarwin had this.
However, I see no reason why this code would
This revision was automatically updated to reflect the committed changes.
Closed by commit rL294019: Push down more common code into PlatformPOSIX
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D29496?vs=86975&id=86982#toc
Repository:
rL LLVM
https://reviews.llvm.or
clayborg added a comment.
indeed very nice
https://reviews.llvm.org/D29496
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
krytarowski accepted this revision.
krytarowski added a comment.
Thank you for this patch.
https://reviews.llvm.org/D29496
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
labath created this revision.
- GetFileWithUUID: All platforms except PlatformDarwin had this.
However, I see no reason why this code would not apply there as well.
- GetProcessInfo, FindProcesses: The implementation was the same in all classes.
- GetFullNameForDylib: This code should apply to
45 matches
Mail list logo
