purescript icon indicating copy to clipboard operation
purescript copied to clipboard

Fix typo in CoreFn traversal function

Open drathier opened this issue 11 months ago • 5 comments

This seems to be a critical unintentional typo

drathier avatar Dec 29 '24 01:12 drathier

Good catch! Please add an internal_ file to CHANGELOG.d per the README.md contained therein, unless this is connected to a bug you've observed, in which case make it fix_ and describe the bug.

rhendric avatar Dec 29 '24 22:12 rhendric

Is this change is correct? As, per the comment above the function:

This function is useful as a building block for recursive functions, but doesn't actually recurse itself.

Note also that the internal definitions aren't used recursively in any other instance either. But this is your function @rhendric so I guess you would know better. :smile:

garyb avatar Dec 30 '24 00:12 garyb

I think this is correct, but it's been a while since I wrote it.

What the doc comment says to me is that if you want to make an actually recursive traversal, you do that by calling the results of traverseCoreFn from within the arguments to traverseCoreFn. traverseCoreFn shouldn't be tying its own knot.

I think the typo ended up having no real effect, because the one use of traverseCoreFn is here, and the function that is named g' inside traverseCoreFn and named handleExpr at that use site is only ever invoked with arguments that are not Let, because of how clauses in a \case expression are ordered. So this is unused code; but if it were to be used, I think the thing that we should expect it to do is call g and not g', as the other branches do.

Edit: Ah, there's another use here, but it's the same story.

rhendric avatar Dec 30 '24 01:12 rhendric

I'll do the requested changes to changelog etc soon, probably next year :)

drathier avatar Dec 30 '24 13:12 drathier

Looks like CI needs some love, we'll need to upgrade some actions

Also the ARM runners are not responsive so I'll have a look at what's up with those

f-f avatar Feb 14 '25 23:02 f-f