Re: [Axiom-developer] vmlisp

[Waldek Hebisch]

Gabriel Dos Reis wrote:
> Gabriel Dos Reis <address@hidden> writes:
> 
> [...]
> 
> | The motivation of moving VMLISP to BOOT is not that we have too many
> | packages. This has nothing to do with Lisp package mehcanism.
> | The motivation for merging both is that we end up with codes that
> | do not know where to pick their symbols from, duplicate definitions in 
> | both packages, and more confusion.  Codes that are so tied should be in one
> | logical unit.
> 
> I realize the above statements may sound very abstract to many.  So,
> let me give an example.
> 
> Take the following fragment, at the scope of package BOOT, from
> src/interp/setq.lisp: 
> 
>     (SETQ ERRORINSTREAM (DEFIOSTREAM
>        '((DEVICE . CONSOLE) (MODE . INPUT) (QUAL . T)) 133 1))
> 
>     (SETQ ERROROUTSTREAM
>      (DEFIOSTREAM '((DEVICE . CONSOLE)(MODE . OUTPUT)) 80 0) )
> 
>     (SETQ |$algebraOutputStream|
>      (DEFIOSTREAM '((DEVICE . CONSOLE)(MODE . OUTPUT)) 255 0) )
> 
> 
> First let me observe that, in reality they are not needed, and I don't
> think they actually work as expected.
> 
> In the above fragment, DEFIOSTREAM is a helper function defined in
> package VMLISP (from src/interp/vmlisp.lisp):
> 
>    (defun DEFIOSTREAM (stream-alist buffer-size char-position)
>     (declare (ignore buffer-size))
>       (let ((mode (or (cdr (assoc 'MODE stream-alist)) 'INPUT))
>             (filename (cdr (assoc 'FILE stream-alist)))
>             (dev (cdr (assoc 'DEVICE stream-alist))))
>          (if (EQ dev 'CONSOLE) (make-synonym-stream '*terminal-io*)
>            (let ((strm (case mode
>                              ((OUTPUT O) (open (make-filename filename)
>                                                :direction :output))
>                              ((INPUT I) (open (make-input-filename filename)
>                                               :direction :input)))))
>              (if (and (numberp char-position) (> char-position 0))
>                  (file-position strm char-position))
>              strm))))
> 
> 
> *If* everything goes well the variables ERRORINSTREAM, ERROROUTSTREAM,
> and |$algebraOutputStream| should be alias for *terminal-io*.  For
> that to work, there ought to be that the symbols CONSOLE from packages
> VMLISP (where the function DEFIOSTREAM is defined) and package BOOT
> (where DEFIOSTREAM is called from) are EQ.  They not EQ.
> 
> 

I am not sure abot last part of the analysis, freshly build
interpsys tells me:

(1) -> )lisp |$algebraOutputStream|

Value = #<synonym stream to *TERMINAL-IO*>
(1) -> )lisp (eq 'VMLISP::CONSOLE 'BOOT::CONSOLE)

Value = T
(1) ->

so IMHO the code _may_ work.  AFAICS ERRORINSTREAM, ERROROUTSTREAM are
unused.  |$algebraOutputStream| is also initialized in other places
and it looks that initialization in patches.lisp wins.

So I think that the snippet above is a prime candidate for removal.
Once we get rid of all uses we will be able to remove DEFIOSTREAM.
We should also simplify RDEFIOSTREAM (it looks that RDEFIOSTREAM uses
the same funky argument format as DEFIOSTREAM).

Let me mention other candidates for removal.  compDefineCapsuleFunction
is defined in two places: in define.boot.pamphlet and in
br-saturn.boot.pamphlet.  I checked that copy in br-saturn is
used during algebra build.  Copy in define.boot.pamphlet is slightly
different.  So IMHO copy in br-saturn should be removed and copy in
define.boot.pamphlet should be changed to match version from br-saturn.

In general, when we multiple versions of code which are doing the
same thing (for example initializing |$algebraOutputStream|) we
should check which copy is used and remove the other ones (this may
require moving/changing code to account for load order).

Another candidate is the following:

--- pp2/wh-20070421/src/interp/postpar.boot.pamphlet    Sun Apr 22 00:24:35 
2007+++ wh-20070421/src/interp/postpar.boot.pamphlet        Sat May  5 21:30:02 
2007@@ -259,15 +259,7 @@

 postForm (u is [op,:argl]) ==
   x:=
-    atom op =>
-      argl':= postTranList argl
-      op':=
-        true=> op
-        $BOOT => op
-        GETL(op,'Led) or GETL(op,'Nud) or op = 'IN => op
-        numOfArgs:= (argl' is [['Tuple,:l]] => #l; 1)
-        INTERNL("*",STRINGIMAGE numOfArgs,PNAME op)
-      [op',:argl']
+    atom op => [op,:postTranList argl]
     op is ['Scripts,:.] => [:postTran op,:postTranList argl]
     u:= postTranList u
     if u is [['Tuple,:.],:.] then


The true branch means that other parts are never executed.  Once
such dead branches in conditionals are removed it may became
apparent that some other code is never referenced.

In general I remove code when I reasonably convinced that the
code is useless:

1) duplicates
2) dead conditional branches (I spare them if I feel that the
   code may be usefull in the furture)
3) unreferenced code

Concerning conditional branches: I sometimes do flow analysis or
track variables to determine if branch is dead -- usually I do
this when the code looks very wrong (say references non existent
functions).

-- 
                              Waldek Hebisch
address@hidden