Re: [Lldb-commits] [PATCH] D13404: Teach CMake to find versions of Python != 2.7

2015-10-03 Thread Vadim Macagon via lldb-commits 
enlight added inline comments.


Comment at: cmake/modules/LLDBConfig.cmake:109
@@ +108,3 @@
+  set (PYTHON_EXECUTABLE 
$<$:${PYTHON_DEBUG_EXE}>$<$>:${PYTHON_RELEASE_EXE}>
 PARENT_SCOPE)
+  set (PYTHON_LIBRARY 
$<$:${PYTHON_DEBUG_LIB}>$<$>:${PYTHON_RELEASE_LIB}>
 PARENT_SCOPE)
+  set (PYTHON_DLL 
$<$:${PYTHON_DEBUG_DLL}>$<$>:${PYTHON_RELEASE_DLL}>
 PARENT_SCOPE)

PYTHON_LIBRARY is used in this scope later on, so it needs to be explicitly set 
for both this scope and the parent scope, like so:

```
set (PYTHON_LIBRARY 
$<$:${PYTHON_DEBUG_LIB}>$<$>:${PYTHON_RELEASE_LIB}>)
set (PYTHON_LIBRARY ${PYTHON_LIBRARY} PARENT_SCOPE)
```



Comment at: cmake/modules/LLDBConfig.cmake:112-114
@@ +111,5 @@
+
+  if (NOT LLDB_RELOCATABLE_PYTHON)
+add_definitions( -DLLDB_PYTHON_HOME="${PYTHON_HOME}" )
+  endif()
+

I think this belongs outside this function, along with `include_directories` 
below.


Comment at: cmake/modules/LLDBConfig.cmake:117
@@ +116,3 @@
+  if (PYTHON_LIBRARY)
+include_directories(${PYTHON_INCLUDE_DIR})
+  endif()

This command will never be executed because PYTHON_LIBRARY is only set in the 
parent scope. However, I'd prefer `include_directories()` wasn't here at all, 
let the caller of `find_python_libs_windows` do that instead so that the 
behavior is more similar to the built-in [[ 
https://cmake.org/cmake/help/v2.8.12/cmake.html#module:FindPythonLibs | 
FindPythonLibs ]] module (which means `PYTHON_INCLUDE_DIRS` should be made 
available in the parent scope, note the extra `S`).


Comment at: cmake/modules/LLDBConfig.cmake:131-133
@@ -104,1 +130,5 @@
+find_python_libs_windows()
+message("-- Found PythonExecutable: ${PYTHON_EXECUTABLE}")
+message("-- Found PythonLibs: ${PYTHON_LIBRARY}")
+message("-- Found PythonDLL: ${PYTHON_DLL}")
   else()

Please move these into `find_python_libs_windows()`, as I'll need access to the 
debug/release paths in order to avoid printing out the generator expressions.


Comment at: cmake/modules/LLDBStandalone.cmake:51-53
@@ -50,5 +50,5 @@
 
   # Verify that we can find a Python 2 interpreter.  Python 3 is unsupported.
   if (PYTHON_EXECUTABLE STREQUAL "")
-set(Python_ADDITIONAL_VERSIONS 2.7 2.6 2.5)
+set(Python_ADDITIONAL_VERSIONS 3.5 3.4 3.3 3.2 3.1 3.0 2.7 2.6 2.5)
 include(FindPythonInterp)

The comment no longer seems accurate.


http://reviews.llvm.org/D13404



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

Re: [Lldb-commits] [PATCH] D13404: Teach CMake to find versions of Python != 2.7

2015-10-03 Thread Vadim Macagon via lldb-commits 
enlight added inline comments.


Comment at: cmake/modules/LLDBConfig.cmake:56
@@ +55,3 @@
+message("LLDB embedded Python on Windows requires specifying a value for 
PYTHON_HOME.  Python support disabled.")
+set(LLDB_DISABLE_PYTHON 1)
+return()

This will only set `LLDB_DISABLE_PYTHON` inside the function, you probably want 
to do `set (LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)`, or `set (LLDB_DISABLE_PYTHON 
1 CACHE INTERNAL "")` instead.


Comment at: cmake/modules/LLDBConfig.cmake:73
@@ +72,3 @@
+message("Python support will be disabled for this build.")
+set(LLDB_DISABLE_PYTHON 1)
+return()

Ditto.


http://reviews.llvm.org/D13404



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

[Lldb-commits] [lldb] r249233 - Add PersistentVariableDelegate to handle language-specific dematerialization.

2015-10-03 Thread Sean Callanan via lldb-commits 
Author: spyffe
Date: Sat Oct  3 04:09:01 2015
New Revision: 249233

URL: http://llvm.org/viewvc/llvm-project?rev=249233&view=rev
Log:
Add PersistentVariableDelegate to handle language-specific dematerialization.

The concept here is that languages may have different ways of communicating
results.  In particular, languages may have different names for their result
variables and in fact may have multiple types of result variables (e.g.,
error results).  Materializer was tied to one specific model of result handling.

Instead, now UserExpressions can register their own handlers for the result
variables they inject.  This allows language-specific code in Materializer to
be moved into the expression parser plug-in, and it simplifies Materializer.
These delegates are subclasses of PersistentVariableDelegate.

PersistentVariableDelegate can provide the name of the result variable, and is
notified when the result variable is populated.  It can also be used to touch
persistent variables if need be, updating language-specific state.  The
UserExpression owns the delegate and can decide on its result based on
consulting all of its (potentially multiple) delegates.

The user expression itself now makes the determination of what the final result
of the expression is, rather than relying on the Materializer, and I've added a
virtual function to UserExpression to allow this.

Modified:
lldb/trunk/include/lldb/Expression/Materializer.h
lldb/trunk/include/lldb/Expression/UserExpression.h
lldb/trunk/source/Expression/Materializer.cpp
lldb/trunk/source/Expression/UserExpression.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp

Modified: lldb/trunk/include/lldb/Expression/Materializer.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/Materializer.h?rev=249233&r1=249232&r2=249233&view=diff
==
--- lldb/trunk/include/lldb/Expression/Materializer.h (original)
+++ lldb/trunk/include/lldb/Expression/Materializer.h Sat Oct  3 04:09:01 2015
@@ -44,7 +44,6 @@ public:
 }
 
 void Dematerialize (Error &err,
-lldb::ExpressionVariableSP &result_sp,
 lldb::addr_t frame_top,
 lldb::addr_t frame_bottom);
 
@@ -84,11 +83,28 @@ public:
 
 DematerializerSP Materialize (lldb::StackFrameSP &frame_sp, IRMemoryMap 
&map, lldb::addr_t process_address, Error &err);
 
-uint32_t AddPersistentVariable (lldb::ExpressionVariableSP 
&persistent_variable_sp, Error &err);
-uint32_t AddVariable (lldb::VariableSP &variable_sp, Error &err);
-uint32_t AddResultVariable (const CompilerType &type, bool is_lvalue, bool 
keep_in_memory, Error &err);
-uint32_t AddSymbol (const Symbol &symbol_sp, Error &err);
-uint32_t AddRegister (const RegisterInfo ®ister_info, Error &err);
+class PersistentVariableDelegate
+{
+public:
+virtual ~PersistentVariableDelegate();
+virtual ConstString GetName() = 0;
+virtual void DidDematerialize(lldb::ExpressionVariableSP &variable) = 
0;
+};
+
+uint32_t AddPersistentVariable (lldb::ExpressionVariableSP 
&persistent_variable_sp,
+PersistentVariableDelegate *delegate,
+Error &err);
+uint32_t AddVariable (lldb::VariableSP &variable_sp,
+  Error &err);
+uint32_t AddResultVariable (const CompilerType &type,
+bool is_lvalue,
+bool keep_in_memory,
+PersistentVariableDelegate *delegate,
+Error &err);
+uint32_t AddSymbol (const Symbol &symbol_sp,
+Error &err);
+uint32_t AddRegister (const RegisterInfo ®ister_info,
+  Error &err);
 
 uint32_t GetStructAlignment ()
 {
@@ -100,14 +116,6 @@ public:
 return m_current_offset;
 }
 
-uint32_t GetResultOffset ()
-{
-if (m_result_entity)
-return m_result_entity->GetOffset();
-else
-return UINT32_MAX;
-}
-
 class Entity
 {
 public:
@@ -163,7 +171,6 @@ private:
 
 DematerializerWPm_dematerializer_wp;
 EntityVectorm_entities;
-Entity *m_result_entity;
 uint32_tm_current_offset;
 uint32_tm_struct_alignment;
 };

Modified: lldb/trunk/include/lldb/Expression/UserE

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

2015-10-03 Thread Todd Fiala via lldb-commits 
tfiala added a comment.

Starting to look at this now.

The code looks reasonable, trying now.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

2015-10-03 Thread Todd Fiala via lldb-commits 
tfiala added a comment.

You'll need to guard against exclusion of libedit:

  #ifndef LLDB_DISABLE_LIBEDIT
  ...
  #endif

We can't exclude this whole file if libedit is disabled, because if we don't 
have libedit, we still need this null stub in on Linux. (i.e. the existing 
behavior needs to be present if no libedit).


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

2015-10-03 Thread Todd Fiala via lldb-commits 
tfiala added a comment.

I'm adjusting the patch for optional libedit.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

2015-10-03 Thread Todd Fiala via lldb-commits 
tfiala added a comment.

I'm still testing, but this seems to do the trick:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 55fdf77..dd2c440 100644

- a/CMakeLists.txt

+++ b/CMakeLists.txt
@@ -4,7 +4,14 @@ include(cmake/modules/LLDBStandalone.cmake)
 include(cmake/modules/LLDBConfig.cmake)
 include(cmake/modules/AddLLDB.cmake)

-#add_subdirectory(include)
+# We need libedit support to go down both the source and
+# the scripts directories.
+set(LLDB_DISABLE_LIBEDIT ${LLDB_DEFAULT_DISABLE_LIBEDIT} CACHE BOOL "Disables 
the use of editline.")
+if (LLDB_DISABLE_LIBEDIT)
+  add_definitions( -DLLDB_DISABLE_LIBEDIT )
+endif()
+
+# add_subdirectory(include)
 add_subdirectory(docs)
 if (NOT LLDB_DISABLE_PYTHON)

  add_subdirectory(scripts)

diff --git a/scripts/Python/modules/readline/CMakeLists.txt 
b/scripts/Python/modules/readline/CMakeLists.txt
index 11089c6..0a4376c 100644

- a/scripts/Python/modules/readline/CMakeLists.txt

+++ b/scripts/Python/modules/readline/CMakeLists.txt
@@ -7,7 +7,11 @@ SET(PYTHON_DIRECTORY 
python${PYTHON_VERSION_MAJOR}.${PYTHON_VERSION_MINOR}/site-
 include_directories(${PYTHON_INCLUDE_DIR})
 add_library(readline SHARED readline.cpp)

-target_link_libraries(readline ${PYTHON_LIBRARY})
+if (NOT LLDB_DISABLE_LIBEDIT)
+  target_link_libraries(readline ${PYTHON_LIBRARY} edit)
+else()
+  target_link_libraries(readline ${PYTHON_LIBRARY})
+endif()

1. FIXME: the LIBRARY_OUTPUT_PATH seems to be ignored - this is not a
2. functional issue for the build dir, though, since the shared lib dir

diff --git a/scripts/Python/modules/readline/readline.cpp 
b/scripts/Python/modules/readline/readline.cpp
index b0d6b74..38268f8 100644

- a/scripts/Python/modules/readline/readline.cpp

+++ b/scripts/Python/modules/readline/readline.cpp
@@ -1,23 +1,68 @@
 #include 
 #include "Python.h"

-// Python readline module intentionally built to not implement the
-// readline module interface. This is meant to work around llvm
-// pr18841 to avoid seg faults in the stock Python readline.so linked
-// against GNU readline.
+#ifndef LLDB_DISABLE_LIBEDIT
+#include 
+#endif
+
+// Simple implementation of the Python readline module using libedit.
+// In the event that libedit is excluded from the build, this turns
+// back into a null implementation that blocks the module from pulling
+// in the GNU readline shared lib, which causes linkage confusion when
+// both readline and libedit's readline compatibility symbols collide.
+//
+// Currently it only installs a PyOS_ReadlineFunctionPointer, without
+// implementing any of the readline module methods. This is meant to
+// work around LLVM pr18841 to avoid seg faults in the stock Python
+// readline.so linked against GNU readline.

static struct PyMethodDef moduleMethods[] =
 {

  {nullptr, nullptr, 0, nullptr}

};

+#ifndef LLDB_DISABLE_LIBEDIT
+PyDoc_STRVAR(
+moduleDocumentation,
+"Simple readline module implementation based on libedit.");
+#else
 PyDoc_STRVAR(

  moduleDocumentation,

- "Stub module meant to effectively disable readline support.");

+"Stub module meant to avoid linking GNU readline.");
+#endif
+
+#ifndef LLDB_DISABLE_LIBEDIT
+static char*
+simple_readline(FILE *stdin, FILE *stdout, char *prompt)
+{
+rl_instream = stdin;
+rl_outstream = stdout;
+char* line = readline(prompt);
+if (!line)
+{
+char* ret = (char*)PyMem_Malloc(1);
+if (ret != NULL)
+*ret = '\0';
+return ret;
+}
+if (*line)
+add_history(line);
+int n = strlen(line);
+char* ret = (char*)PyMem_Malloc(n + 2);
+strncpy(ret, line, n);
+free(line);
+ret[n] = '\n';
+ret[n+1] = '\0';
+return ret;
+}
+#endif

PyMODINIT_FUNC
 initreadline(void)
 {
+#ifndef LLDB_DISABLE_LIBEDIT
+PyOS_ReadlineFunctionPointer = simple_readline;
+#endif

  Py_InitModule4(
  "readline",
  moduleMethods,

diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
index 0470e51..8e85498 100644

- a/source/CMakeLists.txt

+++ b/source/CMakeLists.txt
@@ -6,11 +6,6 @@ else()

  set(LLDB_DEFAULT_DISABLE_LIBEDIT 0)

endif ()

-set(LLDB_DISABLE_LIBEDIT ${LLDB_DEFAULT_DISABLE_LIBEDIT} CACHE BOOL "Disables 
the use of editline.")
-if (LLDB_DISABLE_LIBEDIT)

- add_definitions( -DLLDB_DISABLE_LIBEDIT )

-endif()

- if ( CMAKE_SYSTEM_NAME MATCHES "Linux" ) include_directories( 
Plugins/Process/Linux


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

2015-10-03 Thread Todd Fiala via lldb-commits 
tfiala added a comment.

Yep, that modified patch worked for both:

  cmake -DLLDB_DISABLE_LIBEDIT=1

where libedit is disabled from the build, and for the normal case where libedit 
is available.  In both cases it did the right thing - simple readline support 
with libedit and your change (minor tweaks by me solely on comments and 
conditional libedit), and for the no-libedit case (where it behaves as before, 
including the no-crash part :-) ).

If you can update the patch with that, it looks good to me.


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

2015-10-03 Thread Todd Fiala via lldb-commits 
tfiala requested changes to this revision.
tfiala added a comment.
This revision now requires changes to proceed.

(requesting changes per the previous patch and commentary)


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

Re: [Lldb-commits] [PATCH] D13268: Simple readline functionality for interactive python on linux.

2015-10-03 Thread Todd Fiala via lldb-commits 
tfiala added a comment.

(And nice work on the underlying change - with libedit it works quite nicely 
for basic line editing and history!)


Repository:
  rL LLVM

http://reviews.llvm.org/D13268



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

[Lldb-commits] [lldb] r249256 - cmake: ensure readline python module target is added before finishing swig.

2015-10-03 Thread Todd Fiala via lldb-commits 
Author: tfiala
Date: Sat Oct  3 20:28:51 2015
New Revision: 249256

URL: http://llvm.org/viewvc/llvm-project?rev=249256&view=rev
Log:
cmake: ensure readline python module target is added before finishing swig.

When the readline target exists (only for non-Android Linux currently),
ensure that target is made a dependency of the finish_swig python-wrap-up
steps.  This ensures it is built when building the lldb target.

Fixes:
https://llvm.org/bugs/show_bug.cgi?id=25038

Modified:
lldb/trunk/CMakeLists.txt

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=249256&r1=249255&r2=249256&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Sat Oct  3 20:28:51 2015
@@ -24,6 +24,12 @@ if (NOT LLDB_DISABLE_PYTHON)
 # We depend on liblldb being built before we can do this step.
 add_dependencies(finish_swig liblldb argdumper)
 
+# If we build the readline module, we depend on that happening
+# first.
+if (TARGET readline)
+add_dependencies(finish_swig readline)
+endif()
+
 # Ensure we do the python post-build step when building lldb.
 add_dependencies(lldb finish_swig)
 


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

Re: [Lldb-commits] [lldb] r249256 - cmake: ensure readline python module target is added before finishing swig.

2015-10-03 Thread Zachary Turner via lldb-commits 
You just reminded me.  Since you've been hitting a lot of this stuff lately
and doing a lot of great cleanup work, how do you feel about integrating
the swig python scripts into the Xcode build?  I've been meaning to do this
for a long time but I don't have enough Xcode knowledge.  Having parallel
scripts for Xcode and CMake introduces an obvious technical debt that we
could get rid of if there was just one script.

As far as I know it should be a drop-in replacement, and it supports
everything the shell scripts currently support.  It's been lingering long
enough that there's obviously no rush, but if you ever feel the urge to
look at it, I think it would be easy.

On Sat, Oct 3, 2015 at 6:30 PM Todd Fiala via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: tfiala
> Date: Sat Oct  3 20:28:51 2015
> New Revision: 249256
>
> URL: http://llvm.org/viewvc/llvm-project?rev=249256&view=rev
> Log:
> cmake: ensure readline python module target is added before finishing swig.
>
> When the readline target exists (only for non-Android Linux currently),
> ensure that target is made a dependency of the finish_swig python-wrap-up
> steps.  This ensures it is built when building the lldb target.
>
> Fixes:
> https://llvm.org/bugs/show_bug.cgi?id=25038
>
> Modified:
> lldb/trunk/CMakeLists.txt
>
> Modified: lldb/trunk/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=249256&r1=249255&r2=249256&view=diff
>
> ==
> --- lldb/trunk/CMakeLists.txt (original)
> +++ lldb/trunk/CMakeLists.txt Sat Oct  3 20:28:51 2015
> @@ -24,6 +24,12 @@ if (NOT LLDB_DISABLE_PYTHON)
>  # We depend on liblldb being built before we can do this step.
>  add_dependencies(finish_swig liblldb argdumper)
>
> +# If we build the readline module, we depend on that happening
> +# first.
> +if (TARGET readline)
> +add_dependencies(finish_swig readline)
> +endif()
> +
>  # Ensure we do the python post-build step when building lldb.
>  add_dependencies(lldb finish_swig)
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits