Skip to content

Conversation

@recursion-ninja
Copy link
Collaborator

Update the .hlint.yaml to require using pure instead of return. This change is intended to future-proof the code-base for the "Monad of No Return" proposal.

The changes touch a lot of the code-base and if the churn is deemed too high for negligible benefits, we can delete this pull request.

@recursion-ninja recursion-ninja force-pushed the recursion-ninja/hlint-no-return branch 2 times, most recently from 9f384c5 to 8be794c Compare May 12, 2025 17:10
@jorisdral
Copy link
Collaborator

Update the .hlint.yaml to require using pure instead of return. This change is intended to future-proof the code-base for the "Monad of No Return" proposal.

The changes touch a lot of the code-base and if the churn is deemed too high for negligible benefits, we can delete this pull request.

Would return be removed from base because of the monad of no return proposal? I can't easily see that from the link you posted

Copy link
Collaborator

@wenkokke wenkokke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, but it might be best to not merge this until just before we release version 1.0.0, so that there's fewer PRs impacted by this.

@wenkokke
Copy link
Collaborator

wenkokke commented May 12, 2025

Update the .hlint.yaml to require using pure instead of return. This change is intended to future-proof the code-base for the "Monad of No Return" proposal.

The changes touch a lot of the code-base and if the churn is deemed too high for negligible benefits, we can delete this pull request.

Would return be removed from base because of the monad of no return proposal? I can't easily see that from the link you posted

I think the proposal is to make return a top level function rather than a member of the Monad class.

The reason to prioritise pure over return would be to not require a Monad constraint where it isn't needed. The proposal would take care of this by removing the Monad constraint from return.

However, I do support using pure over return for consistency reasons.

@recursion-ninja
Copy link
Collaborator Author

However, I do support using pure over return for consistency reasons.

I'm of a similar mind.

This looks good to me, but it might be best to not merge this until just before we release version 1.0.0, so that there's fewer PRs impacted by this.

Agreed, let's let this sit for a bit and revisit it just before or just after cutting the v1.0.0 release.

@wenkokke
Copy link
Collaborator

This looks good to me, but it might be best to not merge this until just before we release version 1.0.0, so that there's fewer PRs impacted by this.

Agreed, let's let this sit for a bit and revisit it just before or just after cutting the v1.0.0 release.

Let's do just before.

@recursion-ninja
Copy link
Collaborator Author

This looks good to me, but it might be best to not merge this until just before we release version 1.0.0, so that there's fewer PRs impacted by this.

Agreed, let's let this sit for a bit and revisit it just before or just after cutting the v1.0.0 release.

Let's do just before.

My preference as well 👍.

@jorisdral
Copy link
Collaborator

I'd probably just rip off the band-aid and it merge it 😛

Copy link
Collaborator

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this misunderstand the Monad Of No Return proposal? It would not eliminate return, it would just make it a function that's not a method of the Monad class. So there's no need to switch from return to pure. The only compatibility thing would be to make sure any Monad instances we have (and I can't immediately think of any) don't define a custom return or >>.

@dcoutts
Copy link
Collaborator

dcoutts commented May 22, 2025

The warning was implemented in GHC 8.0 and is called -Wnoncanonical-monad-instances (there are variants of this warning flag for Monoid and Fail) is included in the default warning set since GHC 9.2.

So we must be clear w.r.t. these warnings. So there's nothing to do right?

@wenkokke
Copy link
Collaborator

Doesn't this misunderstand the Monad Of No Return proposal? It would not eliminate return, it would just make it a function that's not a method of the Monad class. So there's no need to switch from return to pure. The only compatibility thing would be to make sure any Monad instances we have (and I can't immediately think of any) don't define a custom return or >>.

Yes, it's very much just (1) consistency, and (2) avoiding unnecessary monad constraints before that proposal lands.

@recursion-ninja
Copy link
Collaborator Author

Yes, it's very much just (1) consistency, and (2) avoiding unnecessary monad constraints before that proposal lands.

These are my thoughts as well when I added the HLint hints.

@wenkokke wenkokke force-pushed the recursion-ninja/hlint-no-return branch from 8be794c to 7fb5241 Compare May 26, 2025 13:56
Copy link
Collaborator

@wenkokke wenkokke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I wrote it. However, it took me upwards of 5 minutes to scroll past all the changes.

@wenkokke wenkokke dismissed dcoutts’s stale review May 26, 2025 14:02

This review is now stale.

@wenkokke wenkokke force-pushed the recursion-ninja/hlint-no-return branch from 7fb5241 to 126ea82 Compare May 26, 2025 14:03
@wenkokke wenkokke enabled auto-merge May 26, 2025 14:03
@wenkokke wenkokke added this pull request to the merge queue May 26, 2025
Merged via the queue into main with commit 9982e14 May 26, 2025
28 checks passed
@wenkokke wenkokke deleted the recursion-ninja/hlint-no-return branch May 26, 2025 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants