[Lldb-commits] [lldb] r289644 - Remove code needed for supporting old VS versions.

[Hafiz Abid Qadeer via lldb-commits]

Author: abidh
Date: Wed Dec 14 05:13:03 2016
New Revision: 289644

URL: http://llvm.org/viewvc/llvm-project?rev=289644&view=rev
Log:
Remove code needed for supporting old VS versions.

Summary: This code was probably needed to support VS2013 and is not needed now. 
I have built it with VS and mingw. Ok to remove it?

Reviewers: zturner, abidh

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D27707

Modified:
lldb/trunk/include/lldb/Host/windows/PosixApi.h

Modified: lldb/trunk/include/lldb/Host/windows/PosixApi.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/windows/PosixApi.h?rev=289644&r1=289643&r2=289644&view=diff
==
--- lldb/trunk/include/lldb/Host/windows/PosixApi.h (original)
+++ lldb/trunk/include/lldb/Host/windows/PosixApi.h Wed Dec 14 05:13:03 2016
@@ -75,15 +75,6 @@ typedef uint32_t pid_t;
 
 #endif // _MSC_VER
 
-// MSVC 2015 and higher have timespec.  Otherwise we need to define it
-// ourselves.
-#if !defined(_MSC_VER) || _MSC_VER < 1900
-struct timespec {
-  time_t tv_sec;
-  long tv_nsec;
-};
-#endif
-
 // Various useful posix functions that are not present in Windows.  We provide
 // custom implementations.
 int vasprintf(char **ret, const char *fmt, va_list ap);
@@ -113,20 +104,4 @@ inline char *ptsname(int fd) { LLVM_BUIL
 inline pid_t fork(void) { LLVM_BUILTIN_UNREACHABLE; }
 inline pid_t setsid(void) { LLVM_BUILTIN_UNREACHABLE; }
 
-// vsnprintf and snprintf are provided in MSVC 2015 and higher.
-#if _MSC_VER < 1900
-namespace lldb_private {
-int vsnprintf(char *buffer, size_t count, const char *format, va_list argptr);
-}
-
-// inline to avoid linkage conflicts
-int inline snprintf(char *buffer, size_t count, const char *format, ...) {
-  va_list argptr;
-  va_start(argptr, format);
-  int r = lldb_private::vsnprintf(buffer, count, format, argptr);
-  va_end(argptr);
-  return r;
-}
-#endif
-
 #endif


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D27707: Remove code needed for supporting old VS versions.

[Hafiz Abid Qadeer via Phabricator via lldb-commits]

abidh accepted this revision.
abidh added a reviewer: abidh.
abidh added a comment.
This revision is now accepted and ready to land.

Accepted on email.


https://reviews.llvm.org/D27707



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r289647 - Replace APFloatBase static fltSemantics data members with getter functions

[Stephan Bergmann via lldb-commits]

Author: sberg
Date: Wed Dec 14 05:57:17 2016
New Revision: 289647

URL: http://llvm.org/viewvc/llvm-project?rev=289647&view=rev
Log:
Replace APFloatBase static fltSemantics data members with getter functions

At least the plugin used by the LibreOffice build
() indirectly
uses those members (through inline functions in LLVM/Clang include files in turn
using them), but they are not exported by utils/extract_symbols.py on Windows,
and accessing data across DLL/EXE boundaries on Windows is generally
problematic.

Differential Revision: https://reviews.llvm.org/D26671

Modified:
lldb/trunk/include/lldb/Core/Scalar.h
lldb/trunk/source/Core/Scalar.cpp

Modified: lldb/trunk/include/lldb/Core/Scalar.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Scalar.h?rev=289647&r1=289646&r2=289647&view=diff
==
--- lldb/trunk/include/lldb/Core/Scalar.h (original)
+++ lldb/trunk/include/lldb/Core/Scalar.h Wed Dec 14 05:57:17 2016
@@ -75,11 +75,11 @@ public:
   Scalar(long double v, bool ieee_quad)
   : m_type(e_long_double), m_float((float)0), m_ieee_quad(ieee_quad) {
 if (ieee_quad)
-  m_float = llvm::APFloat(llvm::APFloat::IEEEquad,
+  m_float = llvm::APFloat(llvm::APFloat::IEEEquad(),
   llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128,
   ((type128 *)&v)->x));
 else
-  m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended,
+  m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended(),
   llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128,
   ((type128 *)&v)->x));
   }

Modified: lldb/trunk/source/Core/Scalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Scalar.cpp?rev=289647&r1=289646&r2=289647&view=diff
==
--- lldb/trunk/source/Core/Scalar.cpp (original)
+++ lldb/trunk/source/Core/Scalar.cpp Wed Dec 14 05:57:17 2016
@@ -403,11 +403,11 @@ Scalar &Scalar::operator=(long double v)
   m_type = e_long_double;
   if (m_ieee_quad)
 m_float = llvm::APFloat(
-llvm::APFloat::IEEEquad,
+llvm::APFloat::IEEEquad(),
 llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128, ((type128 *)&v)->x));
   else
 m_float = llvm::APFloat(
-llvm::APFloat::x87DoubleExtended,
+llvm::APFloat::x87DoubleExtended(),
 llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128, ((type128 *)&v)->x));
   return *this;
 }
@@ -509,9 +509,9 @@ bool Scalar::Promote(Scalar::Type type)
 
 case e_long_double:
   if (m_ieee_quad)
-m_float = llvm::APFloat(llvm::APFloat::IEEEquad, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::IEEEquad(), m_integer);
   else
-m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended(), m_integer);
   success = true;
   break;
 }
@@ -569,9 +569,9 @@ bool Scalar::Promote(Scalar::Type type)
 
 case e_long_double:
   if (m_ieee_quad)
-m_float = llvm::APFloat(llvm::APFloat::IEEEquad, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::IEEEquad(), m_integer);
   else
-m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended(), m_integer);
   success = true;
   break;
 }
@@ -625,9 +625,9 @@ bool Scalar::Promote(Scalar::Type type)
 
 case e_long_double:
   if (m_ieee_quad)
-m_float = llvm::APFloat(llvm::APFloat::IEEEquad, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::IEEEquad(), m_integer);
   else
-m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended(), m_integer);
   success = true;
   break;
 }
@@ -677,9 +677,9 @@ bool Scalar::Promote(Scalar::Type type)
 
 case e_long_double:
   if (m_ieee_quad)
-m_float = llvm::APFloat(llvm::APFloat::IEEEquad, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::IEEEquad(), m_integer);
   else
-m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended(), m_integer);
   success = true;
   break;
 }
@@ -725,9 +725,9 @@ bool Scalar::Promote(Scalar::Type type)
 
 case e_long_double:
   if (m_ieee_quad)
-m_float = llvm::APFloat(llvm::APFloat::IEEEquad, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::IEEEquad(), m_integer);
   else
-m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended, m_integer);
+m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended(), m_integer);
   success 

[Lldb-commits] [PATCH] D27759: Fix build for mingw.

[Hafiz Abid Qadeer via Phabricator via lldb-commits]

abidh created this revision.
abidh added reviewers: labath, zturner.
abidh added a subscriber: lldb-commits.
Herald added subscribers: mgorny, ki.stfu.

I was building lldb using cross mingw-w64 toolchain on Linux and observed some 
issues. This is first patch in the series to fix that build. It mostly corrects 
the case of include files and adjusts some #ifdefs from _MSC_VER to _WIN32 and 
vice versa. I built lldb on windows with VS after applying this patch to make 
sure it does not break the build there.


https://reviews.llvm.org/D27759

Files:
  cmake/LLDBDependencies.cmake
  cmake/modules/AddLLDB.cmake
  cmake/modules/LLDBConfig.cmake
  include/lldb/Host/windows/windows.h
  source/API/SystemInitializerFull.cpp
  source/Core/Mangled.cpp
  source/Host/common/File.cpp
  source/Host/windows/Host.cpp
  source/Host/windows/HostProcessWindows.cpp
  source/Host/windows/ProcessRunLock.cpp
  source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
  source/Plugins/Process/Windows/Common/ExceptionRecord.h
  source/Target/ProcessLaunchInfo.cpp
  tools/driver/Driver.cpp
  tools/lldb-mi/MICmnStreamStdin.cpp
  tools/lldb-mi/MIUtilDebug.cpp

Index: tools/lldb-mi/MIUtilDebug.cpp
===
--- tools/lldb-mi/MIUtilDebug.cpp
+++ tools/lldb-mi/MIUtilDebug.cpp
@@ -9,7 +9,7 @@
 
 // Third party headers:
 #ifdef _WIN32
-#include 
+#include 
 #endif
 
 // In-house headers:
Index: tools/lldb-mi/MICmnStreamStdin.cpp
===
--- tools/lldb-mi/MICmnStreamStdin.cpp
+++ tools/lldb-mi/MICmnStreamStdin.cpp
@@ -9,7 +9,7 @@
 
 // Third Party Headers
 #ifdef _MSC_VER
-#include 
+#include 
 #endif
 #include  // For std::strerror()
 
Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -1208,13 +1208,13 @@
 }
 
 int
-#ifdef WIN32
+#ifdef _MSC_VER
 wmain(int argc, wchar_t const *wargv[])
 #else
 main(int argc, char const *argv[])
 #endif
 {
-#ifdef _WIN32
+#ifdef _MSC_VER
   // Convert wide arguments to UTF-8
   std::vector argvStrings(argc);
   std::vector argvPointers(argc);
Index: source/Target/ProcessLaunchInfo.cpp
===
--- source/Target/ProcessLaunchInfo.cpp
+++ source/Target/ProcessLaunchInfo.cpp
@@ -294,7 +294,7 @@
   __FUNCTION__);
 
 int open_flags = O_RDWR | O_NOCTTY;
-#if !defined(_MSC_VER)
+#if !defined(_WIN32)
 // We really shouldn't be specifying platform specific flags
 // that are intended for a system call in generic code.  But
 // this will have to do for now.
Index: source/Plugins/Process/Windows/Common/ExceptionRecord.h
===
--- source/Plugins/Process/Windows/Common/ExceptionRecord.h
+++ source/Plugins/Process/Windows/Common/ExceptionRecord.h
@@ -12,7 +12,7 @@
 
 #include "lldb/Host/windows/windows.h"
 #include "lldb/lldb-forward.h"
-#include 
+#include 
 
 #include 
 #include 
Index: source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
===
--- source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
+++ source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
@@ -16,7 +16,7 @@
 
 #ifdef _WIN32
 #include "lldb/Host/windows/windows.h"
-#include  // for MiniDumpWriteDump
+#include  // for MiniDumpWriteDump
 #endif
 
 namespace lldb_private {
Index: source/Host/windows/ProcessRunLock.cpp
===
--- source/Host/windows/ProcessRunLock.cpp
+++ source/Host/windows/ProcessRunLock.cpp
@@ -10,16 +10,6 @@
 #include "lldb/Host/ProcessRunLock.h"
 #include "lldb/Host/windows/windows.h"
 
-namespace {
-#if defined(__MINGW32__)
-// Taken from WinNT.h
-typedef struct _RTL_SRWLOCK { PVOID Ptr; } RTL_SRWLOCK, *PRTL_SRWLOCK;
-
-// Taken from WinBase.h
-typedef RTL_SRWLOCK SRWLOCK, *PSRWLOCK;
-#endif
-}
-
 static PSRWLOCK GetLock(lldb::rwlock_t lock) {
   return static_cast(lock);
 }
Index: source/Host/windows/HostProcessWindows.cpp
===
--- source/Host/windows/HostProcessWindows.cpp
+++ source/Host/windows/HostProcessWindows.cpp
@@ -16,7 +16,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ConvertUTF.h"
 
-#include 
+#include 
 
 using namespace lldb_private;
 
Index: source/Host/windows/Host.cpp
===
--- source/Host/windows/Host.cpp
+++ source/Host/windows/Host.cpp
@@ -29,7 +29,7 @@
 #include "llvm/Support/ConvertUTF.h"
 
 // Windows includes
-#include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
Index: source/Host/common/File.cpp
===
--- source/Host/common/File.cpp
+++ source/Host/common/File.cpp
@@ -222,7 +222

[Lldb-commits] [PATCH] D27759: Fix build for mingw.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'll leave Zachary to approve, but it seems reasonable.

BTW, I am curious, are you planning on building python with mingw as well? If 
you are, I'd be interested in knowing how that works out.




Comment at: cmake/modules/LLDBConfig.cmake:246
+if (MSVC)
+add_definitions( /D _UNICODE /D UNICODE )
+elseif (MINGW)

Could you check if it's enough to pass `-DFOO` regardless of the platform. This 
is the only usage of the `/D` version in the llvm codebase, which leads me to 
believe that msvc will accept the `-D` version as well (or cmake will somehow 
convert in for him.



Comment at: source/Core/Mangled.cpp:12
 #include 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 #include "lldb/Host/windows/windows.h"

Should we use LLVM_ON_WIN32 here? That seems to be the llvm preffered 
solution...


https://reviews.llvm.org/D27759



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D27759: Fix build for mingw.

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: cmake/modules/LLDBConfig.cmake:246
+if (MSVC)
+add_definitions( /D _UNICODE /D UNICODE )
+elseif (MINGW)

labath wrote:
> Could you check if it's enough to pass `-DFOO` regardless of the platform. 
> This is the only usage of the `/D` version in the llvm codebase, which leads 
> me to believe that msvc will accept the `-D` version as well (or cmake will 
> somehow convert in for him.
I think the -D syntax works on both, so I would change this back to the 
original condition, and just change the / to a -

For example, right above this line you can see things like `-wd4150`, and right 
below this line you can see things like `-D_LARGEFILE_SOURCE`


https://reviews.llvm.org/D27759



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D27759: Fix build for mingw.

[Hafiz Abid Qadeer via Phabricator via lldb-commits]

abidh added a comment.

> BTW, I am curious, are you planning on building python with mingw as well? If 
> you are, I'd be interested in knowing how that works out.

I would like to do it but have not done it yet. I will keep you posted.




Comment at: cmake/modules/LLDBConfig.cmake:246
+if (MSVC)
+add_definitions( /D _UNICODE /D UNICODE )
+elseif (MINGW)

zturner wrote:
> labath wrote:
> > Could you check if it's enough to pass `-DFOO` regardless of the platform. 
> > This is the only usage of the `/D` version in the llvm codebase, which 
> > leads me to believe that msvc will accept the `-D` version as well (or 
> > cmake will somehow convert in for him.
> I think the -D syntax works on both, so I would change this back to the 
> original condition, and just change the / to a -
> 
> For example, right above this line you can see things like `-wd4150`, and 
> right below this line you can see things like `-D_LARGEFILE_SOURCE`
OK. I will change it accordingly.



Comment at: source/Core/Mangled.cpp:12
 #include 
-#if defined(_MSC_VER)
+#if defined(_WIN32)
 #include "lldb/Host/windows/windows.h"

labath wrote:
> Should we use LLVM_ON_WIN32 here? That seems to be the llvm preffered 
> solution...
Do you mean use LLVM_ON_WIN32 everywhere instead of _WIN32 or just here?


https://reviews.llvm.org/D27759



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D27759: Fix build for mingw.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yeah, I meant using it everywhere as a generic "am I on windows, regardless of 
the compiler" check. (Assuming Zach is fine with that)


https://reviews.llvm.org/D27759



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r289695 - Test num locations >= 1 not == 1.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Wed Dec 14 13:35:56 2016
New Revision: 289695

URL: http://llvm.org/viewvc/llvm-project?rev=289695&view=rev
Log:
Test num locations >= 1 not == 1.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py?rev=289695&r1=289694&r2=289695&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
 Wed Dec 14 13:35:56 2016
@@ -40,7 +40,7 @@ class AddressBreakpointTestCase(TestBase
 breakpoint = target.BreakpointCreateBySourceRegex(
 "Set a breakpoint here", lldb.SBFileSpec("main.c"))
 self.assertTrue(breakpoint and
-breakpoint.GetNumLocations() == 1,
+breakpoint.GetNumLocations() >= 1,
 VALID_BREAKPOINT)
 
 # Get the breakpoint location from breakpoint after we verified that,


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r289711 - Adopt PrettyStackTrace in LLDB

[Sean Callanan via lldb-commits]

Author: spyffe
Date: Wed Dec 14 15:31:31 2016
New Revision: 289711

URL: http://llvm.org/viewvc/llvm-project?rev=289711&view=rev
Log:
Adopt PrettyStackTrace in LLDB

LLDB needs some minor changes to adopt PrettyStackTrace after 
https://reviews.llvm.org/D27683.
We remove our own SetCrashDescription() function and use LLVM-provided RAII 
objects instead.
We also make sure LLDB doesn't define __crashtracer_info__ which would collide 
with LLVM's definition.

Differential Revision: https://reviews.llvm.org/D27735

Modified:
lldb/trunk/include/lldb/Host/Host.h
lldb/trunk/scripts/Xcode/build-llvm.py
lldb/trunk/source/API/SBFrame.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Host/common/Host.cpp
lldb/trunk/source/Host/macosx/Host.mm
lldb/trunk/source/Initialization/SystemInitializerCommon.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/include/lldb/Host/Host.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/Host.h?rev=289711&r1=289710&r2=289711&view=diff
==
--- lldb/trunk/include/lldb/Host/Host.h (original)
+++ lldb/trunk/include/lldb/Host/Host.h Wed Dec 14 15:31:31 2016
@@ -183,23 +183,6 @@ public:
   //--
   static bool ResolveExecutableInBundle(FileSpec &file);
 
-  //--
-  /// Set a string that can be displayed if host application crashes.
-  ///
-  /// Some operating systems have the ability to print a description
-  /// for shared libraries when a program crashes. If the host OS
-  /// supports such a mechanism, it should be implemented to help
-  /// with crash triage.
-  ///
-  /// @param[in] format
-  /// A printf format that will be used to form a new crash
-  /// description string.
-  //--
-  static void SetCrashDescriptionWithFormat(const char *format, ...)
-  __attribute__((format(printf, 1, 2)));
-
-  static void SetCrashDescription(const char *description);
-
   static uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
 ProcessInstanceInfoList &proc_infos);
 

Modified: lldb/trunk/scripts/Xcode/build-llvm.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Xcode/build-llvm.py?rev=289711&r1=289710&r2=289711&view=diff
==
--- lldb/trunk/scripts/Xcode/build-llvm.py (original)
+++ lldb/trunk/scripts/Xcode/build-llvm.py Wed Dec 14 15:31:31 2016
@@ -346,7 +346,8 @@ def cmake_flags():
 "-DCMAKE_C_FLAGS={}".format(get_c_flags()),
 "-DCMAKE_CXX_FLAGS={}".format(get_cxx_flags()),
 
"-DCMAKE_EXE_LINKER_FLAGS={}".format(get_exe_linker_flags()),
-
"-DCMAKE_SHARED_LINKER_FLAGS={}".format(get_shared_linker_flags())]
+
"-DCMAKE_SHARED_LINKER_FLAGS={}".format(get_shared_linker_flags()),
+"-DHAVE_CRASHREPORTER_INFO=1"]
 deployment_target = get_deployment_target()
 if deployment_target:
 cmake_flags.append(

Modified: lldb/trunk/source/API/SBFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFrame.cpp?rev=289711&r1=289710&r2=289711&view=diff
==
--- lldb/trunk/source/API/SBFrame.cpp (original)
+++ lldb/trunk/source/API/SBFrame.cpp Wed Dec 14 15:31:31 2016
@@ -52,6 +52,8 @@
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBVariablesOptions.h"
 
+#include "llvm/Support/PrettyStackTrace.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -1288,10 +1290,11 @@ lldb::SBValue SBFrame::EvaluateExpressio
 if (stop_locker.TryLock(&process->GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
   if (frame) {
+std::unique_ptr PST;
 if (target->GetDisplayExpressionsInCrashlogs()) {
   StreamString frame_description;
   frame->DumpUsingSettingsFormat(&frame_description);
-  Host::SetCrashDescriptionWithFormat(
+  PST = llvm::make_unique(
   "SBFrame::EvaluateExpression (expr = \"%s\", fetch_dynamic_value 
"
   "= %u) %s",
   expr, options.GetFetchDynamicValue(),
@@ -1301,9 +1304,6 @@ lldb::SBValue SBFrame::EvaluateExpressio
 exe_results = target->EvaluateExpression(expr, frame, expr_value_sp,
  options.ref());
 expr_result.SetSP(expr_value_sp, options.GetFetchDynamicValue());
-
-if (target->GetDisplayExpressionsInCrashlogs())
-  Host::SetCrashDescription(nullptr);
   } else {
 if (log)
   log->Printf("SBFrame::EvaluateEx

[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef

[Zachary Turner via Phabricator via lldb-commits]

zturner created this revision.
zturner added reviewers: clayborg, labath.
zturner added a subscriber: lldb-commits.

The major blocker to this until now has been that we can't have global 
constructors, no matter how trivial.

Recently LLVM obtained the `StringLiteral` class which addresses this.  This 
class has a `constexpr` constructor, so as long as a global `StringRef` (or 
class/struct containing a `StringRef` is declared `constexpr`, the `StringRef` 
can be initialized with a `StringLiteral` and not invoke a global constructor.  
This probably sounds confusing, but in practice it's simple.  It looks like 
this:

  // ok
  constexpr StringRef S = llvm::StringLiteral("foo");
  
  // ok
  constexpr StringLiteral T("foo");
  
  // not ok, global constructor invoked.
  StringLiteral U("foo");
  
  struct Foo {
constexpr Foo(StringRef S) : S(S) {}
StringRef S;
  };
  
  // ok
  constexpr Foo F(llvm::StringLiteral("foo"));

Admittedly, it's pretty verbose to type out `StringLiteral` everywhere.  To get 
around this, I alias it to `L` in the individual TUs where we use it.  This way 
we can write:

  using L = llvm::StringLiteral;
  static constexpr OptionDefinition[] g_defs = { ..., ..., L("opt"), ..., ..., 
L("help text") };

Again, note that no global constructor is invoked here, even though this is an 
array.

The advantage of doing all of this is that we can now have access to all the 
wonderful member functions of `StringRef`.  Upon fixing up all the compilation 
failures (there were very few however) triggered by the change, we can already 
see some code being simplified, and in the future I would like to extend this 
to other classes like `RegisterInfo` etc as well.

Note that this patch is incomplete as is.  Before submitting, I would need to 
go change EVERY global definition of `OptionDefinition` to be marked 
`constexpr`, otherwise a global constructor would be invoked.  I intentionally 
did it in only 2 places here, just to illustrate the idea, before changing the 
code everywhere.


https://reviews.llvm.org/D27780

Files:
  lldb/include/lldb/Interpreter/Options.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Host/common/OptionParser.cpp
  lldb/source/Interpreter/Args.cpp
  lldb/source/Interpreter/OptionGroupOutputFile.cpp
  lldb/source/Interpreter/Options.cpp

Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -333,27 +333,14 @@
   }
 }
 
-bool Options::SupportsLongOption(const char *long_option) {
-  if (!long_option || !long_option[0])
+bool Options::SupportsLongOption(llvm::StringRef long_option) {
+  long_option.consume_front("--");
+  if (long_option.empty())
 return false;
 
-  auto opt_defs = GetDefinitions();
-  if (opt_defs.empty())
-return false;
-
-  const char *long_option_name = long_option;
-  if (long_option[0] == '-' && long_option[1] == '-')
-long_option_name += 2;
-
-  for (auto &def : opt_defs) {
-if (!def.long_option)
-  continue;
-
-if (strcmp(def.long_option, long_option_name) == 0)
-  return true;
-  }
-
-  return false;
+  return llvm::any_of(GetDefinitions(), [=](const OptionDefinition &D) {
+return D.long_option == long_option;
+  });
 }
 
 enum OptionDisplayType {
@@ -603,9 +590,10 @@
 
   strm.IndentMore(5);
 
-  if (opt_defs[i].usage_text)
+  if (!opt_defs[i].usage_text.empty())
 OutputFormattedUsageText(strm, opt_defs[i], screen_width);
-  if (opt_defs[i].enum_values != nullptr) {
+
+  if (opt_defs[i].enum_values == nullptr) {
 strm.Indent();
 strm.Printf("Values: ");
 for (int k = 0; opt_defs[i].enum_values[k].string_value != nullptr;
@@ -670,10 +658,6 @@
 
   auto opt_defs = GetDefinitions();
 
-  std::string cur_opt_std_str(input.GetArgumentAtIndex(cursor_index));
-  cur_opt_std_str.erase(char_pos);
-  const char *cur_opt_str = cur_opt_std_str.c_str();
-
   for (size_t i = 0; i < opt_element_vector.size(); i++) {
 int opt_pos = opt_element_vector[i].opt_pos;
 int opt_arg_pos = opt_element_vector[i].opt_arg_pos;
@@ -708,18 +692,17 @@
 }
 return true;
   } else if (opt_defs_index != OptionArgElement::eUnrecognizedArg) {
+auto cur_opt_str = input[cursor_index].ref.take_front(char_pos);
+
 // We recognized it, if it an incomplete long option, complete it anyway
-// (getopt_long_only is
-// happy with shortest unique string, but it's still a nice thing to
-// do.)  Otherwise return
-// The string so the upper level code will know this is a full match and
-// add the " ".
-if (cur_opt_str && strlen(cur_opt_str) > 2 && cur_opt_str[0] == '-' &&
-cur_opt_str[1] == '-' &&
-strcmp(opt_defs[opt_defs_index].long_option, cur_opt_str) != 0) {
+// (getopt_long_only is

[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

There seems to be a bunch of places that we are passing a string literal to 
functions that take a StringRef. Can we pass L("--") instead of "--" to 
functions that take a string ref and have it be more efficient so a StringRef 
isn't implicitly constructed for us each time? See inlined comments for places.




Comment at: lldb/source/Interpreter/Args.cpp:954
+  llvm::SmallString<3> short_buffer =
+  llvm::formatv("-{0}", (char)long_options[long_options_index].val);
+  llvm::SmallString<255> long_buffer = llvm::formatv(

Will this line cause a StringRef constructor to be called on "-{0}"? If so, is 
there a way to avoid this with some constexpr magic?



Comment at: lldb/source/Interpreter/Args.cpp:956
+  llvm::SmallString<255> long_buffer = llvm::formatv(
+  "--{0}", long_options[long_options_index].definition->long_option);
 

Ditto



Comment at: lldb/source/Interpreter/Options.cpp:337
+bool Options::SupportsLongOption(llvm::StringRef long_option) {
+  long_option.consume_front("--");
+  if (long_option.empty())

Will this line cause a StringRef constructor to be called on "--"?



Comment at: lldb/source/Interpreter/Options.cpp:701
+// level code will know this is a full match and add the " ".
+if (cur_opt_str.startswith("--") &&
+cur_opt_str.drop_front(2) != opt_defs[opt_defs_index].long_option) 
{

Will this line cause a StringRef constructor to be called on "--"?



Comment at: lldb/source/Interpreter/Options.cpp:719
 
-if (strstr(def.long_option, cur_opt_str + 2) == def.long_option) {
-  std::string full_name("--");
-  full_name.append(def.long_option);
-  // The options definitions table has duplicates because of the
-  // way the grouping information is stored, so only add once.
-  bool duplicate = false;
-  for (size_t k = 0; k < matches.GetSize(); k++) {
-if (matches.GetStringAtIndex(k) == full_name) {
-  duplicate = true;
-  break;
-}
-  }
-  if (!duplicate)
-matches.AppendString(full_name.c_str());
+if (!cur_opt_str.consume_front("--"))
+  return true;

Will this line cause a StringRef constructor to be called on "--"?



Comment at: lldb/source/Interpreter/Options.cpp:728
+for (auto &def : range) {
+  std::string full_name("--");
+  full_name.append(def.long_option);

Do we still need std::string here for full_name? We might be able to do smarter 
things with StringRef for all uses of full_name below, including the 
matches.GetStringAtIndex() by seeing if the string at index starts with "--", 
and then just comparing the remainder to "def.long_option"?



Comment at: lldb/source/Interpreter/Options.cpp:834
   // restrict it to that shared library.
-  if (cur_opt_name && strcmp(cur_opt_name, "shlib") == 0 &&
-  cur_arg_pos != -1) {
-const char *module_name = input.GetArgumentAtIndex(cur_arg_pos);
-if (module_name) {
+  if (cur_opt_name == "shlib" && cur_arg_pos != -1) {
+auto module_name = input[cur_arg_pos].ref;

Will this line cause a StringRef constructor to be called on "shlib"?


https://reviews.llvm.org/D27780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r289746 - Fix incorrectly named variables.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Wed Dec 14 18:30:30 2016
New Revision: 289746

URL: http://llvm.org/viewvc/llvm-project?rev=289746&view=rev
Log:
Fix incorrectly named variables.

Modified:
lldb/trunk/source/API/SBFrame.cpp
lldb/trunk/source/API/SBTarget.cpp
lldb/trunk/source/Interpreter/CommandInterpreter.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Modified: lldb/trunk/source/API/SBFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFrame.cpp?rev=289746&r1=289745&r2=289746&view=diff
==
--- lldb/trunk/source/API/SBFrame.cpp (original)
+++ lldb/trunk/source/API/SBFrame.cpp Wed Dec 14 18:30:30 2016
@@ -1290,11 +1290,11 @@ lldb::SBValue SBFrame::EvaluateExpressio
 if (stop_locker.TryLock(&process->GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
   if (frame) {
-std::unique_ptr PST;
+std::unique_ptr stack_trace;
 if (target->GetDisplayExpressionsInCrashlogs()) {
   StreamString frame_description;
   frame->DumpUsingSettingsFormat(&frame_description);
-  PST = llvm::make_unique(
+  stack_trace = llvm::make_unique(
   "SBFrame::EvaluateExpression (expr = \"%s\", fetch_dynamic_value 
"
   "= %u) %s",
   expr, options.GetFetchDynamicValue(),

Modified: lldb/trunk/source/API/SBTarget.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBTarget.cpp?rev=289746&r1=289745&r2=289746&view=diff
==
--- lldb/trunk/source/API/SBTarget.cpp (original)
+++ lldb/trunk/source/API/SBTarget.cpp Wed Dec 14 18:30:30 2016
@@ -2130,7 +2130,7 @@ lldb::SBValue SBTarget::EvaluateExpressi
   StreamString frame_description;
   if (frame)
 frame->DumpUsingSettingsFormat(&frame_description);
-  llvm::PrettyStackTraceFormat PST(
+  llvm::PrettyStackTraceFormat stack_trace(
   "SBTarget::EvaluateExpression (expr = \"%s\", fetch_dynamic_value = "
   "%u) %s",
   expr, options.GetFetchDynamicValue(),

Modified: lldb/trunk/source/Interpreter/CommandInterpreter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandInterpreter.cpp?rev=289746&r1=289745&r2=289746&view=diff
==
--- lldb/trunk/source/Interpreter/CommandInterpreter.cpp (original)
+++ lldb/trunk/source/Interpreter/CommandInterpreter.cpp Wed Dec 14 18:30:30 
2016
@@ -1527,7 +1527,7 @@ bool CommandInterpreter::HandleCommand(c
   std::string original_command_string(command_line);
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_COMMANDS));
-  llvm::PrettyStackTraceFormat PST("HandleCommand(command = \"%s\")",
+  llvm::PrettyStackTraceFormat stack_trace("HandleCommand(command = \"%s\")",
command_line);
 
   if (log)

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=289746&r1=289745&r2=289746&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Wed Dec 
14 18:30:30 2016
@@ -1473,7 +1473,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
 }
 
 if (add_method) {
-  llvm::PrettyStackTraceFormat PST(
+  llvm::PrettyStackTraceFormat stack_trace(
   "SymbolFileDWARF::ParseType() is adding a method 
"
   "%s to class %s in DIE 0x%8.8" PRIx64 " from %s",
   type_name_cstr,


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

In response to all the questions about "Will a StringRef constructor be called 
on XXX", the answer is usually yes, but the constructor will be inlined and 
invoke `__builtin_strlen` (which is constexpr on GCC and Clang) when used on a 
string literal.  In other words, there is zero overhead.

So the code generated for `S.startswith("--")` will end up just being `(2 >= 
S.Length) && (::memcmp(S.Data, "--", 2) == 0)`.

The only time we have to worry about constexprness and make sure we use 
`StringLiteral` is with static variables (global or local).  With static 
variables, the compiler has to decide between static link-time initialization 
or runtime initialization, and only if you declare the variable as `constexpr` 
will it choose link-time initialization.  So the generated code for these two 
might look something like this:

  // No generated code, S is initialized at link-time.
  constexpr StringLiteral S("foo");   
  
  
  // mov , 
  // mov , 3
  StringLiteral T("foo");


https://reviews.llvm.org/D27780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D27780: Make OptionDefinition structure store a StringRef

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: lldb/source/Interpreter/Options.cpp:728
+for (auto &def : range) {
+  std::string full_name("--");
+  full_name.append(def.long_option);

clayborg wrote:
> Do we still need std::string here for full_name? We might be able to do 
> smarter things with StringRef for all uses of full_name below, including the 
> matches.GetStringAtIndex() by seeing if the string at index starts with "--", 
> and then just comparing the remainder to "def.long_option"?
Yes, I tried that at first, but soon after noticed that `matches` is an output 
parameter, so we would have to fix up the caller to stop making assumptions 
that the `--` is tacked onto the beginning.  Certainly doable, but it has 
potential for growing into a large CL depending on how far down the rabbit hole 
we'd have to go to fix everything.


https://reviews.llvm.org/D27780



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r289711 - Adopt PrettyStackTrace in LLDB

[Zachary Turner via lldb-commits]

On Wed, Dec 14, 2016 at 1:41 PM Sean Callanan via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

>
> Modified: lldb/trunk/source/API/SBFrame.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFrame.cpp?rev=289711&r1=289710&r2=289711&view=diff
>
> ==
> --- lldb/trunk/source/API/SBFrame.cpp (original)
> +++ lldb/trunk/source/API/SBFrame.cpp Wed Dec 14 15:31:31 2016
> @@ -52,6 +52,8 @@
>  #include "lldb/API/SBValue.h"
>  #include "lldb/API/SBVariablesOptions.h"
>
> +#include "llvm/Support/PrettyStackTrace.h"
> +
>  using namespace lldb;
>  using namespace lldb_private;
>
> @@ -1288,10 +1290,11 @@ lldb::SBValue SBFrame::EvaluateExpressio
>  if (stop_locker.TryLock(&process->GetRunLock())) {
>frame = exe_ctx.GetFramePtr();
>if (frame) {
> +std::unique_ptr PST;
>
Minor nit, but I would move this to the most narrow scope possible that
requires its use.  which in this case means to move the declaration inside
the next if statement.


>  if (target->GetDisplayExpressionsInCrashlogs()) {
>StreamString frame_description;
>frame->DumpUsingSettingsFormat(&frame_description);
> -  Host::SetCrashDescriptionWithFormat(
> +  PST = llvm::make_unique(
>"SBFrame::EvaluateExpression (expr = \"%s\",
> fetch_dynamic_value "
>"= %u) %s",
>expr, options.GetFetchDynamicValue(),
>
If you do the above, then you could declare the variable as `auto` here.
Normally auto is frowned upon, but this is an exception since
`make_unique` is a clear statement of the return
type.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits