Re: [Chicken-hackers] [PATCH] Fix #1620 by ignoring captured state of replaced variables

[megane]

Peter Bex <address@hidden> writes:

> Hi all,
>
> I had a look at #1620 and as far as I can tell there's no reason why
> an aliased variable cannot be marked as replaceable when either the
> alias or the variable it aliases are captured.
>
> Captured simply means that it needs to be wrapped up in the closure,
> AFAIK.  But if it's replaced, then the original variable will need
> to end up in the closure.  The only case where you can't replace is
> if either variable is assigned to, because then they don't point to
> the same thing anymore.

I agree with this.

There's this case where a small improvement could be made. The
replacement is OK if the replacement is only referenced once:

  (let* ((a 1)
         (b a))
    (set! b 2)
    b)

Except in this case (from compiler-tests.scm):

  (let ((outer-bar (##core#undefined)))
    (let ((inner-bar (lambda (x) (if x '1 (outer-bar outer-bar)))))
    (set! outer-bar inner-bar)
    (outer-bar '#f)))

This test would catch the above case:

  (or (not assigned)
      (and (not (db-get db name 'global))
           (not captured)
           (let ((refs (db-get db name 'references)))
             (= 1 (length refs)))))

The added complexity is probably not worth it, though.

###

Regarding the capturing, the capture test is still there:


>        (when (and value (not global))
>          (when (eq? '##core#variable (node-class value))
> -          (let* ((name (first (node-parameters value)))
> -                 (nrefs (db-get db name 'references)) )
> +          (let ((name (first (node-parameters value))) )
>              (when (and (not captured)
                          ^^^^^^^^^^^^^^
                          Here.
I got the impression you wanted to remove this.

>                         (or (and (not (db-get db name 'unknown))
>                                  (db-get db name 'value))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This test prevents the replacing of formal lambda parameters:

  (lambda (a)
    (let ((b a))
      b))
  -->
  (lambda (a) a)

I don't see a reason for preventing replacement in this case.

This seems to work just fine for the full test:

  (and (not assigned)
       (not (db-get db name 'assigned))
       (or (not (variable-visible? name block-compilation))
       (not (db-get db name 'global))))

> -                           (and (not (db-get db name 'captured))
> -                                nrefs
> -                                (= 1 (length nrefs))
> -                                (not assigned)
> +                           (and (not assigned)
>                                  (not (db-get db name 'assigned))
>                                  (or (not (variable-visible?
>                                            name block-compilation))