[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 all non-darwin
platforms. I've kept the PlatformDarwin override as the situation is
different there.
https://reviews.llvm.org/D29496
Files:
source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
source/Plugins/Platform/Kalimba/PlatformKalimba.h
source/Plugins/Platform/Linux/PlatformLinux.cpp
source/Plugins/Platform/Linux/PlatformLinux.h
source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
source/Plugins/Platform/MacOSX/PlatformDarwin.h
source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
source/Plugins/Platform/NetBSD/PlatformNetBSD.h
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.h
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.h
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -105,6 +105,14 @@
lldb::ModuleSP &module_sp,
const lldb_private::FileSpecList *module_search_paths_ptr) override;
+ lldb_private::Error GetFileWithUUID(const lldb_private::FileSpec &platform_file, const lldb_private::UUID *uuid,
+lldb_private::FileSpec &local_file) override;
+
+ bool GetProcessInfo(lldb::pid_t pid, lldb_private::ProcessInstanceInfo &proc_info) override;
+
+ uint32_t FindProcesses(const lldb_private::ProcessInstanceInfoMatch &match_info,
+ lldb_private::ProcessInstanceInfoList &process_infos) override;
+
lldb_private::Error MakeDirectory(const lldb_private::FileSpec &file_spec,
uint32_t mode) override;
@@ -169,6 +177,8 @@
size_t ConnectToWaitingProcesses(lldb_private::Debugger &debugger,
lldb_private::Error &error) override;
+ lldb_private::ConstString GetFullNameForDylib(lldb_private::ConstString basename) override;
+
protected:
std::unique_ptr
m_option_group_platform_rsync;
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -250,6 +250,38 @@
return error;
}
+Error PlatformPOSIX::GetFileWithUUID(const FileSpec &platform_file,
+ const UUID *uuid_ptr,
+ FileSpec &local_file) {
+ if (IsRemote() && m_remote_platform_sp)
+ return m_remote_platform_sp->GetFileWithUUID(platform_file, uuid_ptr,
+ local_file);
+
+ // Default to the local case
+ local_file = platform_file;
+ return Error();
+}
+
+bool PlatformPOSIX::GetProcessInfo(lldb::pid_t pid,
+ ProcessInstanceInfo &process_info) {
+ if (IsHost())
+return Platform::GetProcessInfo(pid, process_info);
+ if (m_remote_platform_sp)
+return m_remote_platform_sp->GetProcessInfo(pid, process_info);
+ return false;
+}
+
+uint32_t
+PlatformPOSIX::FindProcesses(const ProcessInstanceInfoMatch &match_info,
+ ProcessInstanceInfoList &process_infos) {
+ if (IsHost())
+return Platform::FindProcesses(match_info, process_infos);
+ if (m_remote_platform_sp)
+return
+ m_remote_platform_sp->FindProcesses(match_info, process_infos);
+ return 0;
+}
+
Error PlatformPOSIX::MakeDirectory(const FileSpec &file_spec,
uint32_t file_permissions) {
if (m_remote_platform_sp)
@@ -1003,3 +1035,12 @@
return m_remote_platform_sp->ConnectToWaitingProcesses(debugger, error);
return Platform::ConnectToWaitingProcesses(debugger, error);
}
+
+ConstString PlatformPOSIX::GetFullNameForDylib(ConstString basename) {
+ if (basename.IsEmpty())
+return basename;
+
+ StreamString stream;
+ stream.Printf("lib%s.so", basename.GetCString());
+ return ConstString(stream.GetString());
+}
Index: source/Plugins/Platform/NetBSD/PlatformNetBSD.h
===
--- source/Plugins/Platform/NetBSD/PlatformNetBSD.h
+++ source/Plugins/Platform/NetBSD/PlatformNetBSD.h
@@ -81,23 +81,14 @@
const char *GetGroupName(uint32_t gid) override;
- bool GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &proc_info) override;
-
- uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
- ProcessInstanceInfoList &process_infos) override;
-
Error LaunchProcess(ProcessLaunchInfo &launch_info) override;
lldb::ProcessSP Attach(ProcessAttachInfo &at
[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] [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
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.org/D29496
Files:
lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -51,16 +51,9 @@
lldb_private::Target &target,
lldb_private::BreakpointSite *bp_site) override;
- bool GetProcessInfo(lldb::pid_t pid,
- lldb_private::ProcessInstanceInfo &proc_info) override;
-
lldb::BreakpointSP
SetThreadCreationBreakpoint(lldb_private::Target &target) override;
- uint32_t
- FindProcesses(const lldb_private::ProcessInstanceInfoMatch &match_info,
-lldb_private::ProcessInstanceInfoList &process_infos) override;
-
bool ModuleIsExcludedForUnconstrainedSearches(
lldb_private::Target &target, const lldb::ModuleSP &module_sp) override;
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -489,34 +489,6 @@
return 0;
}
-bool PlatformDarwin::GetProcessInfo(lldb::pid_t pid,
-ProcessInstanceInfo &process_info) {
- bool success = false;
- if (IsHost()) {
-success = Platform::GetProcessInfo(pid, process_info);
- } else {
-if (m_remote_platform_sp)
- success = m_remote_platform_sp->GetProcessInfo(pid, process_info);
- }
- return success;
-}
-
-uint32_t
-PlatformDarwin::FindProcesses(const ProcessInstanceInfoMatch &match_info,
- ProcessInstanceInfoList &process_infos) {
- uint32_t match_count = 0;
- if (IsHost()) {
-// Let the base class figure out the host details
-match_count = Platform::FindProcesses(match_info, process_infos);
- } else {
-// If we are remote, we can only return results if we are connected
-if (m_remote_platform_sp)
- match_count =
- m_remote_platform_sp->FindProcesses(match_info, process_infos);
- }
- return match_count;
-}
-
bool PlatformDarwin::ModuleIsExcludedForUnconstrainedSearches(
lldb_private::Target &target, const lldb::ModuleSP &module_sp) {
if (!module_sp)
Index: lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
===
--- lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
+++ lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
@@ -84,23 +84,14 @@
const char *GetGroupName(uint32_t gid) override;
- bool GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &proc_info) override;
-
- uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
- ProcessInstanceInfoList &process_infos) override;
-
Error LaunchProcess(ProcessLaunchInfo &launch_info) override;
lldb::ProcessSP Attach(ProcessAttachInfo &attach_info, Debugger &debugger,
Target *target, Error &error) override;
// FreeBSD processes can not be launched by spawning and attaching.
bool CanDebugProcess() override { return false; }
- // Only on PlatformMacOSX:
- Error GetFileWithUUID(const FileSpec &platform_file, const UUID *uuid,
-FileSpec &local_file) override;
-
Error GetSharedModule(const ModuleSpec &module_spec, Process *process,
lldb::ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr,
Index: lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
===
--- lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
+++ lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
@@ -134,21 +134,6 @@
}
}
-// From PlatformMacOSX only
-Error PlatformFreeBSD::GetFi
[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&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 not apply there as well.
- GetProcessInfo, FindProcesses: The implementation was the same in all classes.
- GetFullNameForDylib: This code should apply to all non-darwin
platforms. I've kept the PlatformDarwin override as the situation is
different there.
Reviewers: clayborg, krytarowski, emaste
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D29496
Modified:
lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
Modified: lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp?rev=294019&r1=294018&r2=294019&view=diff
==
--- lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp Fri Feb 3
11:42:04 2017
@@ -134,21 +134,6 @@ Error PlatformFreeBSD::RunShellCommand(c
}
}
-// From PlatformMacOSX only
-Error PlatformFreeBSD::GetFileWithUUID(const FileSpec &platform_file,
- const UUID *uuid_ptr,
- FileSpec &local_file) {
- if (IsRemote()) {
-if (m_remote_platform_sp)
- return m_remote_platform_sp->GetFileWithUUID(platform_file, uuid_ptr,
- local_file);
- }
-
- // Default to the local case
- local_file = platform_file;
- return Error();
-}
-
//--
/// Default Constructor
//--
@@ -256,33 +241,6 @@ Error PlatformFreeBSD::DisconnectRemote(
return error;
}
-bool PlatformFreeBSD::GetProcessInfo(lldb::pid_t pid,
- ProcessInstanceInfo &process_info) {
- bool success = false;
- if (IsHost()) {
-success = Platform::GetProcessInfo(pid, process_info);
- } else if (m_remote_platform_sp) {
-success = m_remote_platform_sp->GetProcessInfo(pid, process_info);
- }
- return success;
-}
-
-uint32_t
-PlatformFreeBSD::FindProcesses(const ProcessInstanceInfoMatch &match_info,
- ProcessInstanceInfoList &process_infos) {
- uint32_t match_count = 0;
- if (IsHost()) {
-// Let the base class figure out the host details
-match_count = Platform::FindProcesses(match_info, process_infos);
- } else {
-// If we are remote, we can only return results if we are connected
-if (m_remote_platform_sp)
- match_count =
- m_remote_platform_sp->FindProcesses(match_info, process_infos);
- }
- return match_count;
-}
-
const char *PlatformFreeBSD::GetUserName(uint32_t uid) {
// Check the cache in Platform in case we have already looked this uid up
const char *user_name = Platform::GetUserName(uid);
Modified: lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h?rev=294019&r1=294018&r2=294019&view=diff
==
--- lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h (original)
+++ lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h Fri Feb 3
11:42:04 2017
@@ -84,11 +84,6 @@ public:
const char *GetGroupName(uint32_t gid) override;
- bool GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &proc_info)
override;
-
- uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
- ProcessInstanceInfoList &process_infos) override;
-
Error LaunchProcess(ProcessLaunchInfo &launch_info) override;
lldb::ProcessSP Attach(ProcessAttachInfo &attach_info, Debugger &debugger,
@@ -97,10 +92,6 @@ public:
// FreeBSD processes can not be launched by spawning and attaching.
bool CanDebugProcess() override { return false; }
- // Only on
[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&view=rev
Log:
Use LLDB_LOG in NativeRegisterContextLinux*** files
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp?rev=294023&r1=294022&r2=294023&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp Fri
Feb 3 12:50:41 2017
@@ -171,10 +171,7 @@ Error NativeRegisterContextLinux::DoRead
// First cast to an unsigned of the same size to avoid sign extension.
value.SetUInt(static_cast(data), size);
- if (log)
-log->Printf("NativeRegisterContextLinux::%s() reg %s: 0x%lx", __FUNCTION__,
-reg_name, data);
-
+ LLDB_LOG(log, "{0}: {1:x}", reg_name, data);
return error;
}
@@ -183,10 +180,7 @@ Error NativeRegisterContextLinux::DoWrit
Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_REGISTERS));
void *buf = reinterpret_cast(value.GetAsUInt64());
-
- if (log)
-log->Printf("NativeRegisterContextLinux::%s() reg %s: %p", __FUNCTION__,
-reg_name, buf);
+ LLDB_LOG(log, "{0}: {1}", reg_name, buf);
return NativeProcessLinux::PtraceWrapper(
PTRACE_POKEUSER, m_thread.GetID(), reinterpret_cast(offset),
buf);
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp?rev=294023&r1=294022&r2=294023&view=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
(original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp
Fri Feb 3 12:50:41 2017
@@ -17,6 +17,7 @@
#include "lldb/Utility/Error.h"
#include "Plugins/Process/Linux/Procfs.h"
+#include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
#include "Plugins/Process/Utility/RegisterContextLinux_arm.h"
#include
@@ -356,15 +357,11 @@ bool NativeRegisterContextLinux_arm::IsF
uint32_t
NativeRegisterContextLinux_arm::SetHardwareBreakpoint(lldb::addr_t addr,
size_t size) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
-
- if (log)
-log->Printf("NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
-
- Error error;
+ Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+ LLDB_LOG(log, "addr: {0:x}, size: {1:x}", addr, size);
// Read hardware breakpoint and watchpoint information.
- error = ReadHardwareDebugInfo();
+ Error error = ReadHardwareDebugInfo();
if (error.Fail())
return LLDB_INVALID_INDEX32;
@@ -429,10 +426,8 @@ NativeRegisterContextLinux_arm::SetHardw
}
bool NativeRegisterContextLinux_arm::ClearHardwareBreakpoint(uint32_t hw_idx) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
-
- if (log)
-log->Printf("NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
+ Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+ LLDB_LOG(log, "hw_idx: {0}", hw_idx);
Error error;
@@ -477,33 +472,26 @@ bool NativeRegisterContextLinux_arm::Cle
}
uint32_t NativeRegisterContextLinux_arm::NumSupportedHardwareWatchpoints() {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
-
- if (log)
-log->Printf("NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
-
- Error error;
+ Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
// Read hardware breakpoint and watchpoint information.
- error = ReadHardwareDebugInfo();
+ Error error = ReadHardwareDebugInfo();
if (error.Fail())
return 0;
+ LLDB_LOG(log, "{0}", m_max_hwp_supported);
return m_max_hwp_supported;
}
uint32_t NativeRegisterContextLinux_arm::SetHardwareWatchpoint(
lldb::addr_t addr, size_t size, uint32_t watch_flags) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
-
- if (log)
-log->Printf("NativeRegisterContextLinux_arm::%s()", __FUNCTION__);
-
- Error error;
+ Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+ LLDB_LOG(log, "addr: {0:x}, size: {1:x} watch_flags: {2:x}", addr, size,
+ watch_flags);
// Read hardware bre
[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&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/Target/ThreadPlan.cpp
Modified: lldb/trunk/source/Target/ThreadPlan.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadPlan.cpp?rev=294024&r1=294023&r2=294024&view=diff
==
--- lldb/trunk/source/Target/ThreadPlan.cpp (original)
+++ lldb/trunk/source/Target/ThreadPlan.cpp Fri Feb 3 12:50:45 2017
@@ -76,14 +76,11 @@ Vote ThreadPlan::ShouldReportStop(Event
ThreadPlan *prev_plan = GetPreviousPlan();
if (prev_plan) {
Vote prev_vote = prev_plan->ShouldReportStop(event_ptr);
- if (log)
-log->Format(__FILE__, __FUNCTION__,
-"returning previous thread plan vote: {0}", prev_vote);
+ LLDB_LOG(log, "returning previous thread plan vote: {0}", prev_vote);
return prev_vote;
}
}
- if (log)
-log->Printf("Returning vote: {0}", m_stop_vote);
+ LLDB_LOG(log, "Returning vote: {0}", m_stop_vote);
return m_stop_vote;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[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
cmake/modules/LLDBConfig.cmake
scripts/Python/finishSwigPythonLLDB.py
scripts/finishSwigWrapperClasses.py
scripts/utilsArgsParse.py
Index: scripts/utilsArgsParse.py
===
--- scripts/utilsArgsParse.py
+++ scripts/utilsArgsParse.py
@@ -113,12 +113,12 @@
break
if match == 0:
for arg in vListLongArgs:
-argg = "--" + arg[:arg.__len__() - 1]
+argg = "--" + arg.rstrip('=')
if opt == argg:
if "m" == vDictArgReq[opt]:
countMandatoryOpts = countMandatoryOpts + 1
dictArgs[opt] = val
-if val.__len__() == 0:
+if arg[-1:] == '=' and val.__len__() == 0:
bFoundNoInputValue = True
break
Index: scripts/finishSwigWrapperClasses.py
===
--- scripts/finishSwigWrapperClasses.py
+++ scripts/finishSwigWrapperClasses.py
@@ -81,6 +81,7 @@
created for a Windows build.\n\
--argsFile= The args are read from a file instead of the\n\
command line. Other command line args are ignored.\n\
+--useSystemSix Use system six.py version.\n\
\n\
Usage:\n\
finishSwigWrapperClasses.py --srcRoot=ADirPath --targetDir=ADirPath\n\
@@ -178,7 +179,8 @@
"prefix=",
"cmakeBuildConfiguration=",
"lldbLibDir=",
-"argsFile"]
+"argsFile",
+"useSystemSix"]
dictArgReq = {"-h": "o", # o = optional, m = mandatory
"-d": "o",
"-m": "o",
@@ -188,7 +190,8 @@
"--prefix": "o",
"--cmakeBuildConfiguration": "o",
"--lldbLibDir": "o",
- "--argsFile": "o"}
+ "--argsFile": "o",
+ "--useSystemSix": "o"}
# Check for mandatory parameters
nResult, dictArgs, strMsg = utilsArgsParse.parse(vArgv, strListArgs,
@@ -376,9 +379,11 @@
(optional) "lib" by default.
--argsFile= The args are read from a file instead of the
command line. Other command line args are ignored.
+--useSystemSix Use system six.py version.
Usage:
finishSwigWrapperClasses.py --srcRoot=ADirPath --targetDir=ADirPath
--cfgBldDir=ADirPath --prefix=ADirPath --lldbLibDir=ADirPath -m -d
+--useSystemSix
Results:0 Success
-1 Error - invalid parameters passed.
Index: scripts/Python/finishSwigPythonLLDB.py
===
--- scripts/Python/finishSwigPythonLLDB.py
+++ scripts/Python/finishSwigPythonLLDB.py
@@ -821,7 +821,9 @@
bOk, strMsg = create_symlinks(
vDictArgs, strFrameworkPythonDir, strLldbLibDir)
-if bOk:
+bUseSystemSix = "--useSystemSix" in vDictArgs
+
+if not bUseSystemSix and bOk:
bOk, strMsg = copy_six(vDictArgs, strFrameworkPythonDir)
if bOk:
Index: cmake/modules/LLDBConfig.cmake
===
--- cmake/modules/LLDBConfig.cmake
+++ cmake/modules/LLDBConfig.cmake
@@ -29,6 +29,9 @@
set(LLDB_RELOCATABLE_PYTHON 0 CACHE BOOL
"Causes LLDB to use the PYTHONHOME environment variable to locate Python.")
+set(LLDB_USE_SYSTEM_SIX 0 CACHE BOOL
+ "Use six.py shipped with system and do not install a copy of it")
+
if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
set(LLDB_EXPORT_ALL_SYMBOLS 0 CACHE BOOL
"Causes lldb to export all symbols when building liblldb.")
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -20,6 +20,10 @@
# add_subdirectory(include)
add_subdirectory(docs)
if (NOT LLDB_DISABLE_PYTHON)
+ if(LLDB_USE_SYSTEM_SIX)
+set(SIX_EXTRA_ARGS "--useSystemSix")
+ endif()
+
set(LLDB_PYTHON_TARGET_DIR ${LLDB_BINARY_DIR}/scripts)
if(LLDB_BUILD_FRAMEWORK)
set(LLDB_PYTHON_TARGET_DIR
@@ -50,6 +54,7 @@
--prefix=${CMAKE_BINARY_DIR}
--cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}
--lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}
+ ${SIX_EXTRA_ARGS}
${FINISH_EXTRA_ARGS}
VERBATIM
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[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&view=rev
Log:
Add a missing break statement
Modified:
lldb/trunk/source/Commands/CommandObjectLog.cpp
Modified: lldb/trunk/source/Commands/CommandObjectLog.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectLog.cpp?rev=294036&r1=294035&r2=294036&view=diff
==
--- lldb/trunk/source/Commands/CommandObjectLog.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectLog.cpp Fri Feb 3 14:26:57 2017
@@ -133,6 +133,7 @@ public:
break;
case 'F':
log_options |= LLDB_LOG_OPTION_PREPEND_FILE_FUNCTION;
+break;
default:
error.SetErrorStringWithFormat("unrecognized option '%c'",
short_option);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[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 as a shorthand for if(log &&
log->GetVerbose()) and switched most of the affected log statements to
use that (I've only left a couple of cases that were doing complex
computations in an if(log) block).
https://reviews.llvm.org/D29510
Files:
include/lldb/Core/Log.h
include/lldb/Core/Logging.h
source/API/SBBroadcaster.cpp
source/API/SBEvent.cpp
source/Core/DataBufferMemoryMap.cpp
source/Core/Logging.cpp
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
source/Target/Memory.cpp
source/Target/SectionLoadList.cpp
source/Target/ThreadPlanCallFunction.cpp
Index: source/Target/ThreadPlanCallFunction.cpp
===
--- source/Target/ThreadPlanCallFunction.cpp
+++ source/Target/ThreadPlanCallFunction.cpp
@@ -174,9 +174,8 @@
}
void ThreadPlanCallFunction::ReportRegisterState(const char *message) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP |
- LIBLLDB_LOG_VERBOSE));
- if (log) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
+ if (log && log->GetVerbose()) {
StreamString strm;
RegisterContext *reg_ctx = m_thread.GetRegisterContext().get();
Index: source/Target/SectionLoadList.cpp
===
--- source/Target/SectionLoadList.cpp
+++ source/Target/SectionLoadList.cpp
@@ -67,21 +67,13 @@
bool SectionLoadList::SetSectionLoadAddress(const lldb::SectionSP §ion,
addr_t load_addr,
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LIBLLDB_LOG_VERBOSE));
-
+ Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
ModuleSP module_sp(section->GetModule());
if (module_sp) {
-if (log) {
- const FileSpec &module_file_spec(module_sp->GetFileSpec());
- log->Printf("SectionLoadList::%s (section = %p (%s.%s), load_addr = "
- "0x%16.16" PRIx64 ") module = %p",
- __FUNCTION__, static_cast(section.get()),
- module_file_spec.GetPath().c_str(),
- section->GetName().AsCString(), load_addr,
- static_cast(module_sp.get()));
-}
+LLDB_LOGV(log, "(section = {0} ({1}.{2}), load_addr = {3:x}) module = {4}",
+ section.get(), module_sp->GetFileSpec(),
+ section->GetName().AsCString(), load_addr, module_sp.get());
if (section->GetByteSize() == 0)
return false; // No change
@@ -147,10 +139,9 @@
size_t unload_count = 0;
if (section_sp) {
-Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
-LIBLLDB_LOG_VERBOSE));
+Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
-if (log) {
+if (log && log->GetVerbose()) {
ModuleSP module_sp = section_sp->GetModule();
std::string module_name("");
if (module_sp) {
@@ -183,10 +174,9 @@
bool SectionLoadList::SetSectionUnloaded(const lldb::SectionSP §ion_sp,
addr_t load_addr) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LIBLLDB_LOG_VERBOSE));
+ Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
- if (log) {
+ if (log && log->GetVerbose()) {
ModuleSP module_sp = section_sp->GetModule();
std::string module_name("");
if (module_sp) {
Index: source/Target/Memory.cpp
===
--- source/Target/Memory.cpp
+++ source/Target/Memory.cpp
@@ -263,16 +263,15 @@
lldb::addr_t AllocatedBlock::ReserveBlock(uint32_t size) {
addr_t addr = LLDB_INVALID_ADDRESS;
- Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_VERBOSE));
+ Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
if (size <= m_byte_size) {
const uint32_t needed_chunks = CalculateChunksNeededForSize(size);
if (m_offset_to_chunk_size.empty()) {
m_offset_to_chunk_size[0] = needed_chunks;
- if (log)
-log->Printf("[1] AllocatedBlock::ReserveBlock(%p) (size = %u (0x%x)
[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.
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/source/Utility/Error.cpp
llvm/include/llvm/Support/FormatProviders.h
Index: llvm/include/llvm/Support/FormatProviders.h
===
--- llvm/include/llvm/Support/FormatProviders.h
+++ llvm/include/llvm/Support/FormatProviders.h
@@ -18,6 +18,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/Support/FormatVariadicDetails.h"
#include "llvm/Support/NativeFormatting.h"
@@ -262,6 +263,12 @@
}
};
+template <> struct format_provider {
+ static void format(const Twine &T, raw_ostream &Stream, StringRef Style) {
+Stream << T;
+ }
+};
+
/// Implementation of format_provider for floating point types.
///
/// The options string of a floating point type has the format:
Index: lldb/source/Utility/Error.cpp
===
--- lldb/source/Utility/Error.cpp
+++ lldb/source/Utility/Error.cpp
@@ -130,72 +130,6 @@
bool Error::Fail() const { return m_code != 0; }
//--
-// Log the error given a string with format. If the this object
-// contains an error code, update the error string to contain the
-// "error: " followed by the formatted string, followed by the error
-// value and any string that describes the current error. This
-// allows more context to be given to an error string that remains
-// cached in this object. Logging always occurs even when the error
-// code contains a non-error value.
-//--
-void Error::PutToLog(Log *log, const char *format, ...) {
- char *arg_msg = nullptr;
- va_list args;
- va_start(args, format);
- ::vasprintf(&arg_msg, format, args);
- va_end(args);
-
- if (arg_msg != nullptr) {
-if (Fail()) {
- const char *err_str = AsCString();
- if (err_str == nullptr)
-err_str = "???";
-
- SetErrorStringWithFormat("error: %s err = %s (0x%8.8x)", arg_msg, err_str,
- m_code);
- if (log != nullptr)
-log->Error("%s", m_string.c_str());
-} else {
- if (log != nullptr)
-log->Printf("%s err = 0x%8.8x", arg_msg, m_code);
-}
-::free(arg_msg);
- }
-}
-
-//--
-// Log the error given a string with format. If the this object
-// contains an error code, update the error string to contain the
-// "error: " followed by the formatted string, followed by the error
-// value and any string that describes the current error. This
-// allows more context to be given to an error string that remains
-// cached in this object. Logging only occurs even when the error
-// code contains a error value.
-//--
-void Error::LogIfError(Log *log, const char *format, ...) {
- if (Fail()) {
-char *arg_msg = nullptr;
-va_list args;
-va_start(args, format);
-::vasprintf(&arg_msg, format, args);
-va_end(args);
-
-if (arg_msg != nullptr) {
- const char *err_str = AsCString();
- if (err_str == nullptr)
-err_str = "???";
-
- SetErrorStringWithFormat("%s err = %s (0x%8.8x)", arg_msg, err_str,
- m_code);
- if (log != nullptr)
-log->Error("%s", m_string.c_str());
-
- ::free(arg_msg);
-}
- }
-}
-
-//--
// Set accesssor for the error value to "err" and the type to
// "eErrorTypeMachKernel"
//--
@@ -334,3 +268,10 @@
bool Error::WasInterrupted() const {
return (m_type == eErrorTypePOSIX && m_code == EINTR);
}
+
+void llvm::format_provider::format(
+const lldb_private::Error &error, llvm::raw_ostream &OS,
+llvm::StringRef Options) {
+ llvm::format_provider::format(error.AsCString(), OS,
+ Options);
+}
Index: lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
===
--- lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
+++ lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
@@ -319,7 +319,7 @@
for (uint64_t i = 0; i < allglen; ++i) {
goroutines.pu
[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 mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
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 > 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 not apply there as well. > > - GetProcessInfo, FindProcesses: The implementation was the same in all > classes. > > - GetFullNameForDylib: This code should apply to all non-darwin > platforms. I've kept the PlatformDarwin override as the situation is > different there. > > Reviewers: clayborg, krytarowski, emaste > > Subscribers: lldb-commits > > Differential Revision: https://reviews.llvm.org/D29496 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
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 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 >> 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 not apply there as well. >> >> - GetProcessInfo, FindProcesses: The implementation was the same in all >> classes. >> >> - GetFullNameForDylib: This code should apply to all non-darwin >> platforms. I've kept the PlatformDarwin override as the situation is >> different there. >> >> Reviewers: clayborg, krytarowski, emaste >> >> Subscribers: lldb-commits >> >> Differential Revision: 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] 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 though). 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
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 to ensure we print "error:" it is isn't already prefixed with 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] 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
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 svn > commit numbers. > > > > On 2/3/2017 4:54 PM, 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'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 >>> 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 not apply there as well. >>> >>> - GetProcessInfo, FindProcesses: The implementation was the same in all >>> classes. >>> >>> - GetFullNameForDylib: This code should apply to all non-darwin >>> platforms. I've kept the PlatformDarwin override as the situation is >>> different there. >>> >>> Reviewers: clayborg, krytarowski, emaste >>> >>> Subscribers: lldb-commits >>> >>> Differential Revision: https://reviews.llvm.org/D29496 > > > -- > Geoff Berry > Employee of Qualcomm Datacenter Technologies, Inc. > Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm > Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code > Aurora Forum, a Linux Foundation Collaborative Project. > ___ 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, error, "foo");`
> Am I missing something?
I'm all for making it simpler. The reason I did it this way is that I wanted
to keep the exact same syntax as before to minimize the impact of the change.
It's rather confusing, but suppose you have these two errors:
1. E1 = Success, message = "The operation completed successfully", code = 0
2. E2 = Error, message = "The operation failed", code = 0x12345
When you call `PutToLog(E1, "While reading the %d'th item", 7)` it will log the
string `While reading the 7'th item err = 0x0`
When you call `PutToLog(E2, "While reading the %d'th item", 7)` it will log the
string `error: While reading the 7'th item err = The operation failed (0x12345)`
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'th item" in the above
example). So if we did this:
LLDB_LOG(log, error, "While reading the {0}'th item", 7)
Then the entire error must be formatted independently of the entire context
string. So we could produce the output `error: The operation failed (0x12345),
context: While reading the 7'th item` but this is slightly different from what
was being printed before.
I didn't actually try this approach and see if anything broke, because Windows
runs much fewer tests than the rest of LLDB, so even if my test suite passed,
it wouldn't guarantee that another test somewhere else wasn't relying on the
message being exactly as it currently prints.
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
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'th item" in the above
> example). So if we did this:
>
> LLDB_LOG(log, error, "While reading the {0}'th item", 7)
>
>
> Then the entire error must be formatted independently of the entire context
> string. So we could produce the output `error: The operation failed
> (0x12345), context: While reading the 7'th item` but this is slightly
> different from what was being printed before.
>
> I didn't actually try this approach and see if anything broke, because
> Windows runs much fewer tests than the rest of LLDB, so even if my test suite
> passed, it wouldn't guarantee that another test somewhere else wasn't relying
> on the message being exactly as it currently prints.
I doubt anything is depending on the exact string printed to the log. We should
just make the new output makes sense.
I'd format this message like this:
`LLDB_LOG(log, "Reading the {0}'th item: {1}", 7, error)`
which should give you something like:
`Reading 7th item: Too many open files.`
if you want, we can add the "error:" prefix, numeric error code, or something
to the format provider,, but this would be enough for me.
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. 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
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/listinfo/lldb-commits
[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category
zturner added inline comments.
Comment at: source/Target/SectionLoadList.cpp:70
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LIBLLDB_LOG_VERBOSE));
-
+ Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
ModuleSP module_sp(section->GetModule());
There's a subtle behavioral change here, which is that previously this line
would get logged even if `LIBLLDB_LOG_DYNAMIC_LOADER` was not set, and now it
won't. However, I've always found the `GetLogIfAny` / `GetLogIfAll` split
unnecessarily confusing, so I'm not opposed to this change. Just want to point
it out.
Comment at: source/Target/SectionLoadList.cpp:76
+ section.get(), module_sp->GetFileSpec(),
+ section->GetName().AsCString(), load_addr, module_sp.get());
No need to call `AsCString()`.
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] 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
lldb/source/Utility/Error.cpp
Index: lldb/source/Utility/Error.cpp
===
--- lldb/source/Utility/Error.cpp
+++ lldb/source/Utility/Error.cpp
@@ -130,72 +130,6 @@
bool Error::Fail() const { return m_code != 0; }
//--
-// Log the error given a string with format. If the this object
-// contains an error code, update the error string to contain the
-// "error: " followed by the formatted string, followed by the error
-// value and any string that describes the current error. This
-// allows more context to be given to an error string that remains
-// cached in this object. Logging always occurs even when the error
-// code contains a non-error value.
-//--
-void Error::PutToLog(Log *log, const char *format, ...) {
- char *arg_msg = nullptr;
- va_list args;
- va_start(args, format);
- ::vasprintf(&arg_msg, format, args);
- va_end(args);
-
- if (arg_msg != nullptr) {
-if (Fail()) {
- const char *err_str = AsCString();
- if (err_str == nullptr)
-err_str = "???";
-
- SetErrorStringWithFormat("error: %s err = %s (0x%8.8x)", arg_msg, err_str,
- m_code);
- if (log != nullptr)
-log->Error("%s", m_string.c_str());
-} else {
- if (log != nullptr)
-log->Printf("%s err = 0x%8.8x", arg_msg, m_code);
-}
-::free(arg_msg);
- }
-}
-
-//--
-// Log the error given a string with format. If the this object
-// contains an error code, update the error string to contain the
-// "error: " followed by the formatted string, followed by the error
-// value and any string that describes the current error. This
-// allows more context to be given to an error string that remains
-// cached in this object. Logging only occurs even when the error
-// code contains a error value.
-//--
-void Error::LogIfError(Log *log, const char *format, ...) {
- if (Fail()) {
-char *arg_msg = nullptr;
-va_list args;
-va_start(args, format);
-::vasprintf(&arg_msg, format, args);
-va_end(args);
-
-if (arg_msg != nullptr) {
- const char *err_str = AsCString();
- if (err_str == nullptr)
-err_str = "???";
-
- SetErrorStringWithFormat("%s err = %s (0x%8.8x)", arg_msg, err_str,
- m_code);
- if (log != nullptr)
-log->Error("%s", m_string.c_str());
-
- ::free(arg_msg);
-}
- }
-}
-
-//--
// Set accesssor for the error value to "err" and the type to
// "eErrorTypeMachKernel"
//--
@@ -334,3 +268,10 @@
bool Error::WasInterrupted() const {
return (m_type == eErrorTypePOSIX && m_code == EINTR);
}
+
+void llvm::format_provider::format(
+const lldb_private::Error &error, llvm::raw_ostream &OS,
+llvm::StringRef Options) {
+ llvm::format_provider::format(error.AsCString(), OS,
+ Options);
+}
Index: lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
===
--- lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
+++ lldb/source/Plugins/OperatingSystem/Go/OperatingSystemGo.cpp
@@ -319,7 +319,7 @@
for (uint64_t i = 0; i < allglen; ++i) {
goroutines.push_back(CreateGoroutineAtIndex(i, err));
if (err.Fail()) {
- err.PutToLog(log, "OperatingSystemGo::UpdateThreadList");
+ LLDB_LOG(log, "error: {0}", err);
return new_thread_list.GetSize(false) > 0;
}
}
Index: lldb/source/Host/common/Host.cpp
===
--- lldb/source/Host/common/Host.cpp
+++ lldb/source/Host/common/Host.cpp
@@ -685,10 +685,10 @@
posix_spawnattr_t attr;
error.SetError(::posix_spawnattr_init(&attr), eErrorTypePOSIX);
- if (error.Fail() || log)
-error.PutToLog(log, "::posix_spawnattr_init ( &attr )");
- if (error.Fail())
+ if (error.Fail()) {
+LLDB_LOG(log, "error: {0}, ::posix_spawnattr_init ( &attr )", error);
return error;
+ }
// Make a quick class that will cleanup the posix spawn attributes in case
// we return in the middle of this function.
@@ -709,11 +709,12 @@
short flags = GetPosixspawnFlags(launch_info);
error.SetError(::posix_spawnattr_setflags(&attr, flags), eErrorTypePOSIX);
- i
[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] 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/lldb-private.h"
This is also unnecessary.
Comment at: lldb/include/lldb/Utility/Error.h:14
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
All these includes are now unnecessary :)
Comment at: lldb/source/Host/common/Host.cpp:911
eErrorTypePOSIX);
- if (log && (error.Fail() || log))
-error.PutToLog(log,
lol :)
Comment at: lldb/source/Host/common/Host.cpp:952
+ if (error.Fail())
+LLDB_LOG_ERROR(
+log, error, "posix_spawn_file_actions_addopen (action={0}, "
looks like you forgot this one.
https://reviews.llvm.org/D29514
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D29514: Change Error::PutToLog to LLDB_LOG_ERROR
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.
>
> 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/lldb-private.h"
>
> This is also unnecessary.
>
>
>
> Comment at: lldb/include/lldb/Utility/Error.h:14
>
> +#include "llvm/ADT/StringRef.h"
> +#include "llvm/ADT/Twine.h"
>
> All these includes are now unnecessary :)
>
>
>
> Comment at: lldb/source/Host/common/Host.cpp:911
>eErrorTypePOSIX);
> - if (log && (error.Fail() || log))
> -error.PutToLog(log,
>
> lol :)
>
>
>
> Comment at: lldb/source/Host/common/Host.cpp:952
> + if (error.Fail())
> +LLDB_LOG_ERROR(
> +log, error, "posix_spawn_file_actions_addopen (action={0}, "
>
> looks like you forgot this one.
>
>
> 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
labath added a comment.
Comment at: source/Target/SectionLoadList.cpp:70
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LIBLLDB_LOG_VERBOSE));
-
+ Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
ModuleSP module_sp(section->GetModule());
zturner wrote:
> There's a subtle behavioral change here, which is that previously this line
> would get logged even if `LIBLLDB_LOG_DYNAMIC_LOADER` was not set, and now it
> won't. However, I've always found the `GetLogIfAny` / `GetLogIfAll` split
> unnecessarily confusing, so I'm not opposed to this change. Just want to
> point it out.
You're right I didn't notice that. However, I suspect that this was actually
not intended, so it should be fine. It'd be interesting to see how many calls
to IfAny/IfAll we have with more than one category after this change.
Comment at: source/Target/SectionLoadList.cpp:76
+ section.get(), module_sp->GetFileSpec(),
+ section->GetName().AsCString(), load_addr, module_sp.get());
zturner wrote:
> No need to call `AsCString()`.
We don't have a formatter for ConstString yet. I'll interpret this as a request
to add one.
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
clayborg added inline comments.
Comment at: source/Target/SectionLoadList.cpp:70
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LIBLLDB_LOG_VERBOSE));
-
+ Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
ModuleSP module_sp(section->GetModule());
labath wrote:
> zturner wrote:
> > There's a subtle behavioral change here, which is that previously this line
> > would get logged even if `LIBLLDB_LOG_DYNAMIC_LOADER` was not set, and now
> > it won't. However, I've always found the `GetLogIfAny` / `GetLogIfAll`
> > split unnecessarily confusing, so I'm not opposed to this change. Just
> > want to point it out.
> You're right I didn't notice that. However, I suspect that this was actually
> not intended, so it should be fine. It'd be interesting to see how many calls
> to IfAny/IfAll we have with more than one category after this change.
The previous usage was incorrect indeed. It should have been IfAll. IfAny only
works when you might have something that should be logged for say "process" or
for "thread". Then IfAny makes sense. Never makes sense to use IfAny with one
any flag and verbose...
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] D29514: Change Error::PutToLog to LLDB_LOG_ERROR
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-commits
[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 have some doc that references where the built-in formatters are? 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 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-commits
[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? There are a couple of Format > calls that use it, maybe they should have some doc that references where the > built-in formatters are? A couple ideas come to mind. 1. Try it and see if it compiles. This won't tell you if a formatter exists and you didn't include the right header for it, though. Although if the formatter is always defined in the same header file that the object itself is defined in, this wouldn't be a problem. 2. Grep the codebase for `format_provider` and `FormatAdapter` 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
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/SBBroadcaster.cpp
source/API/SBEvent.cpp
source/Core/DataBufferMemoryMap.cpp
source/Core/Logging.cpp
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
source/Plugins/Platform/MacOSX/PlatformRemoteAppleTV.cpp
source/Plugins/Platform/MacOSX/PlatformRemoteAppleWatch.cpp
source/Plugins/Platform/MacOSX/PlatformRemoteiOS.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
source/Target/Memory.cpp
source/Target/SectionLoadList.cpp
source/Target/ThreadPlanCallFunction.cpp
source/Utility/ConstString.cpp
unittests/Utility/CMakeLists.txt
unittests/Utility/ConstStringTest.cpp
Index: unittests/Utility/ConstStringTest.cpp
===
--- /dev/null
+++ unittests/Utility/ConstStringTest.cpp
@@ -0,0 +1,18 @@
+//===-- ConstStringTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Utility/ConstString.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+TEST(ConstStringTest, format_provider) {
+ EXPECT_EQ("foo", llvm::formatv("{0}", ConstString("foo")).str());
+}
Index: unittests/Utility/CMakeLists.txt
===
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
add_lldb_unittest(UtilityTests
+ ConstStringTest.cpp
ModuleCacheTest.cpp
StringExtractorTest.cpp
TaskPoolTest.cpp
Index: source/Utility/ConstString.cpp
===
--- source/Utility/ConstString.cpp
+++ source/Utility/ConstString.cpp
@@ -334,3 +334,9 @@
// Get the size of the static string pool
return StringPool().MemorySize();
}
+
+void llvm::format_provider::format(const ConstString &CS,
+llvm::raw_ostream &OS,
+llvm::StringRef Options) {
+ format_provider::format(CS.AsCString(), OS, Options);
+}
Index: source/Target/ThreadPlanCallFunction.cpp
===
--- source/Target/ThreadPlanCallFunction.cpp
+++ source/Target/ThreadPlanCallFunction.cpp
@@ -174,9 +174,8 @@
}
void ThreadPlanCallFunction::ReportRegisterState(const char *message) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP |
- LIBLLDB_LOG_VERBOSE));
- if (log) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
+ if (log && log->GetVerbose()) {
StreamString strm;
RegisterContext *reg_ctx = m_thread.GetRegisterContext().get();
Index: source/Target/SectionLoadList.cpp
===
--- source/Target/SectionLoadList.cpp
+++ source/Target/SectionLoadList.cpp
@@ -67,21 +67,13 @@
bool SectionLoadList::SetSectionLoadAddress(const lldb::SectionSP §ion,
addr_t load_addr,
bool warn_multiple) {
- Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
- LIBLLDB_LOG_VERBOSE));
-
+ Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
ModuleSP module_sp(section->GetModule());
if (module_sp) {
-if (log) {
- const FileSpec &module_file_spec(module_sp->GetFileSpec());
- log->Printf("SectionLoadList::%s (section = %p (%s.%s), load_addr = "
- "0x%16.16" PRIx64 ") module = %p",
- __FUNCTION__, static_cast(section.get()),
- module_file_spec.GetPath().c_str(),
- section->GetName().AsCString(), load_addr,
- static_cast(module_sp.get()));
-}
+LLDB_LOGV(log, "(section = {0} ({1}.{2}), load_addr = {3:x}) module = {4}",
+ section.get(), module_sp->GetFileSpec(), section->GetName(),
+ load_addr, module_sp.get());
if (section->GetByteSize() == 0)
return false; // No change
@@ -147,10 +139,9 @@
size_t unload_count = 0;
if (section_sp) {
-Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER |
-LIBLLDB_LOG_VERBOSE));
+Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_
[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. 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] [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&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 with distribution's
py-six copy.
Introduce new configure option LLDB_USE_SYSTEM_SIX (disabled by default). Once
specified as TRUE, six.py won't be installed to Python's directory.
Add new option in finishSwigWrapperClasses.py, namely --useSystemSix.
Sponsored by
Reviewers: mgorny, emaste, clayborg, joerg, labath
Reviewed By: labath
Subscribers: #lldb
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D29405
Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/cmake/modules/LLDBConfig.cmake
lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
lldb/trunk/scripts/finishSwigWrapperClasses.py
lldb/trunk/scripts/utilsArgsParse.py
Modified: lldb/trunk/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=294071&r1=294070&r2=294071&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Fri Feb 3 18:20:24 2017
@@ -20,6 +20,10 @@ endif()
# add_subdirectory(include)
add_subdirectory(docs)
if (NOT LLDB_DISABLE_PYTHON)
+ if(LLDB_USE_SYSTEM_SIX)
+set(SIX_EXTRA_ARGS "--useSystemSix")
+ endif()
+
set(LLDB_PYTHON_TARGET_DIR ${LLDB_BINARY_DIR}/scripts)
if(LLDB_BUILD_FRAMEWORK)
set(LLDB_PYTHON_TARGET_DIR
@@ -50,6 +54,7 @@ if (NOT LLDB_DISABLE_PYTHON)
--prefix=${CMAKE_BINARY_DIR}
--cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}
--lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}
+ ${SIX_EXTRA_ARGS}
${FINISH_EXTRA_ARGS}
VERBATIM
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=294071&r1=294070&r2=294071&view=diff
==
--- lldb/trunk/cmake/modules/LLDBConfig.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBConfig.cmake Fri Feb 3 18:20:24 2017
@@ -29,6 +29,9 @@ set(LLDB_DISABLE_CURSES ${LLDB_DEFAULT_D
set(LLDB_RELOCATABLE_PYTHON 0 CACHE BOOL
"Causes LLDB to use the PYTHONHOME environment variable to locate Python.")
+set(LLDB_USE_SYSTEM_SIX 0 CACHE BOOL
+ "Use six.py shipped with system and do not install a copy of it")
+
if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
set(LLDB_EXPORT_ALL_SYMBOLS 0 CACHE BOOL
"Causes lldb to export all symbols when building liblldb.")
Modified: lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/finishSwigPythonLLDB.py?rev=294071&r1=294070&r2=294071&view=diff
==
--- lldb/trunk/scripts/Python/finishSwigPythonLLDB.py (original)
+++ lldb/trunk/scripts/Python/finishSwigPythonLLDB.py Fri Feb 3 18:20:24 2017
@@ -821,7 +821,9 @@ def main(vDictArgs):
bOk, strMsg = create_symlinks(
vDictArgs, strFrameworkPythonDir, strLldbLibDir)
-if bOk:
+bUseSystemSix = "--useSystemSix" in vDictArgs
+
+if not bUseSystemSix and bOk:
bOk, strMsg = copy_six(vDictArgs, strFrameworkPythonDir)
if bOk:
Modified: lldb/trunk/scripts/finishSwigWrapperClasses.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/finishSwigWrapperClasses.py?rev=294071&r1=294070&r2=294071&view=diff
==
--- lldb/trunk/scripts/finishSwigWrapperClasses.py (original)
+++ lldb/trunk/scripts/finishSwigWrapperClasses.py Fri Feb 3 18:20:24 2017
@@ -81,6 +81,7 @@ Args: -h (optional) Print
created for a Windows build.\n\
--argsFile= The args are read from a file instead of the\n\
command line. Other command line args are ignored.\n\
+--useSystemSix Use system six.py version.\n\
\n\
Usage:\n\
finishSwigWrapperClasses.py --srcRoot=ADirPath --targetDir=ADirPath\n\
@@ -178,7 +179,8 @@ def validate_arguments(vArgv):
"prefix=",
"cmakeBuildConfiguration=",
"lldbLibDir=",
-"argsFile"]
+"argsFile",
+"useSystemSix"]
dictArgReq = {"-h": "o", # o = optional, m = mandatory
"-d": "o",
"-m": "o",
@@ -188,7 +190,8 @@ def validate_arguments(vArgv):
"--prefix": "o",
"--cmakeBuildConfiguration": "o",
"--lldbLibDir": "o",
- "--argsFile": "o"}
+ "--argsFile": "o",
+ "--useSystemSix": "o"}
# Check for
[Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category
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 you learned some more about how it works. Probably someone using this would try to print something, and get a compiler error that triggers a helpful static_assert which says "missing format provider for type = Foo!". That gives them the next piece of the puzzle they need to understand a little bit more. Or they look at the code for `formatv` to see how it works and see all the documentation. It sounds analogous to asking "how does someone new to the codebase know that commands in LLDB are implemented by something which inherits from CommandObject?". Of course you don't if you're brand new, but once you figure it out you don't really have to spend much time thinking about it again. https://reviews.llvm.org/D29510 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
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 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 you learned some more about how it > works. Probably someone using this would try to print something, and get a > compiler error that triggers a helpful static_assert which says "missing > format provider for type = Foo!". That gives them the next piece of the > puzzle they need to understand a little bit more. Or they look at the code > for `formatv` to see how it works and see all the documentation. > > It sounds analogous to asking "how does someone new to the codebase know that > commands in LLDB are implemented by something which inherits from > CommandObject?". Of course you don't if you're brand new, but once you > figure it out you don't really have to spend much time thinking about it > again. > > > 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
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 on their own. https://reviews.llvm.org/D29510 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category
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 Fri, Feb 3, 2017 at 4:48 PM Jim Ingham wrote: > 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 < > revi...@reviews.llvm.org> 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, 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 you learned some more about > how it works. Probably someone using this would try to print something, > and get a compiler error that triggers a helpful static_assert which says > "missing format provider for type = Foo!". That gives them the next piece > of the puzzle they need to understand a little bit more. Or they look at > the code for `formatv` to see how it works and see all the documentation. > > > > It sounds analogous to asking "how does someone new to the codebase know > that commands in LLDB are implemented by something which inherits from > CommandObject?". Of course you don't if you're brand new, but once you > figure it out you don't really have to spend much time thinking about it > again. > > > > > > https://reviews.llvm.org/D29510 > > > > > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category
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 see what's there. Jim > On Feb 3, 2017, at 4:49 PM, Zachary Turner wrote: > > 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 Fri, Feb 3, 2017 at 4:48 PM Jim Ingham wrote: > 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, 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 you learned some more about > > how it works. Probably someone using this would try to print something, > > and get a compiler error that triggers a helpful static_assert which says > > "missing format provider for type = Foo!". That gives them the next piece > > of the puzzle they need to understand a little bit more. Or they look at > > the code for `formatv` to see how it works and see all the documentation. > > > > It sounds analogous to asking "how does someone new to the codebase know > > that commands in LLDB are implemented by something which inherits from > > CommandObject?". Of course you don't if you're brand new, but once you > > figure it out you don't really have to spend much time thinking about it > > again. > > > > > > https://reviews.llvm.org/D29510 > > > > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D29510: Remove LIBLLDB_LOG_VERBOSE category
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 available formatters > centralized somewhere so folks don't have to go scouring through the code > to see what's there. > > Jim > > > On Feb 3, 2017, at 4:49 PM, Zachary Turner wrote: > > > > 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 Fri, Feb 3, 2017 at 4:48 PM Jim Ingham wrote: > > 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 < > revi...@reviews.llvm.org> 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, 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 you learned some > more about how it works. Probably someone using this would try to print > something, and get a compiler error that triggers a helpful static_assert > which says "missing format provider for type = Foo!". That gives them the > next piece of the puzzle they need to understand a little bit more. Or > they look at the code for `formatv` to see how it works and see all the > documentation. > > > > > > It sounds analogous to asking "how does someone new to the codebase > know that commands in LLDB are implemented by something which inherits from > CommandObject?". Of course you don't if you're brand new, but once you > figure it out you don't really have to spend much time thinking about it > again. > > > > > > > > > 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] D29513: Add -gdb-set and -gdb-show support for non-stop to lldb-mi
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::GetInternalVariableValue("target.x86-disassembly-flavor",
-
rDbgr.GetInstanceName()).GetStringAtIndex(0);
+ m_strValue = lldb::SBDebugger::GetInternalVariableValue(
+ "target.x86-disassembly-flavor", rDbgr.GetInstanceName())
I think clang-format will format it the different way.
https://reviews.llvm.org/D29513
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
