Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

when in for loop #213

Closed
sorawee opened this issue Aug 30, 2023 · 4 comments
Closed

when in for loop #213

sorawee opened this issue Aug 30, 2023 · 4 comments
Labels
new lint Issues suggesting new lints or pull requests implementing new lints

Comments

@sorawee
Copy link

sorawee commented Aug 30, 2023

Rewrites

(for (....)
  (when ... .....))

to

(for (....
      #:when ...)
  ....)
@Metaxal
Copy link
Collaborator

Metaxal commented Aug 30, 2023

Personally, I don't like this transformation. Syntactically, the test gets substantial rightward drift even though the body is less indented. The intent is also less clear because the #:when clause behaves in a rather subtle way.

(I got caught by a logical mistake with a #:when clause last, but I can't recall what it was.)

@Metaxal
Copy link
Collaborator

Metaxal commented Aug 31, 2023

This is probably more helpful:

(for (clause1 ...)
  (when test
    (for (clause2 ...)
      body ...)))
;; ->
(for (clause1 ...
      #:when test
      clause2 ...)
  body ...)

But I think @jackfirth wants to see an example in the wild before adding a rule.

A very beneficial use case would be for code that tries to do for/list with #:when but using nested for/list and append instead, or using an external accumulator. The #:when version is a big save there.

@jackfirth
Copy link
Owner

jackfirth commented Aug 31, 2023

I like this rule, since it unlocks further for loop simplifications. For example, that for loop flattening that @Metaxal suggested is already implemented. The (for ... (when ... (for ...))) example would be simplified in two passes if we had this rule.

I'm sympathetic to the idea that #:when is a bit less intuitive than (when ...). Perhaps if we limited this to cases where the condition expression is short? Limiting it only to cases that unlock further simplifications is also doable, but it's more work.

@jackfirth jackfirth added the new lint Issues suggesting new lints or pull requests implementing new lints label Aug 31, 2023
@jackfirth
Copy link
Owner

I implemented this in #249 and forgot that this issue exists 😅

I implemented it because I hit a few loops in real-world code that this would simplify. It appears that complex condition expressions in when and unless aren't too common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new lint Issues suggesting new lints or pull requests implementing new lints
Projects
None yet
Development

No branches or pull requests

3 participants