public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch] [python] Fix Python 3 build and testsuite issues
@ 2013-08-19 14:50 Phil Muldoon
  2013-08-19 16:19 ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Phil Muldoon @ 2013-08-19 14:50 UTC (permalink / raw)
  To: gdb-patches


This patch fixes up both current build and test-failures for Python 3
in GDB.  Most of the errors were mine, as introduced with the frame
filter patchset.  But while I was there I fixed up the other failures
too.

Okay?

Cheers,

Phil


2013-08-19  Phil Muldoon  <pmuldoon@redhat.com>

	* python/py-framefilter.c (py_print_frame): Remove usage of
	PyString_AsString.  Use python_string_to_host_string instead.
	Refactor function to work with a string as a new allocation
	instead of a pointer.
	(py_print_frame): Ditto.
	* python/lib/gdb/frames.py (return_list): Move "all" branch.
	Chain iterators together instead of adding them as a list.
	(_sort_list): Call return_list, and remove duplicate code.
	(execute_frame_filters): Remove len() check.  Count number of
	iterations instead.
	(execute_frame_filters): Check itertools for imap attribute.
	Otherwise use map().
	* python/lib/gdb/command/frame_filters.py
	(SetFrameFilterPriority._set_filter_priority): Convert priority
	attribute to an integer.
	* python/lib/gdb/FrameIterator.py (FrameIterator.next): Define
	wrapper function __next__.
	* python/lib/gdb/FrameDecorator.py (FrameVars.fetch_b): Reverse "if"
	test to check for a symbol first.

2013-08-19  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/py-framefilter.py (FrameFilter.filter): Check
	itertools for imap attribute.  Otherwise use map().
	(ElidingIterator): Define wrapper function __next__.
	* gdb.python/py-framefilter-mi.exp: Do not use execfile,
	use exec (open (read ())) instead.
	* gdb.python/py-framefilter.exp: Ditto.
	* gdb.python/py-arch.exp: Update print based test to Python 3.x
	compliance.
	* gdb.python/py-frame.exp: Ditto.
	* gdb.python/py-type.exp: Ditto.

--

diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index cacab4d..305bd37 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -236,12 +236,13 @@ class FrameVars(object):
         # SYM may be a string instead of a symbol in the case of
         # synthetic local arguments or locals.  If that is the case,
         # always fetch.
-        if isinstance(sym, basestring):
-            return True
+        if isinstance(sym, gdb.Symbol):
+            sym_type = sym.addr_class
 
-        sym_type = sym.addr_class
+            return self.symbol_class.get(sym_type, False)
+        else:
+            return True
 
-        return self.symbol_class.get(sym_type, False)
 
     def fetch_frame_locals(self):
         """Public utility method to fetch frame local variables for
diff --git a/gdb/python/lib/gdb/FrameIterator.py b/gdb/python/lib/gdb/FrameIterator.py
index b3af94b..3f33bbb 100644
--- a/gdb/python/lib/gdb/FrameIterator.py
+++ b/gdb/python/lib/gdb/FrameIterator.py
@@ -43,3 +43,9 @@ class FrameIterator(object):
             raise StopIteration
         self.frame = result.older()
         return result
+
+    # Python 3.x requires __next__(self) while Python 2.x requires
+    # next(self).  Define next(self), and for Python 3.x create this
+    # wrapper.
+    def __next__(self):
+        return self.next()
diff --git a/gdb/python/lib/gdb/command/frame_filters.py b/gdb/python/lib/gdb/command/frame_filters.py
index 1b73059..b5d34ad 100644
--- a/gdb/python/lib/gdb/command/frame_filters.py
+++ b/gdb/python/lib/gdb/command/frame_filters.py
@@ -335,7 +335,10 @@ class SetFrameFilterPriority(gdb.Command):
 
         list_op = command_tuple[0]
         frame_filter = command_tuple[1]
-        priority = command_tuple[2]
+
+        # GDB returns arguments as a string, so convert priority to
+        # a number.
+        priority = int(command_tuple[2])
 
         op_list = gdb.frames.return_list(list_op)
 
diff --git a/gdb/python/lib/gdb/frames.py b/gdb/python/lib/gdb/frames.py
index 10dce8e..f79c299 100644
--- a/gdb/python/lib/gdb/frames.py
+++ b/gdb/python/lib/gdb/frames.py
@@ -103,18 +103,7 @@ def return_list(name):
     Raises:
         gdb.GdbError:  A dictionary of that name cannot be found.
     """
-
-    # If all dictionaries are wanted in the case of "all" we
-    # cannot return a combined dictionary as keys() may clash in
-    # between different dictionaries.  As we just want all the frame
-    # filters to enable/disable them all, just return the combined
-    # items() as a list.
-    if name == "all":
-        all_dicts = gdb.frame_filters.values()
-        all_dicts = all_dicts + gdb.current_progspace().frame_filters.values()
-        for objfile in gdb.objfiles():
-            all_dicts = all_dicts + objfile.frame_filters.values()
-            return all_dicts
+    iterable = None
 
     if name == "global":
         return gdb.frame_filters
@@ -127,6 +116,20 @@ def return_list(name):
                 if name == objfile.filename:
                     return objfile.frame_filters
 
+    # If all dictionaries are wanted in the case of "all" we
+    # cannot return a combined dictionary as keys() may clash in
+    # between different dictionaries.  As we just want all the frame
+    # filters to enable/disable them all, just return the combined
+    # items() as a chained iterator of dictionary values.                
+    if name == "all":
+        glob = gdb.frame_filters.values()
+        prog = gdb.current_progspace().frame_filters.values()
+        return_iter = itertools.chain(glob, prog)
+        for objfile in gdb.objfiles():
+            return_iter = itertools.chain(return_iter, objfile.frame_filters.values())
+
+        return return_iter
+
     msg = "Cannot find frame-filter dictionary for '" + name + "'"
     raise gdb.GdbError(msg)
 
@@ -140,14 +143,7 @@ def _sort_list():
                      execute.
     """
 
-    all_filters = []
-    for objfile in gdb.objfiles():
-        all_filters = all_filters + objfile.frame_filters.values()
-    cp = gdb.current_progspace()
-
-    all_filters = all_filters + cp.frame_filters.values()
-    all_filters = all_filters + gdb.frame_filters.values()
-
+    all_filters = return_list("all")
     sorted_frame_filters = sorted(all_filters, key = get_priority,
                                   reverse = True)
 
@@ -181,21 +177,26 @@ def execute_frame_filters(frame, frame_low, frame_high):
 
     # Get a sorted list of frame filters.
     sorted_list = _sort_list()
-
-    # Check to see if there are any frame-filters.  If not, just
-    # return None and let default backtrace printing occur.
-    if len(sorted_list) == 0:
-        return None
+    filter_count = 0
 
     frame_iterator = FrameIterator(frame)
 
-    # Apply a basic frame decorator to all gdb.Frames.  This unifies the
-    # interface.
-    frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
+    # Apply a basic frame decorator to all gdb.Frames.  This unifies
+    # the interface.  Python 3.x moved the itertools.imap
+    # functionality to map(), so check if it is available.
+    if hasattr(itertools,"imap"):
+        frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
+    else:
+        frame_iterator = map(FrameDecorator, frame_iterator)
 
     for ff in sorted_list:
+        filter_count = filter_count + 1
         frame_iterator = ff.filter(frame_iterator)
 
+    # If there are no filters, return None.
+    if filter_count == 0:
+        return None
+
     # Slicing
 
     # Is this a slice from the end of the backtrace, ie bt -2?
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index d62c596..bd1045f 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1166,11 +1166,11 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 
 	  if (py_func != NULL)
 	    {
-	      const char *function = NULL;
+	      char *function = NULL;
 
 	      if (gdbpy_is_string (py_func))
 		{
-		  function = PyString_AsString (py_func);
+		  function = python_string_to_host_string (py_func);
 
 		  if (function == NULL)
 		    {
@@ -1188,7 +1188,8 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 
 		  msymbol = lookup_minimal_symbol_by_pc (addr);
 		  if (msymbol.minsym != NULL)
-		    function = SYMBOL_PRINT_NAME (msymbol.minsym);
+		    /* Duplicate to maintain clean-up consistency.  */
+		    function = xstrdup (SYMBOL_PRINT_NAME (msymbol.minsym));
 		}
 	      else if (py_func != Py_None)
 		{
@@ -1210,10 +1211,12 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 		}
 	      if (except.reason < 0)
 		{
+		  xfree (function);
 		  Py_DECREF (py_func);
 		  gdbpy_convert_exception (except);
 		  goto error;
 		}
+	      xfree (function);
 	    }
 	  Py_DECREF (py_func);
 	}
@@ -1251,7 +1254,7 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 	    {
 	      if (py_fn != Py_None)
 		{
-		  char *filename = PyString_AsString (py_fn);
+		  char *filename = python_string_to_host_string (py_fn);
 
 		  if (filename == NULL)
 		    {
@@ -1268,10 +1271,12 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 		    }
 		  if (except.reason < 0)
 		    {
+		      xfree (filename);
 		      Py_DECREF (py_fn);
 		      gdbpy_convert_exception (except);
 		      goto error;
 		    }
+		  xfree (filename);
 		}
 	      Py_DECREF (py_fn);
 	    }
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 4e736b8..c0cada4 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -38,16 +38,16 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
 gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(pc)" \
   "disassemble no end no count" 0
 
-gdb_test "python print len(insn_list1)" "1" "test number of instructions 1"
-gdb_test "python print len(insn_list2)" "1" "test number of instructions 2"
-gdb_test "python print len(insn_list3)" "1" "test number of instructions 3"
-gdb_test "python print len(insn_list4)" "1" "test number of instructions 4"
+gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
+gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
+gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
+gdb_test "python print (len(insn_list4))" "1" "test number of instructions 4"
 
 gdb_py_test_silent_cmd "python insn = insn_list1\[0\]" "get instruction" 0
 
-gdb_test "python print \"addr\" in insn" "True" "test key addr"
-gdb_test "python print \"asm\" in insn" "True" "test key asm"
-gdb_test "python print \"length\" in insn" "True" "test key length"
+gdb_test "python print (\"addr\" in insn)" "True" "test key addr"
+gdb_test "python print (\"asm\" in insn)" "True" "test key asm"
+gdb_test "python print (\"length\" in insn)" "True" "test key length"
 
 # Negative test
 gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 806da94..03b92e2 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -40,7 +40,7 @@ gdb_py_test_silent_cmd "python bf1 = gdb.selected_frame ()" "get frame" 0
 
 # Test Frame.architecture() method.
 gdb_py_test_silent_cmd "python show_arch_str = gdb.execute(\"show architecture\", to_string=True)" "show arch" 0
-gdb_test "python print bf1.architecture().name() in show_arch_str" "True" "test Frame.architecture()"
+gdb_test "python print (bf1.architecture().name() in show_arch_str)" "True" "test Frame.architecture()"
 
 # First test that read_var is unaffected by PR 11036 changes.
 gdb_test "python print (bf1.read_var(\"i\"))" "\"stuff\"" "test i"
diff --git a/gdb/testsuite/gdb.python/py-framefilter-mi.exp b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
index 54fedf8..08d822c 100644
--- a/gdb/testsuite/gdb.python/py-framefilter-mi.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
@@ -46,7 +46,7 @@ mi_runto main
 
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${pyfile}]
 
-mi_gdb_test "python execfile ('${remote_python_file}')" ".*\\^done." \
+mi_gdb_test "python exec (open ('${remote_python_file}').read ())" ".*\\^done." \
     "Load python file"
 
 # Multiple blocks test
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 6c9946b..0664e03 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -58,7 +58,8 @@ gdb_test_no_output "set python print-stack full" \
 
 # Load global frame-filters
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
-gdb_test_no_output "python execfile ('${remote_python_file}')" \
+
+gdb_test_no_output "python exec (open ('${remote_python_file}').read ())" \
     "Load python file"
 
 gdb_breakpoint [gdb_get_line_number "Backtrace end breakpoint"]
@@ -219,7 +220,8 @@ gdb_test_no_output "set python print-stack full" \
 
 # Load global frame-filters
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
-gdb_test_no_output "python execfile ('${remote_python_file}')" \
+
+gdb_test_no_output "python exec (open ('${remote_python_file}').read ())" \
     "Load python file for no debuginfo tests"
 
 # Disable Reverse
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index d31bbbe..e70db11 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -70,8 +70,14 @@ class FrameFilter ():
         gdb.frame_filters [self.name] = self
 
     def filter (self, frame_iter):
-        frame_iter = itertools.imap (Reverse_Function,
-                                     frame_iter)
+        # Python 3.x moved the itertools.imap functionality to map(),
+        # so check if it is available.
+        if hasattr(itertools, "imap"):
+            frame_iter = itertools.imap (Reverse_Function,
+                                         frame_iter)
+        else:
+            frame_iter = map(Reverse_Function, frame_iter)
+
         return frame_iter
 
 class ElidingFrameDecorator(FrameDecorator):
@@ -102,6 +108,12 @@ class ElidingIterator:
         elided = next(self.input_iterator)
         return ElidingFrameDecorator(frame, [elided])
 
+    # Python 3.x requires __next__(self) while Python 2.x requires
+    # next(self).  Define next(self), and for Python 3.x create this
+    # wrapper.
+    def __next__(self):
+        return self.next()
+
 class FrameElider ():
 
     def __init__ (self):
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 8e1f9ed..f6b1d96 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -65,7 +65,7 @@ proc test_fields {lang} {
 
     if {$lang == "c++"} {
       # Test usage with a class
-      gdb_py_test_silent_cmd "print c" "print value (c)" 1
+      gdb_py_test_silent_cmd "print (c)" "print value (c)" 1
       gdb_py_test_silent_cmd "python c = gdb.history (0)" "get value (c) from history" 1
       gdb_py_test_silent_cmd "python fields = c.type.fields()" "get fields from c.type" 1
       gdb_test "python print (len(fields))" "2" "Check number of fields (c)"
@@ -78,7 +78,7 @@ proc test_fields {lang} {
     }
 
     # Test normal fields usage in structs.
-    gdb_py_test_silent_cmd "print st" "print value (st)" 1
+    gdb_py_test_silent_cmd "print (st)" "print value (st)" 1
     gdb_py_test_silent_cmd "python st = gdb.history (0)" "get value (st) from history" 1
     gdb_py_test_silent_cmd "python fields = st.type.fields()" "get fields from st.type" 1
     gdb_test "python print (len(fields))" "2" "Check number of fields (st)"
@@ -109,7 +109,7 @@ proc test_fields {lang} {
     gdb_test "python print (not not st.type\['a'\].type)" "True"
   
     # Test regression PR python/10805
-    gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+    gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
     gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
     gdb_test "python fields = ar.type.fields()"
     gdb_test "python print (len(fields))" "1" "Check the number of fields"
@@ -118,7 +118,7 @@ proc test_fields {lang} {
     # Test gdb.Type.array.
     gdb_test "python print (ar\[0\].cast(ar\[0\].type.array(1)))" \
         ".1, 2." "cast to array with one argument"
-    gdb_test "python print ar\[0\].cast(ar\[0\].type.array(0, 1))" \
+    gdb_test "python print (ar\[0\].cast(ar\[0\].type.array(0, 1)))" \
         ".1, 2." "cast to array with two arguments"
 
     gdb_test "python print (ar\[0\].type == ar\[0\].type)" "True"
@@ -126,26 +126,26 @@ proc test_fields {lang} {
     # Test gdb.Type.vector.
     # Note: vectors cast differently than arrays.  Here ar[0] is replicated
     # for the size of the vector.
-    gdb_py_test_silent_cmd "print vec_data_1" "print value (vec_data_1)" 1
+    gdb_py_test_silent_cmd "print (vec_data_1)" "print value (vec_data_1)" 1
     gdb_py_test_silent_cmd "python vec_data_1 = gdb.history (0)" "get value (vec_data_1) from history" 1
 
-    gdb_py_test_silent_cmd "print vec_data_2" "print value (vec_data_2)" 1
+    gdb_py_test_silent_cmd "print (vec_data_2)" "print value (vec_data_2)" 1
     gdb_py_test_silent_cmd "python vec_data_2 = gdb.history (0)" "get value (vec_data_2) from history" 1
 
     gdb_py_test_silent_cmd "python vec1 = vec_data_1.cast(ar\[0\].type.vector(1))" "set vec1" 1
-    gdb_test "python print vec1" ".1, 1." "cast to vector with one argument"
+    gdb_test "python print (vec1)" ".1, 1." "cast to vector with one argument"
     gdb_py_test_silent_cmd "python vec2 = vec_data_1.cast(ar\[0\].type.vector(0, 1))" "set vec2" 1
-    gdb_test "python print vec2" ".1, 1." "cast to vector with two arguments"
-    gdb_test "python print vec1 == vec2" "True"
+    gdb_test "python print (vec2)" ".1, 1." "cast to vector with two arguments"
+    gdb_test "python print (vec1 == vec2)" "True"
     gdb_py_test_silent_cmd "python vec3 = vec_data_2.cast(ar\[0\].type.vector(1))" "set vec3" 1
-    gdb_test "python print vec1 == vec3" "False"
+    gdb_test "python print (vec1 == vec3)" "False"
   }
 }
 
 proc test_enums {} {
   with_test_prefix "test_enum" {
-    gdb_py_test_silent_cmd "print e" "print value (e)" 1
-    gdb_py_test_silent_cmd "python e = gdb.history (0)" "get value (e) from history" 1
+    gdb_py_test_silent_cmd "print (e)" "print value (e)" 1
+    gdb_py_test_silent_cmd "python (e) = gdb.history (0)" "get value (e) from history" 1
     gdb_py_test_silent_cmd "python fields = e.type.fields()" "extract type fields from e" 1
     gdb_test "python print (len(fields))" "3" "Check the number of enum fields"
     gdb_test "python print (fields\[0\].name)" "v1" "Check enum field\[0\] name"
@@ -161,7 +161,7 @@ proc test_enums {} {
 }
 proc test_base_class {} {
   with_test_prefix "test_base_class" {
-    gdb_py_test_silent_cmd "print d" "print value (d)" 1
+    gdb_py_test_silent_cmd "print (d)" "print value (d)" 1
     gdb_py_test_silent_cmd "python d = gdb.history (0)" "get value (d) from history" 1
     gdb_py_test_silent_cmd "python fields = d.type.fields()" "extract type fields from d" 1
     gdb_test "python print (len(fields))" "3" "Check the number of fields"
@@ -174,7 +174,7 @@ proc test_range {} {
   with_test_prefix "test_range" {
     with_test_prefix "on ranged value" {
       # Test a valid range request.
-      gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+      gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
       gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
       gdb_test "python print (len(ar.type.range()))" "2" "Check correct tuple length"
       gdb_test "python print (ar.type.range()\[0\])" "0" "Check range low bound"
@@ -183,7 +183,7 @@ proc test_range {} {
 
     with_test_prefix "on ranged type" {
       # Test a range request on a ranged type.
-      gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+      gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
       gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
       gdb_py_test_silent_cmd "python fields = ar.type.fields()" "get fields" 1
       gdb_test "python print (fields\[0\].type.range()\[0\])" "0" "Check range low bound"
@@ -192,7 +192,7 @@ proc test_range {} {
 
     with_test_prefix "on unranged value" {
       # Test where a range does not exist.
-      gdb_py_test_silent_cmd "print st" "print value (st)" 1
+      gdb_py_test_silent_cmd "print (st)" "print value (st)" 1
       gdb_py_test_silent_cmd "python st = gdb.history (0)" "get value (st) from history" 1
       gdb_test "python print (st.type.range())" "RuntimeError: This type does not have a range.*" "Check range for non ranged type."
     }

	

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-19 14:50 [patch] [python] Fix Python 3 build and testsuite issues Phil Muldoon
@ 2013-08-19 16:19 ` Tom Tromey
  2013-08-19 16:45   ` Phil Muldoon
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2013-08-19 16:19 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> This patch fixes up both current build and test-failures for Python 3
Phil> in GDB.  Most of the errors were mine, as introduced with the frame
Phil> filter patchset.  But while I was there I fixed up the other failures
Phil> too.

Thanks, Phil.

Phil> --- a/gdb/python/lib/gdb/FrameDecorator.py
Phil> +++ b/gdb/python/lib/gdb/FrameDecorator.py
Phil> @@ -236,12 +236,13 @@ class FrameVars(object):
Phil>          # SYM may be a string instead of a symbol in the case of
Phil>          # synthetic local arguments or locals.  If that is the case,
Phil>          # always fetch.
Phil> -        if isinstance(sym, basestring):
Phil> -            return True
Phil> +        if isinstance(sym, gdb.Symbol):
Phil> +            sym_type = sym.addr_class
 
Phil> -        sym_type = sym.addr_class
Phil> +            return self.symbol_class.get(sym_type, False)
Phil> +        else:
Phil> +            return True

This seems like a semantic change to me.

Previously it checked for a string type and all other types fell through.
This means you could pass in a "symbol-like" class and get a certain
behavior thanks to duck-typing.

Now, it checks specifically for gdb.Symbol and treats other types as the
"string" case.

Perhaps a hasattr check is more appropriate.

FWIW I couldn't see any way in-tree that this method could be called
with a non-symbol-like argument.  But perhaps we're concerned about
out-of-tree callers.

Phil> diff --git a/gdb/python/lib/gdb/frames.py b/gdb/python/lib/gdb/frames.py
[...]

Phil> +    iterable = None

This new variable is never used.

Phil> +    # If all dictionaries are wanted in the case of "all" we
Phil> +    # cannot return a combined dictionary as keys() may clash in
Phil> +    # between different dictionaries.  As we just want all the frame
Phil> +    # filters to enable/disable them all, just return the combined
Phil> +    # items() as a chained iterator of dictionary values.                
Phil> +    if name == "all":
Phil> +        glob = gdb.frame_filters.values()
Phil> +        prog = gdb.current_progspace().frame_filters.values()
Phil> +        return_iter = itertools.chain(glob, prog)
Phil> +        for objfile in gdb.objfiles():
Phil> +            return_iter = itertools.chain(return_iter, objfile.frame_filters.values())
Phil> +
Phil> +        return return_iter

I don't understand why this code block was moved.

Phil>      # Get a sorted list of frame filters.
Phil>      sorted_list = _sort_list()
Phil> -
Phil> -    # Check to see if there are any frame-filters.  If not, just
Phil> -    # return None and let default backtrace printing occur.
Phil> -    if len(sorted_list) == 0:
Phil> -        return None
Phil> +    filter_count = 0
 
Phil>      frame_iterator = FrameIterator(frame)
 
Phil> -    # Apply a basic frame decorator to all gdb.Frames.  This unifies the
Phil> -    # interface.
Phil> -    frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
Phil> +    # Apply a basic frame decorator to all gdb.Frames.  This unifies
Phil> +    # the interface.  Python 3.x moved the itertools.imap
Phil> +    # functionality to map(), so check if it is available.
Phil> +    if hasattr(itertools,"imap"):
Phil> +        frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
Phil> +    else:
Phil> +        frame_iterator = map(FrameDecorator, frame_iterator)
 
Phil>      for ff in sorted_list:
Phil> +        filter_count = filter_count + 1
Phil>          frame_iterator = ff.filter(frame_iterator)
 
Phil> +    # If there are no filters, return None.
Phil> +    if filter_count == 0:
Phil> +        return None

This approach means constructing extra objects in the case where no
frame filters apply.

I think it is simpler and more efficient to keep the early exit and
either have _sort_list return a list, or do:

    sorted_list = list(_sort_list())


Phil>  	  if (py_func != NULL)
Phil>  	    {
Phil> -	      const char *function = NULL;
Phil> +	      char *function = NULL;
 
Phil>  	      if (gdbpy_is_string (py_func))
Phil>  		{
Phil> -		  function = PyString_AsString (py_func);
Phil> +		  function = python_string_to_host_string (py_func);
 
Phil>  		  if (function == NULL)
Phil>  		    {
Phil> @@ -1188,7 +1188,8 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 
Phil>  		  msymbol = lookup_minimal_symbol_by_pc (addr);
Phil>  		  if (msymbol.minsym != NULL)
Phil> -		    function = SYMBOL_PRINT_NAME (msymbol.minsym);
Phil> +		    /* Duplicate to maintain clean-up consistency.  */
Phil> +		    function = xstrdup (SYMBOL_PRINT_NAME (msymbol.minsym));

I think it is arguably better, and certainly more efficient, to make an
explicit cleanup on the one branch where it is needed.  Cleanups are
already in use in this function so no added logic is needed.  And, this
lets us keep "function" const, which is clearer in a large function.

This applies elsewhere as well.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-19 16:19 ` Tom Tromey
@ 2013-08-19 16:45   ` Phil Muldoon
  2013-08-19 18:34     ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Phil Muldoon @ 2013-08-19 16:45 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

On 19/08/13 17:19, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> This patch fixes up both current build and test-failures for Python 3
> Phil> in GDB.  Most of the errors were mine, as introduced with the frame
> Phil> filter patchset.  But while I was there I fixed up the other failures
> Phil> too.
> 
> Thanks, Phil.
> 
> Phil> --- a/gdb/python/lib/gdb/FrameDecorator.py
> Phil> +++ b/gdb/python/lib/gdb/FrameDecorator.py
> Phil> @@ -236,12 +236,13 @@ class FrameVars(object):
> Phil>          # SYM may be a string instead of a symbol in the case of
> Phil>          # synthetic local arguments or locals.  If that is the case,
> Phil>          # always fetch.
> Phil> -        if isinstance(sym, basestring):
> Phil> -            return True
> Phil> +        if isinstance(sym, gdb.Symbol):
> Phil> +            sym_type = sym.addr_class
>  
> Phil> -        sym_type = sym.addr_class
> Phil> +            return self.symbol_class.get(sym_type, False)
> Phil> +        else:
> Phil> +            return True
> 
> This seems like a semantic change to me.
> 
> Previously it checked for a string type and all other types fell through.
> This means you could pass in a "symbol-like" class and get a certain
> behavior thanks to duck-typing.
> 
> Now, it checks specifically for gdb.Symbol and treats other types as the
> "string" case.
> 
> Perhaps a hasattr check is more appropriate.

Yes, I agree. The duck-typing argument did not occur to me.
Basically, I reversed the check as Python does not have basestring in
Python 3.x so it seemed simpler to reverse the condition.  But, of
course, if they pass in some object that looks like a gdb.Symbol - but
isn't - it will fail.  I believe we do say in the documentation that
this has to be a "gdb.Symbol or a string".  But I like your comment
better - there is no reason to force it to a strict gdb.Symbol type.


> FWIW I couldn't see any way in-tree that this method could be called
> with a non-symbol-like argument.  But perhaps we're concerned about
> out-of-tree callers.

The branch occurs as we allow synthetic locals and arguments (ie, ones
that do not exist in the inferior).

> Phil> diff --git a/gdb/python/lib/gdb/frames.py b/gdb/python/lib/gdb/frames.py
> [...]
> 
> Phil> +    iterable = None
> 
> This new variable is never used.

Oops apologies.

> Phil> +    # If all dictionaries are wanted in the case of "all" we
> Phil> +    # cannot return a combined dictionary as keys() may clash in
> Phil> +    # between different dictionaries.  As we just want all the frame
> Phil> +    # filters to enable/disable them all, just return the combined
> Phil> +    # items() as a chained iterator of dictionary values.                
> Phil> +    if name == "all":
> Phil> +        glob = gdb.frame_filters.values()
> Phil> +        prog = gdb.current_progspace().frame_filters.values()
> Phil> +        return_iter = itertools.chain(glob, prog)
> Phil> +        for objfile in gdb.objfiles():
> Phil> +            return_iter = itertools.chain(return_iter, objfile.frame_filters.values())
> Phil> +
> Phil> +        return return_iter
> 
> I don't understand why this code block was moved.

Just a mistake, not sure why I did not notice it.

> Phil>      # Get a sorted list of frame filters.
> Phil>      sorted_list = _sort_list()
> Phil> -
> Phil> -    # Check to see if there are any frame-filters.  If not, just
> Phil> -    # return None and let default backtrace printing occur.
> Phil> -    if len(sorted_list) == 0:
> Phil> -        return None
> Phil> +    filter_count = 0
>  
> Phil>      frame_iterator = FrameIterator(frame)
>  
> Phil> -    # Apply a basic frame decorator to all gdb.Frames.  This unifies the
> Phil> -    # interface.
> Phil> -    frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
> Phil> +    # Apply a basic frame decorator to all gdb.Frames.  This unifies
> Phil> +    # the interface.  Python 3.x moved the itertools.imap
> Phil> +    # functionality to map(), so check if it is available.
> Phil> +    if hasattr(itertools,"imap"):
> Phil> +        frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
> Phil> +    else:
> Phil> +        frame_iterator = map(FrameDecorator, frame_iterator)
>  
> Phil>      for ff in sorted_list:
> Phil> +        filter_count = filter_count + 1
> Phil>          frame_iterator = ff.filter(frame_iterator)
>  
> Phil> +    # If there are no filters, return None.
> Phil> +    if filter_count == 0:
> Phil> +        return None
> 
> This approach means constructing extra objects in the case where no
> frame filters apply.
> 
> I think it is simpler and more efficient to keep the early exit and
> either have _sort_list return a list, or do:
> 
>     sorted_list = list(_sort_list())

I avoided this because I did not want to convert an iterable to a list
(the new dict.keys/values/items returns a lightweight iterator instead
of a list.)  My thoughts were that creating an imap or map like object
would be cheaper than creating a list from a lightweight iterator.


> Phil>  	  if (py_func != NULL)
> Phil>  	    {
> Phil> -	      const char *function = NULL;
> Phil> +	      char *function = NULL;
>  
> Phil>  	      if (gdbpy_is_string (py_func))
> Phil>  		{
> Phil> -		  function = PyString_AsString (py_func);
> Phil> +		  function = python_string_to_host_string (py_func);
>  
> Phil>  		  if (function == NULL)
> Phil>  		    {
> Phil> @@ -1188,7 +1188,8 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
>  
> Phil>  		  msymbol = lookup_minimal_symbol_by_pc (addr);
> Phil>  		  if (msymbol.minsym != NULL)
> Phil> -		    function = SYMBOL_PRINT_NAME (msymbol.minsym);
> Phil> +		    /* Duplicate to maintain clean-up consistency.  */
> Phil> +		    function = xstrdup (SYMBOL_PRINT_NAME (msymbol.minsym));
> 
> I think it is arguably better, and certainly more efficient, to make an
> explicit cleanup on the one branch where it is needed.  Cleanups are
> already in use in this function so no added logic is needed.  And, this
> lets us keep "function" const, which is clearer in a large function.

I chose this for consistency in dealing with the function name cleanup.  I
have no strong objections, but the paradigm of duplicating a string in
one branch where other branches allocate a new string is already 
entrenched in the Python code.

Though I do take your point in that this is dealing with symbols and
is likely to be often traversed code.  I'll fix it up.  You mentioned
it applies elsewhere in the patchset? The only similar path is for the
filename, but that always creates a newly allocated string from
python_string_to_host_string.  There is no other
newly-allocated/referenced pointer split in the code path in this
patch?

Cheers,

Phil


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-19 16:45   ` Phil Muldoon
@ 2013-08-19 18:34     ` Tom Tromey
  2013-08-20 19:43       ` Phil Muldoon
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2013-08-19 18:34 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

>> sorted_list = list(_sort_list())

Phil> I avoided this because I did not want to convert an iterable to a list
Phil> (the new dict.keys/values/items returns a lightweight iterator instead
Phil> of a list.)  My thoughts were that creating an imap or map like object
Phil> would be cheaper than creating a list from a lightweight iterator.

Perhaps it's a wash.
But the old code was also simpler to understand.

Phil> You mentioned
Phil> it applies elsewhere in the patchset? The only similar path is for the
Phil> filename, but that always creates a newly allocated string from
Phil> python_string_to_host_string.  There is no other
Phil> newly-allocated/referenced pointer split in the code path in this
Phil> patch?

Yeah, I misread the filename handling hunk.
Sorry about that.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-19 18:34     ` Tom Tromey
@ 2013-08-20 19:43       ` Phil Muldoon
  2013-08-20 19:59         ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Phil Muldoon @ 2013-08-20 19:43 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 19/08/13 19:34, Tom Tromey wrote:
>>> sorted_list = list(_sort_list())
> 
> Phil> I avoided this because I did not want to convert an iterable to a list
> Phil> (the new dict.keys/values/items returns a lightweight iterator instead
> Phil> of a list.)  My thoughts were that creating an imap or map like object
> Phil> would be cheaper than creating a list from a lightweight iterator.
> 
> Perhaps it's a wash.
> But the old code was also simpler to understand.

I ended up going with your suggestions as the readability trumps any
argument I had.


I have regenerated the patch with your requested updates.  

What do you think?

Cheers,

Phil

--

diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index cacab4d..9a4367b 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -236,7 +236,7 @@ class FrameVars(object):
         # SYM may be a string instead of a symbol in the case of
         # synthetic local arguments or locals.  If that is the case,
         # always fetch.
-        if isinstance(sym, basestring):
+        if isinstance(sym, str):
             return True
 
         sym_type = sym.addr_class
diff --git a/gdb/python/lib/gdb/FrameIterator.py b/gdb/python/lib/gdb/FrameIterator.py
index b3af94b..3f33bbb 100644
--- a/gdb/python/lib/gdb/FrameIterator.py
+++ b/gdb/python/lib/gdb/FrameIterator.py
@@ -43,3 +43,9 @@ class FrameIterator(object):
             raise StopIteration
         self.frame = result.older()
         return result
+
+    # Python 3.x requires __next__(self) while Python 2.x requires
+    # next(self).  Define next(self), and for Python 3.x create this
+    # wrapper.
+    def __next__(self):
+        return self.next()
diff --git a/gdb/python/lib/gdb/command/frame_filters.py b/gdb/python/lib/gdb/command/frame_filters.py
index 1b73059..b5d34ad 100644
--- a/gdb/python/lib/gdb/command/frame_filters.py
+++ b/gdb/python/lib/gdb/command/frame_filters.py
@@ -335,7 +335,10 @@ class SetFrameFilterPriority(gdb.Command):
 
         list_op = command_tuple[0]
         frame_filter = command_tuple[1]
-        priority = command_tuple[2]
+
+        # GDB returns arguments as a string, so convert priority to
+        # a number.
+        priority = int(command_tuple[2])
 
         op_list = gdb.frames.return_list(list_op)
 
diff --git a/gdb/python/lib/gdb/frames.py b/gdb/python/lib/gdb/frames.py
index 10dce8e..c148b98 100644
--- a/gdb/python/lib/gdb/frames.py
+++ b/gdb/python/lib/gdb/frames.py
@@ -108,13 +108,15 @@ def return_list(name):
     # cannot return a combined dictionary as keys() may clash in
     # between different dictionaries.  As we just want all the frame
     # filters to enable/disable them all, just return the combined
-    # items() as a list.
+    # items() as a chained iterator of dictionary values.
     if name == "all":
-        all_dicts = gdb.frame_filters.values()
-        all_dicts = all_dicts + gdb.current_progspace().frame_filters.values()
+        glob = gdb.frame_filters.values()
+        prog = gdb.current_progspace().frame_filters.values()
+        return_iter = itertools.chain(glob, prog)
         for objfile in gdb.objfiles():
-            all_dicts = all_dicts + objfile.frame_filters.values()
-            return all_dicts
+            return_iter = itertools.chain(return_iter, objfile.frame_filters.values())
+
+        return return_iter
 
     if name == "global":
         return gdb.frame_filters
@@ -140,14 +142,7 @@ def _sort_list():
                      execute.
     """
 
-    all_filters = []
-    for objfile in gdb.objfiles():
-        all_filters = all_filters + objfile.frame_filters.values()
-    cp = gdb.current_progspace()
-
-    all_filters = all_filters + cp.frame_filters.values()
-    all_filters = all_filters + gdb.frame_filters.values()
-
+    all_filters = return_list("all")
     sorted_frame_filters = sorted(all_filters, key = get_priority,
                                   reverse = True)
 
@@ -180,7 +175,7 @@ def execute_frame_filters(frame, frame_low, frame_high):
     """
 
     # Get a sorted list of frame filters.
-    sorted_list = _sort_list()
+    sorted_list = list(_sort_list())
 
     # Check to see if there are any frame-filters.  If not, just
     # return None and let default backtrace printing occur.
@@ -189,9 +184,13 @@ def execute_frame_filters(frame, frame_low, frame_high):
 
     frame_iterator = FrameIterator(frame)
 
-    # Apply a basic frame decorator to all gdb.Frames.  This unifies the
-    # interface.
-    frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
+    # Apply a basic frame decorator to all gdb.Frames.  This unifies
+    # the interface.  Python 3.x moved the itertools.imap
+    # functionality to map(), so check if it is available.
+    if hasattr(itertools,"imap"):
+        frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
+    else:
+        frame_iterator = map(FrameDecorator, frame_iterator)
 
     for ff in sorted_list:
         frame_iterator = ff.filter(frame_iterator)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index d62c596..57795eb 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1166,17 +1166,20 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 
 	  if (py_func != NULL)
 	    {
-	      const char *function = NULL;
+	      char *function_to_free = NULL;
+	      const char *function;
 
 	      if (gdbpy_is_string (py_func))
 		{
-		  function = PyString_AsString (py_func);
+		  function = function_to_free =
+		    python_string_to_host_string (py_func);
 
 		  if (function == NULL)
 		    {
 		      Py_DECREF (py_func);
 		      goto error;
 		    }
+		  make_cleanup (xfree, function_to_free);
 		}
 	      else if (PyLong_Check (py_func))
 		{
@@ -1251,13 +1254,15 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 	    {
 	      if (py_fn != Py_None)
 		{
-		  char *filename = PyString_AsString (py_fn);
+		  char *filename = python_string_to_host_string (py_fn);
 
 		  if (filename == NULL)
 		    {
 		      Py_DECREF (py_fn);
 		      goto error;
 		    }
+
+		  make_cleanup (xfree, filename);
 		  TRY_CATCH (except, RETURN_MASK_ALL)
 		    {
 		      ui_out_wrap_hint (out, "   ");
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 4e736b8..c0cada4 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -38,16 +38,16 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
 gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(pc)" \
   "disassemble no end no count" 0
 
-gdb_test "python print len(insn_list1)" "1" "test number of instructions 1"
-gdb_test "python print len(insn_list2)" "1" "test number of instructions 2"
-gdb_test "python print len(insn_list3)" "1" "test number of instructions 3"
-gdb_test "python print len(insn_list4)" "1" "test number of instructions 4"
+gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
+gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
+gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
+gdb_test "python print (len(insn_list4))" "1" "test number of instructions 4"
 
 gdb_py_test_silent_cmd "python insn = insn_list1\[0\]" "get instruction" 0
 
-gdb_test "python print \"addr\" in insn" "True" "test key addr"
-gdb_test "python print \"asm\" in insn" "True" "test key asm"
-gdb_test "python print \"length\" in insn" "True" "test key length"
+gdb_test "python print (\"addr\" in insn)" "True" "test key addr"
+gdb_test "python print (\"asm\" in insn)" "True" "test key asm"
+gdb_test "python print (\"length\" in insn)" "True" "test key length"
 
 # Negative test
 gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 806da94..63e4afb 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -40,7 +40,7 @@ gdb_py_test_silent_cmd "python bf1 = gdb.selected_frame ()" "get frame" 0
 
 # Test Frame.architecture() method.
 gdb_py_test_silent_cmd "python show_arch_str = gdb.execute(\"show architecture\", to_string=True)" "show arch" 0
-gdb_test "python print bf1.architecture().name() in show_arch_str" "True" "test Frame.architecture()"
+gdb_test "python print (bf1.architecture().name() in show_arch_str)" "True" "test Frame.architecture()"
 
 # First test that read_var is unaffected by PR 11036 changes.
 gdb_test "python print (bf1.read_var(\"i\"))" "\"stuff\"" "test i"
diff --git a/gdb/testsuite/gdb.python/py-framefilter-mi.exp b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
index 54fedf8..08d822c 100644
--- a/gdb/testsuite/gdb.python/py-framefilter-mi.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
@@ -46,7 +46,7 @@ mi_runto main
 
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${pyfile}]
 
-mi_gdb_test "python execfile ('${remote_python_file}')" ".*\\^done." \
+mi_gdb_test "python exec (open ('${remote_python_file}').read ())" ".*\\^done." \
     "Load python file"
 
 # Multiple blocks test
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 6c9946b..0664e03 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -58,7 +58,8 @@ gdb_test_no_output "set python print-stack full" \
 
 # Load global frame-filters
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
-gdb_test_no_output "python execfile ('${remote_python_file}')" \
+
+gdb_test_no_output "python exec (open ('${remote_python_file}').read ())" \
     "Load python file"
 
 gdb_breakpoint [gdb_get_line_number "Backtrace end breakpoint"]
@@ -219,7 +220,8 @@ gdb_test_no_output "set python print-stack full" \
 
 # Load global frame-filters
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
-gdb_test_no_output "python execfile ('${remote_python_file}')" \
+
+gdb_test_no_output "python exec (open ('${remote_python_file}').read ())" \
     "Load python file for no debuginfo tests"
 
 # Disable Reverse
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index d31bbbe..e70db11 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -70,8 +70,14 @@ class FrameFilter ():
         gdb.frame_filters [self.name] = self
 
     def filter (self, frame_iter):
-        frame_iter = itertools.imap (Reverse_Function,
-                                     frame_iter)
+        # Python 3.x moved the itertools.imap functionality to map(),
+        # so check if it is available.
+        if hasattr(itertools, "imap"):
+            frame_iter = itertools.imap (Reverse_Function,
+                                         frame_iter)
+        else:
+            frame_iter = map(Reverse_Function, frame_iter)
+
         return frame_iter
 
 class ElidingFrameDecorator(FrameDecorator):
@@ -102,6 +108,12 @@ class ElidingIterator:
         elided = next(self.input_iterator)
         return ElidingFrameDecorator(frame, [elided])
 
+    # Python 3.x requires __next__(self) while Python 2.x requires
+    # next(self).  Define next(self), and for Python 3.x create this
+    # wrapper.
+    def __next__(self):
+        return self.next()
+
 class FrameElider ():
 
     def __init__ (self):
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 8e1f9ed..f6b1d96 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -65,7 +65,7 @@ proc test_fields {lang} {
 
     if {$lang == "c++"} {
       # Test usage with a class
-      gdb_py_test_silent_cmd "print c" "print value (c)" 1
+      gdb_py_test_silent_cmd "print (c)" "print value (c)" 1
       gdb_py_test_silent_cmd "python c = gdb.history (0)" "get value (c) from history" 1
       gdb_py_test_silent_cmd "python fields = c.type.fields()" "get fields from c.type" 1
       gdb_test "python print (len(fields))" "2" "Check number of fields (c)"
@@ -78,7 +78,7 @@ proc test_fields {lang} {
     }
 
     # Test normal fields usage in structs.
-    gdb_py_test_silent_cmd "print st" "print value (st)" 1
+    gdb_py_test_silent_cmd "print (st)" "print value (st)" 1
     gdb_py_test_silent_cmd "python st = gdb.history (0)" "get value (st) from history" 1
     gdb_py_test_silent_cmd "python fields = st.type.fields()" "get fields from st.type" 1
     gdb_test "python print (len(fields))" "2" "Check number of fields (st)"
@@ -109,7 +109,7 @@ proc test_fields {lang} {
     gdb_test "python print (not not st.type\['a'\].type)" "True"
   
     # Test regression PR python/10805
-    gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+    gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
     gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
     gdb_test "python fields = ar.type.fields()"
     gdb_test "python print (len(fields))" "1" "Check the number of fields"
@@ -118,7 +118,7 @@ proc test_fields {lang} {
     # Test gdb.Type.array.
     gdb_test "python print (ar\[0\].cast(ar\[0\].type.array(1)))" \
         ".1, 2." "cast to array with one argument"
-    gdb_test "python print ar\[0\].cast(ar\[0\].type.array(0, 1))" \
+    gdb_test "python print (ar\[0\].cast(ar\[0\].type.array(0, 1)))" \
         ".1, 2." "cast to array with two arguments"
 
     gdb_test "python print (ar\[0\].type == ar\[0\].type)" "True"
@@ -126,26 +126,26 @@ proc test_fields {lang} {
     # Test gdb.Type.vector.
     # Note: vectors cast differently than arrays.  Here ar[0] is replicated
     # for the size of the vector.
-    gdb_py_test_silent_cmd "print vec_data_1" "print value (vec_data_1)" 1
+    gdb_py_test_silent_cmd "print (vec_data_1)" "print value (vec_data_1)" 1
     gdb_py_test_silent_cmd "python vec_data_1 = gdb.history (0)" "get value (vec_data_1) from history" 1
 
-    gdb_py_test_silent_cmd "print vec_data_2" "print value (vec_data_2)" 1
+    gdb_py_test_silent_cmd "print (vec_data_2)" "print value (vec_data_2)" 1
     gdb_py_test_silent_cmd "python vec_data_2 = gdb.history (0)" "get value (vec_data_2) from history" 1
 
     gdb_py_test_silent_cmd "python vec1 = vec_data_1.cast(ar\[0\].type.vector(1))" "set vec1" 1
-    gdb_test "python print vec1" ".1, 1." "cast to vector with one argument"
+    gdb_test "python print (vec1)" ".1, 1." "cast to vector with one argument"
     gdb_py_test_silent_cmd "python vec2 = vec_data_1.cast(ar\[0\].type.vector(0, 1))" "set vec2" 1
-    gdb_test "python print vec2" ".1, 1." "cast to vector with two arguments"
-    gdb_test "python print vec1 == vec2" "True"
+    gdb_test "python print (vec2)" ".1, 1." "cast to vector with two arguments"
+    gdb_test "python print (vec1 == vec2)" "True"
     gdb_py_test_silent_cmd "python vec3 = vec_data_2.cast(ar\[0\].type.vector(1))" "set vec3" 1
-    gdb_test "python print vec1 == vec3" "False"
+    gdb_test "python print (vec1 == vec3)" "False"
   }
 }
 
 proc test_enums {} {
   with_test_prefix "test_enum" {
-    gdb_py_test_silent_cmd "print e" "print value (e)" 1
-    gdb_py_test_silent_cmd "python e = gdb.history (0)" "get value (e) from history" 1
+    gdb_py_test_silent_cmd "print (e)" "print value (e)" 1
+    gdb_py_test_silent_cmd "python (e) = gdb.history (0)" "get value (e) from history" 1
     gdb_py_test_silent_cmd "python fields = e.type.fields()" "extract type fields from e" 1
     gdb_test "python print (len(fields))" "3" "Check the number of enum fields"
     gdb_test "python print (fields\[0\].name)" "v1" "Check enum field\[0\] name"
@@ -161,7 +161,7 @@ proc test_enums {} {
 }
 proc test_base_class {} {
   with_test_prefix "test_base_class" {
-    gdb_py_test_silent_cmd "print d" "print value (d)" 1
+    gdb_py_test_silent_cmd "print (d)" "print value (d)" 1
     gdb_py_test_silent_cmd "python d = gdb.history (0)" "get value (d) from history" 1
     gdb_py_test_silent_cmd "python fields = d.type.fields()" "extract type fields from d" 1
     gdb_test "python print (len(fields))" "3" "Check the number of fields"
@@ -174,7 +174,7 @@ proc test_range {} {
   with_test_prefix "test_range" {
     with_test_prefix "on ranged value" {
       # Test a valid range request.
-      gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+      gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
       gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
       gdb_test "python print (len(ar.type.range()))" "2" "Check correct tuple length"
       gdb_test "python print (ar.type.range()\[0\])" "0" "Check range low bound"
@@ -183,7 +183,7 @@ proc test_range {} {
 
     with_test_prefix "on ranged type" {
       # Test a range request on a ranged type.
-      gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+      gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
       gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
       gdb_py_test_silent_cmd "python fields = ar.type.fields()" "get fields" 1
       gdb_test "python print (fields\[0\].type.range()\[0\])" "0" "Check range low bound"
@@ -192,7 +192,7 @@ proc test_range {} {
 
     with_test_prefix "on unranged value" {
       # Test where a range does not exist.
-      gdb_py_test_silent_cmd "print st" "print value (st)" 1
+      gdb_py_test_silent_cmd "print (st)" "print value (st)" 1
       gdb_py_test_silent_cmd "python st = gdb.history (0)" "get value (st) from history" 1
       gdb_test "python print (st.type.range())" "RuntimeError: This type does not have a range.*" "Check range for non ranged type."
     }


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-20 19:43       ` Phil Muldoon
@ 2013-08-20 19:59         ` Tom Tromey
  2013-08-20 20:32           ` Phil Muldoon
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2013-08-20 19:59 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> @@ -236,7 +236,7 @@ class FrameVars(object):
Phil>          # SYM may be a string instead of a symbol in the case of
Phil>          # synthetic local arguments or locals.  If that is the case,
Phil>          # always fetch.
Phil> -        if isinstance(sym, basestring):
Phil> +        if isinstance(sym, str):
Phil>              return True

Does this work in all versions?
I thought perhaps hasattr would be more robust here.

Phil>  	  if (py_func != NULL)
Phil>  	    {
Phil> -	      const char *function = NULL;
Phil> +	      char *function_to_free = NULL;
Phil> +	      const char *function;
 
Phil>  	      if (gdbpy_is_string (py_func))
Phil>  		{
Phil> -		  function = PyString_AsString (py_func);
Phil> +		  function = function_to_free =
Phil> +		    python_string_to_host_string (py_func);

I think it's preferable to declare function_to_free in the innermost
scope.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-20 19:59         ` Tom Tromey
@ 2013-08-20 20:32           ` Phil Muldoon
  2013-08-21 14:29             ` Phil Muldoon
  2013-08-21 14:56             ` Tom Tromey
  0 siblings, 2 replies; 15+ messages in thread
From: Phil Muldoon @ 2013-08-20 20:32 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 20/08/13 20:59, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> @@ -236,7 +236,7 @@ class FrameVars(object):
> Phil>          # SYM may be a string instead of a symbol in the case of
> Phil>          # synthetic local arguments or locals.  If that is the case,
> Phil>          # always fetch.
> Phil> -        if isinstance(sym, basestring):
> Phil> +        if isinstance(sym, str):
> Phil>              return True
> 
> Does this work in all versions?
> I thought perhaps hasattr would be more robust here.

Tested on both 2.7.3 and 3.3.0 on Fedora 18.  basestring seems to
originate from Python 2.3 and was retired in 3.0.

http://docs.python.org/2/library/functions.html

basestring()

    This abstract type is the superclass for str and unicode. It
    cannot be called or instantiated, but it can be used to test
    whether an object is an instance of str or
    unicode. isinstance(obj, basestring) is equivalent to
    isinstance(obj,(str, unicode)).
    
    New in version 2.3.

As str is a subclass to basestring, I find it hard to believe that it
would also not work in Pythons 2.3.  All tests pass in GDB with both
Python 2.7.3 and 3.3.

Further the Python what's new guide to 3.0 specifies:

http://docs.python.org/3.0/whatsnew/3.0.html

"The builtin basestring abstract type was removed. Use str instead. The
str and bytes types don't have functionality enough in common
to warrant a shared base class. The 2to3 tool (see below) replaces
every occurrence of basestring with str."

So the 2to3 tool does this as well.

For the hasattr point, I believe the equivalent check is the same in
this version, as the incompatible Python 3 frame filters code.  But I
use several methods of gdb.Symbol so which one would we use?  Remember
this is not the Symbol/Value like interface that frame_args, and
frame_locals returns, but the actual return of the symbol() function
from that interface.  At this point in the code we have already called
that symbol() API and have the object it returned.

See SymValueWrapper class in FrameDecorator.py.  This object interface
specifies that it must return either a string, or a gdb.Symbol.  I
suppose you could return a Symbol like object, but you would have to
implement pretty much all of the gdb.Symbol methods.  This would not
make a great deal of sense.  Instead we should make gdb.Symbol
inheritable, so the user can sub-class.  This new child class of
gdb.Symbol would still work with the above code as the code  makes no
assumptions of the class other than it has the available methods.

> 
> Phil>  	  if (py_func != NULL)
> Phil>  	    {
> Phil> -	      const char *function = NULL;
> Phil> +	      char *function_to_free = NULL;
> Phil> +	      const char *function;
>  
> Phil>  	      if (gdbpy_is_string (py_func))
> Phil>  		{
> Phil> -		  function = PyString_AsString (py_func);
> Phil> +		  function = function_to_free =
> Phil> +		    python_string_to_host_string (py_func);
> 
> I think it's preferable to declare function_to_free in the innermost
> scope.

Sure thing.  Will fix.  I will post an updated patch once the
basestring discussion is completed.

Cheers

Phil

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-20 20:32           ` Phil Muldoon
@ 2013-08-21 14:29             ` Phil Muldoon
  2013-08-21 14:59               ` Tom Tromey
  2013-08-21 14:56             ` Tom Tromey
  1 sibling, 1 reply; 15+ messages in thread
From: Phil Muldoon @ 2013-08-21 14:29 UTC (permalink / raw)
  To: gdb-patches

On 20/08/13 21:32, Phil Muldoon wrote:
> On 20/08/13 20:59, Tom Tromey wrote:
>>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>>
>> Phil> @@ -236,7 +236,7 @@ class FrameVars(object):
>> Phil>          # SYM may be a string instead of a symbol in the case of
>> Phil>          # synthetic local arguments or locals.  If that is the case,
>> Phil>          # always fetch.
>> Phil> -        if isinstance(sym, basestring):
>> Phil> +        if isinstance(sym, str):
>> Phil>              return True
>>
>> Does this work in all versions?
>> I thought perhaps hasattr would be more robust here.
> 
> Tested on both 2.7.3 and 3.3.0 on Fedora 18.  basestring seems to
> originate from Python 2.3 and was retired in 3.0.

I was partially wrong here.

Strings in Python 3 are now always encoded and are encapsulated by the
"str" class.

In Python 2 you had str() and unicode(), where unicode was encoded and
str just represented bytes (IE just an unencoded string).

So in Python 2:

>>> a = "foo"
>>> b = u"bar"
>>> type(a)
<type 'str'>
>>> type(b)
<type 'unicode'>
>>> print isinstance(a,str)
True
>>> print isinstance(b,str)
False

Whereas in Python 3:

>>> a = "foo"
>>> b = u"bar"
>>> type(a)
<class 'str'>
>>> type(b)
<class 'str'>
>>> print (isinstance(a,str))
True
>>> print (isinstance(b,str))
True

So the patch hunk:


-        if isinstance(sym, basestring):
+        if isinstance(sym, str):

Will work in all case for Python 3.  Will work in the str() case of
Python 2.x (unencoded), but will not work for encoded unicode strings
in Python 2.x.

Reading around the suggestion seems to be to do this:

try:
   # basestring catches both types of Python 2.x strings
   if isinstance(sym, basestring)
        return True
except NameError:
   # If we are here, basestring does not exist, so Python 3.x
   if isinstance(sym, str)
        return True
# Continue to process objects that are not a string.

This kind of sucks.  We could come back to a hasattr test for a
string, but I am just not sure what is feasible and will remain stable
through Python 2.x and 3.x release cycles.  We could reverse the
condition and hasattr for a symbol like object, but see my previous
note on the expectations of the API.

Regardless, I think at some point in GDB's future we will have to
declare "Python 3.x only from GDB X.X on" as maintaining the two
versions of Python is going to become arduous.

Cheers,

Phil


	  

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-20 20:32           ` Phil Muldoon
  2013-08-21 14:29             ` Phil Muldoon
@ 2013-08-21 14:56             ` Tom Tromey
  2013-08-22 10:46               ` Phil Muldoon
  1 sibling, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2013-08-21 14:56 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> For the hasattr point, I believe the equivalent check is the same in
Phil> this version, as the incompatible Python 3 frame filters code.  But I
Phil> use several methods of gdb.Symbol so which one would we use?  Remember
Phil> this is not the Symbol/Value like interface that frame_args, and
Phil> frame_locals returns, but the actual return of the symbol() function
Phil> from that interface.  At this point in the code we have already called
Phil> that symbol() API and have the object it returned.

We could pick any unique-to-Symbol method as a sentinel.
Say, the first one called in this method.

Phil> See SymValueWrapper class in FrameDecorator.py.  This object interface
Phil> specifies that it must return either a string, or a gdb.Symbol.  I
Phil> suppose you could return a Symbol like object, but you would have to
Phil> implement pretty much all of the gdb.Symbol methods.  This would not
Phil> make a great deal of sense.  Instead we should make gdb.Symbol
Phil> inheritable, so the user can sub-class.  This new child class of
Phil> gdb.Symbol would still work with the above code as the code  makes no
Phil> assumptions of the class other than it has the available methods.

I think subclassing Symbol is problematic.
For example I think it means it is hard to make a purely synthetic one.
What would the native state look like?  Where would it come from?

It seems better to go with Python's duck-typing approach instead.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-21 14:29             ` Phil Muldoon
@ 2013-08-21 14:59               ` Tom Tromey
  2013-08-21 15:37                 ` Paul_Koning
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2013-08-21 14:59 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

Phil> Strings in Python 3 are now always encoded and are encapsulated by the
Phil> "str" class.

Phil> In Python 2 you had str() and unicode(), where unicode was encoded and
Phil> str just represented bytes (IE just an unencoded string).

Phil> Reading around the suggestion seems to be to do this:

Phil> try:
Phil>    # basestring catches both types of Python 2.x strings
Phil>    if isinstance(sym, basestring)
Phil>         return True
Phil> except NameError:
Phil>    # If we are here, basestring does not exist, so Python 3.x
Phil>    if isinstance(sym, str)
Phil>         return True
Phil> # Continue to process objects that are not a string.

We can do this check once, at top-level:


try:
   if isinstance('hi', basestring):
      def is_string(x):
         return isinstance(x, basestring)
except NameError:
   def isinstance(x):
     return isinstance(x, str)


Maybe duck typing is still preferable though.

Not sure if this needs a third def in case the 'if' fails without throwing.
Probably not.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-21 14:59               ` Tom Tromey
@ 2013-08-21 15:37                 ` Paul_Koning
  2013-08-21 15:42                   ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Paul_Koning @ 2013-08-21 15:37 UTC (permalink / raw)
  To: tromey; +Cc: pmuldoon, gdb-patches


On Aug 21, 2013, at 10:59 AM, Tom Tromey <tromey@redhat.com> wrote:

> Phil> Strings in Python 3 are now always encoded and are encapsulated by the
> Phil> "str" class.
> 
> Phil> In Python 2 you had str() and unicode(), where unicode was encoded and
> Phil> str just represented bytes (IE just an unencoded string).
> 
> Phil> Reading around the suggestion seems to be to do this:
> 
> Phil> try:
> Phil>    # basestring catches both types of Python 2.x strings
> Phil>    if isinstance(sym, basestring)
> Phil>         return True
> Phil> except NameError:
> Phil>    # If we are here, basestring does not exist, so Python 3.x
> Phil>    if isinstance(sym, str)
> Phil>         return True
> Phil> # Continue to process objects that are not a string.
> 
> We can do this check once, at top-level:
> 
> 
> try:
>   if isinstance('hi', basestring):
>      def is_string(x):
>         return isinstance(x, basestring)
> except NameError:
>   def isinstance(x):
>     return isinstance(x, str)
> 
> 
> Maybe duck typing is still preferable though.
> 
> Not sure if this needs a third def in case the 'if' fails without throwing.
> Probably not.

I would write it this way:
try:
    basestring
except NameError:
    basestring = str

Similar techniques can be used for type "long" which isn't in Python 3 either.

 --paul

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-21 15:37                 ` Paul_Koning
@ 2013-08-21 15:42                   ` Tom Tromey
  0 siblings, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2013-08-21 15:42 UTC (permalink / raw)
  To: Paul_Koning; +Cc: pmuldoon, gdb-patches

Paul> I would write it this way:
Paul> try:
Paul>     basestring
Paul> except NameError:
Paul>     basestring = str

Nice trick!

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-21 14:56             ` Tom Tromey
@ 2013-08-22 10:46               ` Phil Muldoon
  2013-08-27 15:41                 ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Phil Muldoon @ 2013-08-22 10:46 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 21/08/13 15:56, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> For the hasattr point, I believe the equivalent check is the same in
> Phil> this version, as the incompatible Python 3 frame filters code.  But I
> Phil> use several methods of gdb.Symbol so which one would we use?  Remember
> Phil> this is not the Symbol/Value like interface that frame_args, and
> Phil> frame_locals returns, but the actual return of the symbol() function
> Phil> from that interface.  At this point in the code we have already called
> Phil> that symbol() API and have the object it returned.
> 
> We could pick any unique-to-Symbol method as a sentinel.
> Say, the first one called in this method.

I ended up using Paul's trick with basestring assignment later on this
thread. I think this OK? How about you?

This and other requested changes have been made. Is this OK to commit?

Cheers,

Phil

--

diff --git a/gdb/python/lib/gdb/FrameDecorator.py b/gdb/python/lib/gdb/FrameDecorator.py
index cacab4d..4e9cd7a 100644
--- a/gdb/python/lib/gdb/FrameDecorator.py
+++ b/gdb/python/lib/gdb/FrameDecorator.py
@@ -15,6 +15,15 @@
 
 import gdb
 
+# This small code snippet deals with problem of strings in Python 2.x
+# and Python 3.x.  Python 2.x has str and unicode classes which are
+# sub-classes of basestring.  In Python 3.x all strings are encoded
+# and basestring has been removed.
+try:
+    basestring
+except NameError:
+    basestring = str
+
 class FrameDecorator(object):
     """Basic implementation of a Frame Decorator"""
 
diff --git a/gdb/python/lib/gdb/FrameIterator.py b/gdb/python/lib/gdb/FrameIterator.py
index b3af94b..3f33bbb 100644
--- a/gdb/python/lib/gdb/FrameIterator.py
+++ b/gdb/python/lib/gdb/FrameIterator.py
@@ -43,3 +43,9 @@ class FrameIterator(object):
             raise StopIteration
         self.frame = result.older()
         return result
+
+    # Python 3.x requires __next__(self) while Python 2.x requires
+    # next(self).  Define next(self), and for Python 3.x create this
+    # wrapper.
+    def __next__(self):
+        return self.next()
diff --git a/gdb/python/lib/gdb/command/frame_filters.py b/gdb/python/lib/gdb/command/frame_filters.py
index 1b73059..b5d34ad 100644
--- a/gdb/python/lib/gdb/command/frame_filters.py
+++ b/gdb/python/lib/gdb/command/frame_filters.py
@@ -335,7 +335,10 @@ class SetFrameFilterPriority(gdb.Command):
 
         list_op = command_tuple[0]
         frame_filter = command_tuple[1]
-        priority = command_tuple[2]
+
+        # GDB returns arguments as a string, so convert priority to
+        # a number.
+        priority = int(command_tuple[2])
 
         op_list = gdb.frames.return_list(list_op)
 
diff --git a/gdb/python/lib/gdb/frames.py b/gdb/python/lib/gdb/frames.py
index 10dce8e..c148b98 100644
--- a/gdb/python/lib/gdb/frames.py
+++ b/gdb/python/lib/gdb/frames.py
@@ -108,13 +108,15 @@ def return_list(name):
     # cannot return a combined dictionary as keys() may clash in
     # between different dictionaries.  As we just want all the frame
     # filters to enable/disable them all, just return the combined
-    # items() as a list.
+    # items() as a chained iterator of dictionary values.
     if name == "all":
-        all_dicts = gdb.frame_filters.values()
-        all_dicts = all_dicts + gdb.current_progspace().frame_filters.values()
+        glob = gdb.frame_filters.values()
+        prog = gdb.current_progspace().frame_filters.values()
+        return_iter = itertools.chain(glob, prog)
         for objfile in gdb.objfiles():
-            all_dicts = all_dicts + objfile.frame_filters.values()
-            return all_dicts
+            return_iter = itertools.chain(return_iter, objfile.frame_filters.values())
+
+        return return_iter
 
     if name == "global":
         return gdb.frame_filters
@@ -140,14 +142,7 @@ def _sort_list():
                      execute.
     """
 
-    all_filters = []
-    for objfile in gdb.objfiles():
-        all_filters = all_filters + objfile.frame_filters.values()
-    cp = gdb.current_progspace()
-
-    all_filters = all_filters + cp.frame_filters.values()
-    all_filters = all_filters + gdb.frame_filters.values()
-
+    all_filters = return_list("all")
     sorted_frame_filters = sorted(all_filters, key = get_priority,
                                   reverse = True)
 
@@ -180,7 +175,7 @@ def execute_frame_filters(frame, frame_low, frame_high):
     """
 
     # Get a sorted list of frame filters.
-    sorted_list = _sort_list()
+    sorted_list = list(_sort_list())
 
     # Check to see if there are any frame-filters.  If not, just
     # return None and let default backtrace printing occur.
@@ -189,9 +184,13 @@ def execute_frame_filters(frame, frame_low, frame_high):
 
     frame_iterator = FrameIterator(frame)
 
-    # Apply a basic frame decorator to all gdb.Frames.  This unifies the
-    # interface.
-    frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
+    # Apply a basic frame decorator to all gdb.Frames.  This unifies
+    # the interface.  Python 3.x moved the itertools.imap
+    # functionality to map(), so check if it is available.
+    if hasattr(itertools,"imap"):
+        frame_iterator = itertools.imap(FrameDecorator, frame_iterator)
+    else:
+        frame_iterator = map(FrameDecorator, frame_iterator)
 
     for ff in sorted_list:
         frame_iterator = ff.filter(frame_iterator)
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index d62c596..11cec5c 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1166,17 +1166,21 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 
 	  if (py_func != NULL)
 	    {
-	      const char *function = NULL;
+	      const char *function;
 
 	      if (gdbpy_is_string (py_func))
 		{
-		  function = PyString_AsString (py_func);
+		  char *function_to_free = NULL;
+
+		  function = function_to_free =
+		    python_string_to_host_string (py_func);
 
 		  if (function == NULL)
 		    {
 		      Py_DECREF (py_func);
 		      goto error;
 		    }
+		  make_cleanup (xfree, function_to_free);
 		}
 	      else if (PyLong_Check (py_func))
 		{
@@ -1251,13 +1255,15 @@ py_print_frame (PyObject *filter, int flags, enum py_frame_args args_type,
 	    {
 	      if (py_fn != Py_None)
 		{
-		  char *filename = PyString_AsString (py_fn);
+		  char *filename = python_string_to_host_string (py_fn);
 
 		  if (filename == NULL)
 		    {
 		      Py_DECREF (py_fn);
 		      goto error;
 		    }
+
+		  make_cleanup (xfree, filename);
 		  TRY_CATCH (except, RETURN_MASK_ALL)
 		    {
 		      ui_out_wrap_hint (out, "   ");
diff --git a/gdb/testsuite/gdb.python/py-arch.exp b/gdb/testsuite/gdb.python/py-arch.exp
index 4e736b8..c0cada4 100644
--- a/gdb/testsuite/gdb.python/py-arch.exp
+++ b/gdb/testsuite/gdb.python/py-arch.exp
@@ -38,16 +38,16 @@ gdb_py_test_silent_cmd "python insn_list3 = arch.disassemble(pc, count=1)" \
 gdb_py_test_silent_cmd "python insn_list4 = arch.disassemble(pc)" \
   "disassemble no end no count" 0
 
-gdb_test "python print len(insn_list1)" "1" "test number of instructions 1"
-gdb_test "python print len(insn_list2)" "1" "test number of instructions 2"
-gdb_test "python print len(insn_list3)" "1" "test number of instructions 3"
-gdb_test "python print len(insn_list4)" "1" "test number of instructions 4"
+gdb_test "python print (len(insn_list1))" "1" "test number of instructions 1"
+gdb_test "python print (len(insn_list2))" "1" "test number of instructions 2"
+gdb_test "python print (len(insn_list3))" "1" "test number of instructions 3"
+gdb_test "python print (len(insn_list4))" "1" "test number of instructions 4"
 
 gdb_py_test_silent_cmd "python insn = insn_list1\[0\]" "get instruction" 0
 
-gdb_test "python print \"addr\" in insn" "True" "test key addr"
-gdb_test "python print \"asm\" in insn" "True" "test key asm"
-gdb_test "python print \"length\" in insn" "True" "test key length"
+gdb_test "python print (\"addr\" in insn)" "True" "test key addr"
+gdb_test "python print (\"asm\" in insn)" "True" "test key asm"
+gdb_test "python print (\"length\" in insn)" "True" "test key length"
 
 # Negative test
 gdb_test "python arch.disassemble(0, 0)" ".*gdb\.MemoryError.*" \
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index 806da94..63e4afb 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -40,7 +40,7 @@ gdb_py_test_silent_cmd "python bf1 = gdb.selected_frame ()" "get frame" 0
 
 # Test Frame.architecture() method.
 gdb_py_test_silent_cmd "python show_arch_str = gdb.execute(\"show architecture\", to_string=True)" "show arch" 0
-gdb_test "python print bf1.architecture().name() in show_arch_str" "True" "test Frame.architecture()"
+gdb_test "python print (bf1.architecture().name() in show_arch_str)" "True" "test Frame.architecture()"
 
 # First test that read_var is unaffected by PR 11036 changes.
 gdb_test "python print (bf1.read_var(\"i\"))" "\"stuff\"" "test i"
diff --git a/gdb/testsuite/gdb.python/py-framefilter-mi.exp b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
index 54fedf8..08d822c 100644
--- a/gdb/testsuite/gdb.python/py-framefilter-mi.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter-mi.exp
@@ -46,7 +46,7 @@ mi_runto main
 
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${pyfile}]
 
-mi_gdb_test "python execfile ('${remote_python_file}')" ".*\\^done." \
+mi_gdb_test "python exec (open ('${remote_python_file}').read ())" ".*\\^done." \
     "Load python file"
 
 # Multiple blocks test
diff --git a/gdb/testsuite/gdb.python/py-framefilter.exp b/gdb/testsuite/gdb.python/py-framefilter.exp
index 6c9946b..0664e03 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.exp
+++ b/gdb/testsuite/gdb.python/py-framefilter.exp
@@ -58,7 +58,8 @@ gdb_test_no_output "set python print-stack full" \
 
 # Load global frame-filters
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
-gdb_test_no_output "python execfile ('${remote_python_file}')" \
+
+gdb_test_no_output "python exec (open ('${remote_python_file}').read ())" \
     "Load python file"
 
 gdb_breakpoint [gdb_get_line_number "Backtrace end breakpoint"]
@@ -219,7 +220,8 @@ gdb_test_no_output "set python print-stack full" \
 
 # Load global frame-filters
 set remote_python_file [remote_download host ${srcdir}/${subdir}/${testfile}.py]
-gdb_test_no_output "python execfile ('${remote_python_file}')" \
+
+gdb_test_no_output "python exec (open ('${remote_python_file}').read ())" \
     "Load python file for no debuginfo tests"
 
 # Disable Reverse
diff --git a/gdb/testsuite/gdb.python/py-framefilter.py b/gdb/testsuite/gdb.python/py-framefilter.py
index d31bbbe..e70db11 100644
--- a/gdb/testsuite/gdb.python/py-framefilter.py
+++ b/gdb/testsuite/gdb.python/py-framefilter.py
@@ -70,8 +70,14 @@ class FrameFilter ():
         gdb.frame_filters [self.name] = self
 
     def filter (self, frame_iter):
-        frame_iter = itertools.imap (Reverse_Function,
-                                     frame_iter)
+        # Python 3.x moved the itertools.imap functionality to map(),
+        # so check if it is available.
+        if hasattr(itertools, "imap"):
+            frame_iter = itertools.imap (Reverse_Function,
+                                         frame_iter)
+        else:
+            frame_iter = map(Reverse_Function, frame_iter)
+
         return frame_iter
 
 class ElidingFrameDecorator(FrameDecorator):
@@ -102,6 +108,12 @@ class ElidingIterator:
         elided = next(self.input_iterator)
         return ElidingFrameDecorator(frame, [elided])
 
+    # Python 3.x requires __next__(self) while Python 2.x requires
+    # next(self).  Define next(self), and for Python 3.x create this
+    # wrapper.
+    def __next__(self):
+        return self.next()
+
 class FrameElider ():
 
     def __init__ (self):
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 8e1f9ed..f6b1d96 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -65,7 +65,7 @@ proc test_fields {lang} {
 
     if {$lang == "c++"} {
       # Test usage with a class
-      gdb_py_test_silent_cmd "print c" "print value (c)" 1
+      gdb_py_test_silent_cmd "print (c)" "print value (c)" 1
       gdb_py_test_silent_cmd "python c = gdb.history (0)" "get value (c) from history" 1
       gdb_py_test_silent_cmd "python fields = c.type.fields()" "get fields from c.type" 1
       gdb_test "python print (len(fields))" "2" "Check number of fields (c)"
@@ -78,7 +78,7 @@ proc test_fields {lang} {
     }
 
     # Test normal fields usage in structs.
-    gdb_py_test_silent_cmd "print st" "print value (st)" 1
+    gdb_py_test_silent_cmd "print (st)" "print value (st)" 1
     gdb_py_test_silent_cmd "python st = gdb.history (0)" "get value (st) from history" 1
     gdb_py_test_silent_cmd "python fields = st.type.fields()" "get fields from st.type" 1
     gdb_test "python print (len(fields))" "2" "Check number of fields (st)"
@@ -109,7 +109,7 @@ proc test_fields {lang} {
     gdb_test "python print (not not st.type\['a'\].type)" "True"
   
     # Test regression PR python/10805
-    gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+    gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
     gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
     gdb_test "python fields = ar.type.fields()"
     gdb_test "python print (len(fields))" "1" "Check the number of fields"
@@ -118,7 +118,7 @@ proc test_fields {lang} {
     # Test gdb.Type.array.
     gdb_test "python print (ar\[0\].cast(ar\[0\].type.array(1)))" \
         ".1, 2." "cast to array with one argument"
-    gdb_test "python print ar\[0\].cast(ar\[0\].type.array(0, 1))" \
+    gdb_test "python print (ar\[0\].cast(ar\[0\].type.array(0, 1)))" \
         ".1, 2." "cast to array with two arguments"
 
     gdb_test "python print (ar\[0\].type == ar\[0\].type)" "True"
@@ -126,26 +126,26 @@ proc test_fields {lang} {
     # Test gdb.Type.vector.
     # Note: vectors cast differently than arrays.  Here ar[0] is replicated
     # for the size of the vector.
-    gdb_py_test_silent_cmd "print vec_data_1" "print value (vec_data_1)" 1
+    gdb_py_test_silent_cmd "print (vec_data_1)" "print value (vec_data_1)" 1
     gdb_py_test_silent_cmd "python vec_data_1 = gdb.history (0)" "get value (vec_data_1) from history" 1
 
-    gdb_py_test_silent_cmd "print vec_data_2" "print value (vec_data_2)" 1
+    gdb_py_test_silent_cmd "print (vec_data_2)" "print value (vec_data_2)" 1
     gdb_py_test_silent_cmd "python vec_data_2 = gdb.history (0)" "get value (vec_data_2) from history" 1
 
     gdb_py_test_silent_cmd "python vec1 = vec_data_1.cast(ar\[0\].type.vector(1))" "set vec1" 1
-    gdb_test "python print vec1" ".1, 1." "cast to vector with one argument"
+    gdb_test "python print (vec1)" ".1, 1." "cast to vector with one argument"
     gdb_py_test_silent_cmd "python vec2 = vec_data_1.cast(ar\[0\].type.vector(0, 1))" "set vec2" 1
-    gdb_test "python print vec2" ".1, 1." "cast to vector with two arguments"
-    gdb_test "python print vec1 == vec2" "True"
+    gdb_test "python print (vec2)" ".1, 1." "cast to vector with two arguments"
+    gdb_test "python print (vec1 == vec2)" "True"
     gdb_py_test_silent_cmd "python vec3 = vec_data_2.cast(ar\[0\].type.vector(1))" "set vec3" 1
-    gdb_test "python print vec1 == vec3" "False"
+    gdb_test "python print (vec1 == vec3)" "False"
   }
 }
 
 proc test_enums {} {
   with_test_prefix "test_enum" {
-    gdb_py_test_silent_cmd "print e" "print value (e)" 1
-    gdb_py_test_silent_cmd "python e = gdb.history (0)" "get value (e) from history" 1
+    gdb_py_test_silent_cmd "print (e)" "print value (e)" 1
+    gdb_py_test_silent_cmd "python (e) = gdb.history (0)" "get value (e) from history" 1
     gdb_py_test_silent_cmd "python fields = e.type.fields()" "extract type fields from e" 1
     gdb_test "python print (len(fields))" "3" "Check the number of enum fields"
     gdb_test "python print (fields\[0\].name)" "v1" "Check enum field\[0\] name"
@@ -161,7 +161,7 @@ proc test_enums {} {
 }
 proc test_base_class {} {
   with_test_prefix "test_base_class" {
-    gdb_py_test_silent_cmd "print d" "print value (d)" 1
+    gdb_py_test_silent_cmd "print (d)" "print value (d)" 1
     gdb_py_test_silent_cmd "python d = gdb.history (0)" "get value (d) from history" 1
     gdb_py_test_silent_cmd "python fields = d.type.fields()" "extract type fields from d" 1
     gdb_test "python print (len(fields))" "3" "Check the number of fields"
@@ -174,7 +174,7 @@ proc test_range {} {
   with_test_prefix "test_range" {
     with_test_prefix "on ranged value" {
       # Test a valid range request.
-      gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+      gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
       gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
       gdb_test "python print (len(ar.type.range()))" "2" "Check correct tuple length"
       gdb_test "python print (ar.type.range()\[0\])" "0" "Check range low bound"
@@ -183,7 +183,7 @@ proc test_range {} {
 
     with_test_prefix "on ranged type" {
       # Test a range request on a ranged type.
-      gdb_py_test_silent_cmd "print ar" "print value (ar)" 1
+      gdb_py_test_silent_cmd "print (ar)" "print value (ar)" 1
       gdb_py_test_silent_cmd "python ar = gdb.history (0)" "get value (ar) from history" 1
       gdb_py_test_silent_cmd "python fields = ar.type.fields()" "get fields" 1
       gdb_test "python print (fields\[0\].type.range()\[0\])" "0" "Check range low bound"
@@ -192,7 +192,7 @@ proc test_range {} {
 
     with_test_prefix "on unranged value" {
       # Test where a range does not exist.
-      gdb_py_test_silent_cmd "print st" "print value (st)" 1
+      gdb_py_test_silent_cmd "print (st)" "print value (st)" 1
       gdb_py_test_silent_cmd "python st = gdb.history (0)" "get value (st) from history" 1
       gdb_test "python print (st.type.range())" "RuntimeError: This type does not have a range.*" "Check range for non ranged type."
     }


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-22 10:46               ` Phil Muldoon
@ 2013-08-27 15:41                 ` Tom Tromey
  2013-08-29 10:08                   ` Phil Muldoon
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2013-08-27 15:41 UTC (permalink / raw)
  To: Phil Muldoon; +Cc: gdb-patches

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> I ended up using Paul's trick with basestring assignment later on this
Phil> thread. I think this OK? How about you?

Yes, thanks, please put it in.
I'm sorry about the delay on this, I missed it somehow on the list.

Tom

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [patch] [python] Fix Python 3 build and testsuite issues
  2013-08-27 15:41                 ` Tom Tromey
@ 2013-08-29 10:08                   ` Phil Muldoon
  0 siblings, 0 replies; 15+ messages in thread
From: Phil Muldoon @ 2013-08-29 10:08 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

On 27/08/13 16:41, Tom Tromey wrote:
>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
> 
> Phil> I ended up using Paul's trick with basestring assignment later on this
> Phil> thread. I think this OK? How about you?
> 
> Yes, thanks, please put it in.
> I'm sorry about the delay on this, I missed it somehow on the list.
> 
> Tom
> 


So committed with the following modified ChangeLog entries:

2013-08-29  Phil Muldoon  <pmuldoon@redhat.com>

	* python/py-framefilter.c (py_print_frame): Remove usage of
	PyString_AsString.  Use python_string_to_host_string instead.
	Refactor function to work with a string as a new allocation
	instead of a pointer.
	(py_print_frame): Ditto.
	* python/lib/gdb/frames.py (return_list): Chain iterators together
	instead of adding them as a list.
	(_sort_list): Call return_list, and remove duplicate code.
	(execute_frame_filters): Convert iterator to a list with list().
	* python/lib/gdb/command/frame_filters.py
	(SetFrameFilterPriority._set_filter_priority): Convert priority
	attribute to an integer.
	* python/lib/gdb/FrameIterator.py (FrameIterator.next): Define
	wrapper function __next__.
	* python/lib/gdb/FrameDecorator.py: If basestring not defined,
	define as "str".


2013-08-29  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/py-framefilter.py (FrameFilter.filter): Check
	itertools for imap attribute.  Otherwise use map().
	(ElidingIterator): Define wrapper function __next__.
	* gdb.python/py-framefilter-mi.exp: Do not use execfile,
	use exec (open (read ())) instead.
	* gdb.python/py-framefilter.exp: Ditto.
	* gdb.python/py-arch.exp: Update print based test to Python 3.x
	compliance.
	* gdb.python/py-frame.exp: Ditto.
	* gdb.python/py-type.exp: Ditto.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2013-08-29 10:08 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-19 14:50 [patch] [python] Fix Python 3 build and testsuite issues Phil Muldoon
2013-08-19 16:19 ` Tom Tromey
2013-08-19 16:45   ` Phil Muldoon
2013-08-19 18:34     ` Tom Tromey
2013-08-20 19:43       ` Phil Muldoon
2013-08-20 19:59         ` Tom Tromey
2013-08-20 20:32           ` Phil Muldoon
2013-08-21 14:29             ` Phil Muldoon
2013-08-21 14:59               ` Tom Tromey
2013-08-21 15:37                 ` Paul_Koning
2013-08-21 15:42                   ` Tom Tromey
2013-08-21 14:56             ` Tom Tromey
2013-08-22 10:46               ` Phil Muldoon
2013-08-27 15:41                 ` Tom Tromey
2013-08-29 10:08                   ` Phil Muldoon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).