Skip to content
New issue

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

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

Already on GitHub? # to your account

Suggest using in-inclusive-range instead of in-range #153

Open
jackfirth opened this issue Mar 31, 2022 · 3 comments · May be fixed by #376
Open

Suggest using in-inclusive-range instead of in-range #153

jackfirth opened this issue Mar 31, 2022 · 3 comments · May be fixed by #376
Labels
new lint Issues suggesting new lints or pull requests implementing new lints

Comments

@jackfirth
Copy link
Owner

This code:

(for ([x (in-range a (add1 b))])
  body ...)

Can be refactored to this:

(for ([x (in-inclusive-range a b)])
  body ...)

See this discussion with @sorawee for some tricky details:

notjack
okay so in-inclusive-range is harder than I thought to handle correctly, because when it's used directly in a for form the (in-inclusive-range ...) expression is never actually seen by the macro expander. the for macro rips it apart before the expander ever sees it. and resyntax searches for expressions to rewrite by observing the macro expander's behavior, so it can't see them.
sorawee
Oh, I still thought that Resyntax only analyzes surface syntax. That's not the case anymore?
This is very cool
notjack
resyntax analyzes every expression that appears during macro expansion, though it only tries to apply rules to syntax-original? expressions - that's why resyntax can refactor things correctly even in the face of renamings and shadowings
the downside is that if an expression is invisible to the expander, then it's invisible to resyntax as well
in theory maybe it could work to reuse the disappeared uses information that macros emit for check syntax to figure out that some expressions are part of the program even though they're never expanded
sorawee
FYI: Typed Racket has a functionality to recover correspondence between surface and expanded code. Not sure if it will be helpful to you.
https://github.com/racket/typed-racket/blob/master/source-syntax/source-syntax.rkt
notjack
oh cool that's neat! that might help to go from an in-range identifier in a disappeared uses property to its enclosing (in-range ...) expression

@jackfirth jackfirth added the new lint Issues suggesting new lints or pull requests implementing new lints label Mar 31, 2022
@jackfirth jackfirth changed the title Use in-inclusive-range Suggest using in-inclusive-range instead of in-range Mar 31, 2022
@jackfirth
Copy link
Owner Author

Found a tricky case that the 'disappeared-use strategy can't solve:

(let-syntax ([add1 (syntax-rules () [(_ x) x])])
  (for ([i (in-range 1 (add1 5))])
    (displayln i)))

The add1 is shadowed, so this isn't safe to refactor to in-inclusive-range. However, to tell that it's shadowed, we need to analyze the #'(in-range 1 (add1 5)) syntax object with the scopes that were present on it at the time the expander visited it indirectly via the for expression. We can't rely on Resyntax's existing logic to reconstruct scopes from fully expanded code, because the add1 identifier in the (in-range 1 (add1 5)) expression won't appear in the fully expanded code.

@jackfirth
Copy link
Owner Author

I've got a working implementation of this rule that relies on a patch to Racket to add a 'disappeared-visit property.

@jackfirth
Copy link
Owner Author

See also #369.

jackfirth added a commit that referenced this issue Oct 20, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
new lint Issues suggesting new lints or pull requests implementing new lints
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant