[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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.

[Lldb-commits] [PATCH] D29405: Install six.py conditionally

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

[Lldb-commits] [lldb] r294036 - Add a missing break statement

Author: labath Date: Fri Feb 3 14:26:57 2017 New Revision: 294036 URL: http://llvm.org/viewvc/llvm-project?rev=294036=rev Log: Add a missing break statement Modified: lldb/trunk/source/Commands/CommandObjectLog.cpp Modified: lldb/trunk/source/Commands/CommandObjectLog.cpp URL:

[Lldb-commits] [PATCH] D29405: Install six.py conditionally

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

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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

[Lldb-commits] [PATCH] D29496: Push down more common code into PlatformPOSIX

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=86982#toc Repository: rL LLVM

[Lldb-commits] [lldb] r294019 - Push down more common code into PlatformPOSIX

Author: labath Date: Fri Feb 3 11:42:04 2017 New Revision: 294019 URL: http://llvm.org/viewvc/llvm-project?rev=294019=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 not

[Lldb-commits] [PATCH] D29496: Push down more common code into PlatformPOSIX

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

[Lldb-commits] [lldb] r294023 - Use LLDB_LOG in NativeRegisterContextLinux*** files

Author: labath Date: Fri Feb 3 12:50:41 2017 New Revision: 294023 URL: http://llvm.org/viewvc/llvm-project?rev=294023=rev Log: Use LLDB_LOG in NativeRegisterContextLinux*** files Modified: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp

[Lldb-commits] [lldb] r294024 - Fix incorrect logging in ThreadPlan::ShouldReportStop

Author: labath Date: Fri Feb 3 12:50:45 2017 New Revision: 294024 URL: http://llvm.org/viewvc/llvm-project?rev=294024=rev Log: Fix incorrect logging in ThreadPlan::ShouldReportStop it used printf with formatv style specifications. Also, switch to LLDB_LOG. Modified:

[Lldb-commits] [PATCH] D29496: Push down more common code into PlatformPOSIX

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

[Lldb-commits] [PATCH] D29496: Push down more common code into PlatformPOSIX

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

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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.

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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

Re: [Lldb-commits] [lldb] r294019 - Push down more common code into PlatformPOSIX

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 >

Re: [Lldb-commits] [lldb] r294019 - Push down more common code into PlatformPOSIX

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

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

clayborg added inline comments. Comment at: source/Target/SectionLoadList.cpp:70 bool warn_multiple) { - Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER | -

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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

[Lldb-commits] [lldb] r294071 - Install six.py conditionally

Author: kamil Date: Fri Feb 3 18:20:24 2017 New Revision: 294071 URL: http://llvm.org/viewvc/llvm-project?rev=294071=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 with

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

labath added a comment. Comment at: source/Target/SectionLoadList.cpp:70 bool warn_multiple) { - Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER | -

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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

[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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

Re: [Lldb-commits] [lldb] r294019 - Push down more common code into PlatformPOSIX

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

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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,

[Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR

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-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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?

Re: [Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category

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