jackfirth / resyntax

A Racket refactoring engine
Apache License 2.0
58 stars 10 forks source link

Suggest using `list` instead of unnecessary quasiquoting #123

Closed jackfirth closed 1 year ago

jackfirth commented 3 years ago

Saw this code today:

`(,(mod-full-name m) ,(mod-mappings m))

The quasiquoting is doing nothing at all here. The above is equivalent to (list (mod-full-name m) (mod-mappings m)).

rocketnia commented 3 years ago

Here's some more context from racket/collects/compiler/embed.rkt line 867:

    `(module #%resolver '#%kernel
       (let-values ([(orig) (current-module-name-resolver)]
                    [(regs) (make-hasheq)]
                    [(mapping-table) (quote
                                      ,(map
                                        (lambda (m)
                                          `(,(mod-full-name m)
                                            ,(mod-mappings m)))
                                        code-l))]
                    <...>)
         <...>))

The s-expressions (<full-name> <mappings>) are pieces of code which are being inserted into a larger (module <...>) form. The use of a quotation operator makes it clear that these parentheses belong to quoted code like the others do.

jackfirth commented 3 years ago

I don't think it makes that clear at all, especially since the (list (mod-full-name m) (mod-mappings m)) being built in that code isn't quoted code: because it's being inserted under a quote in the template, it's not code, it's data. It's a list of two-element lists, and that list isn't being interpreted or evaluated at all. It's just a table treated like an ordinary data structure by the surrounding code it's inserted into.

I think there's plenty of reasonable uses of quasiquotation. But I don't think that just because a datum is being inserted into a code template means that datum should be constructed with quasiquotation. I'm not sure where the line is, but I think "could have been constructed with a single list call without any quoting at all" is definitely on the "too much quotation" side of the line.

(You might also be interested in #122, which is an analogous situation for hashes.)

rocketnia commented 3 years ago

I don't think it makes that clear at all, especially since the (list (mod-full-name m) (mod-mappings m)) being built in that code isn't quoted code: because it's being inserted under a quote in the template, it's not code, it's data. It's a list of two-element lists, and that list isn't being interpreted or evaluated at all. It's just a table treated like an ordinary data structure by the surrounding code it's inserted into.

I actually agree with you there, and I just didn't bring that up because it might be a confusing tangent.

That's because if I refactor that quote into list calls the way I like it, we end up with this:

    `(module #%resolver '#%kernel
       (let-values ([(orig) (current-module-name-resolver)]
                    [(regs) (make-hasheq)]
                    [(mapping-table) (list
                                      ,@(map
                                         (lambda (m)
                                           `(list ,(mod-full-name m)
                                                  ,(mod-mappings m)))
                                         code-l))]
                    <...>)
         <...>))

And now instead of `(,(mod-full-name m) ,(mod-mappings m)), we have `(list ,(mod-full-name m) ,(mod-mappings m)), where the use of quasiquotation helps clarify that list is an operator that's being invoked.

I get the impression that people use quote pretty often with association lists and other cons-based data structures. These data structures aren't common in my own use of Racket, and as such I haven't become a particular fan of using quasiquotation for this purpose, but I suspect it's the main factor in why the code is currently written this way. I suspect the same is true of the association list in #122.

In this case there's a second potential concern, performance. I'm not sure how much this plays a part here, but if it is the main concern, then I'm with you: Everything under the quote isn't code; it's a quoted datum that only incidentally gets represented as an s-expression, and (list (mod-full-name m) (mod-mappings m)) is the way I would prefer building it up.

jackfirth commented 3 years ago

I think I wouldn't suggest rewriting (list ,(mod-full-name m) ,(mod-mappings)). For quasiquoted lists, I'd probably only rewrite cases where everything in the list is immediately unquoted.