Shinmera / qtools

Qtools is a collection of utilities to aid development with CommonQt
https://shinmera.github.io/qtools
zlib License
210 stars 17 forks source link

with-finalizing* finalizes only the first binding #3

Closed orivej closed 9 years ago

orivej commented 9 years ago

For example, when with-finalizing* expands to

(let ((#:values1308 nil))
  (unwind-protect
      (let* ((red-color (q+ "MAKE-QCOLOR" 255 0 0))
             (#:values1308 (push red-color #:values1308))
             (red-brush (q+ "MAKE-QBRUSH" red-color))
             (#:values1308 (push red-brush #:values1308)))
        (q+ "ADD-ELLIPSE" scene 0 0 100 100 (q+ "MAKE-QPEN") red-brush)
        (q+ "ADD-RECT" scene 100 0 100 100 (q+ "MAKE-QPEN") red-brush))
    (mapc #'finalize #:values1308)))

the first inner binding for #:values1308 shadows its outer binding, and only red-color gets finalized. It seems that with-finalizing* should be implemented just like with-finalizing with let* instead of let.

Shinmera commented 9 years ago

Yeah, looks like I had a different idea about how this should work at some point, but only fixed one of them. Though to be honest, the way it's done now isn't exactly pleasing to me. I'll see about it.

orivej commented 9 years ago

Sorry, but 2a66e6d would not work: in

(let (#:red-color1326 #:red-brush1327)
  (unwind-protect
      (let* ((#:red-color1326 (q+ "MAKE-QCOLOR" 255 0 0))
             (red-color #:red-color1326)
             (#:red-brush1327 (q+ "MAKE-QBRUSH" red-color))
             (red-brush #:red-brush1327))
        (q+ "ADD-ELLIPSE" scene 0 0 100 100 (q+ "MAKE-QPEN") red-brush)
        (q+ "ADD-RECT" scene 100 0 100 100 (q+ "MAKE-QPEN") red-brush))
    (finalize #:red-brush1327)
    (finalize #:red-color1326)))

finalization happens out of scope of inner let*, where temporaries are not initialized, and this also would not be accepted by let where further bindings can not reference preceding ones.

Shinmera commented 9 years ago

Your first issue is correct, but the second one isn't. You can't reference gensyms. I think I see now why I went the gross route of pushing things to a list before. Eugh.

orivej commented 9 years ago

I meant that (let ((#:red-color1326 (q+ "MAKE-QCOLOR" 255 0 0)) (red-color #:red-color1326) is not compilable because red-color can not reference preceding #:red-color1326 in let.

Shinmera commented 9 years ago

Ahh-- right. I'm stupid. I really shouldn't be doing stuff like this early in the morning, sorry.

Shinmera commented 9 years ago

Can you confirm it working now? I don't trust myself during mornings anymore.

orivej commented 9 years ago

Nice fix. Thanks!