clean up use of unsafePartial

[matthewleon]

A bit of code cleanup that restricts use of unsafePartial to where it's really needed.

[hdgarrood]

Thanks, but the reason it's been done this way is to ensure that unsafePartial is only evaluated once, so that we're not constructing and calling extra functions unnecessarily.

[matthewleon]

I thought unsafePartial is optimized away by the compiler?

[matthewleon]

purescript/purescript@3f1a50e

[matthewleon]

And even if that weren't the case, I would understand leaving up the way it is, but the usage of unsafePartial in down still is a bit unclear to me.

[hdgarrood]

I suppose it's possible these are actually equivalent, then. Is there a difference in the generated code? With the code the way it is now, it is at least crystal clear that even if the unsafePartial inlining fails for whatever reason, you'll still only evaluate unsafePartial once.

[hdgarrood]

Just FYI, I'm becoming less keen on refactoring changes which don't change behaviour, because I think the maintenance effort (i.e. reviewing, merging, releasing, etc) effort to payoff ratio is quite small compared to other work I could be doing, and also because it makes git blame less useful.

[matthewleon]

Ah, fair enough. That makes sense.

FWIW, the generated code is indeed equivalent. I get what you are saying as regards the change to up, which shifts unsafePartial to the other side of a function arrow, but the change in down simply eliminates unsafePartial entirely.

That said, I understand the hesitation regarding merging of refactoring changes. I was working on some other, more useful changes, of which these were a (tiny) part. Perhaps if those changes make sense, we can include this kind of thing in there.

[hdgarrood]

Ok, thanks, that's good to know. Re down: oops, I didn't notice that! Since unsafePartial isn't actually needed in down then I think that crosses the threshold of usefulness to be worth considering on its own.

[hdgarrood]

Well that's not quite true; I did notice but I failed to consider the implications properly.

[matthewleon]

I've amended the branch accordingly. If you think it's worth incorporating the change to down please feel free to reopen the PR.

[hdgarrood]

GitHub won't let me reopen for some reason, would you mind sending a new PR?