Re: [patch] [python] Fix Python 3 build and testsuite issues

[Phil Muldoon <pmuldoon@redhat.com>]

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