Fix LocationFunc crash

Also change it to error if the argument is not a function, and to
return fail (instead of an empty string) if the location can not
be determined.

Finally, make the underlying kernel function `GET_LOCATION_BODY` resilient
about function bodies that store no location information at all,
as produced by the code which turns syntax tree object back into
functions. This previously crashed.

lib/function.g

@@ -313,47 +313,48 @@ BIND_GLOBAL( "EndlineFunc", ENDLINE_FUNC );
 ##
 ##  <Description>
 ##  Let <A>func</A> be a function.
-##  Returns a string describing the location of <A>func</A>, or an empty
-##  string if the information cannot be found. This uses the information
+##  Returns a string describing the location of <A>func</A>, or <K>fail</K>
+##  if the information cannot be found. This uses the information
 ##  provided by <Ref Func="FilenameFunc"/> and <Ref Func="StartlineFunc"/>
 ##  <P/>
 ##  <Log><![CDATA[
 ##  gap> LocationFunc( Intersection );
 ##  "... some path ... gap/lib/coll.gi:2467"
 ##  # String is an attribute, so no information is stored
 ##  gap> LocationFunc( String );
-##  ""
+##  fail
 ##  ]]></Log>
 ##  </Description>
 ##  </ManSection>
 ##  <#/GAPDoc>
 ##
-BIND_GLOBAL( "LocationFunc", function(x)
+BIND_GLOBAL( "LocationFunc", function(func)
     local nam, line, ret;
     # If someone passes something which isn't a true function,
     # like a method or attribute, just return.
-    if not(IS_FUNCTION(x)) then
-        return "";
+    if not IS_FUNCTION(func) then
+        Error("<func> must be a function");
     fi;
     ret := "";
-    nam := FILENAME_FUNC(x);
+    nam := FILENAME_FUNC(func);
     if nam = fail then
-        return ret;
+        return fail;
     fi;
-    line := STARTLINE_FUNC(x);
+    line := STARTLINE_FUNC(func);
     if line <> fail then
         APPEND_LIST(ret, nam);
         APPEND_LIST(ret, ":");
         APPEND_LIST(ret, STRING_INT(line));
         return ret;
     fi;
-    line := LOCATION_FUNC(x);
+    line := LOCATION_FUNC(func);
     if line <> fail then
         APPEND_LIST(ret, nam);
         APPEND_LIST(ret, ":");
         APPEND_LIST(ret, line);
+        return ret;
     fi;
-    return ret;
+    return fail;
 end);
 
 

lib/methwhy.g

@@ -182,7 +182,7 @@ local oper,narg,args,skip,verbos,fams,flags,i,j,methods,flag,flag2,
       Print("#I  Method ",i,": ``",nam,"''");
       if IsBound(m.location) then
         Print(" at ", m.location[1], ":", m.location[2]);
-      elif LocationFunc(oper) <> "" then
+      elif LocationFunc(oper) <> fail then
         Print(" at ",LocationFunc(oper));
       fi;
       Print(", value: ");
@@ -217,7 +217,7 @@ local oper,narg,args,skip,verbos,fams,flags,i,j,methods,flag,flag2,
 	  Print("#I  Method ",i,": ``",nam,"''");
           if IsBound(m.location) then
             Print(" at ", m.location[1], ":", m.location[2]);
-	  elif LocationFunc(oper) <> "" then
+	  elif LocationFunc(oper) <> fail then
 	    Print(" at ",LocationFunc(oper));
 	  fi;
 	  Print(" , value: ");

src/code.c

@@ -211,7 +211,7 @@ void SET_GAPNAMEID_BODY(Obj body, UInt val)
 Obj GET_LOCATION_BODY(Obj body)
 {
     Obj location = BODY_HEADER(body)->startline_or_location;
-    return IS_STRING_REP(location) ? location : 0;
+    return (location && IS_STRING_REP(location)) ? location : 0;
 }
 
 void SET_LOCATION_BODY(Obj body, Obj val)

tst/testinstall/opers/LocationFunc.tst

@@ -1,5 +1,9 @@
 gap> START_TEST("LocationFunc.tst");
 
+#
+gap> LocationFunc(fail);
+Error, <func> must be a function
+
 # regular GAP function
 gap> f:=x->x;;
 gap> LocationFunc(f);
@@ -24,5 +28,13 @@ true
 gap> LocationFunc(APPEND_LIST_INTR);
 "src/listfunc.c:APPEND_LIST_INTR"
 
+# String is an attribute, so no information is stored
+gap> LocationFunc( String );
+fail
+
+# functions created from a syntax tree have no location
+gap> LocationFunc(SYNTAX_TREE_CODE(SYNTAX_TREE(x->x)));
+fail
+
 #
 gap> STOP_TEST("LocationFunc.tst", 1);