Skip to content

Warn for pure unused functions #4479

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

Merged
merged 10 commits into from
Apr 24, 2025

Conversation

GearsDatapacks
Copy link
Member

Closes #4477

@GearsDatapacks GearsDatapacks marked this pull request as draft April 19, 2025 15:03
@GearsDatapacks GearsDatapacks force-pushed the warn-for-pure-unused-functions branch from ad5c87e to c11707d Compare April 19, 2025 15:24
@GearsDatapacks GearsDatapacks marked this pull request as ready for review April 19, 2025 15:36
@GearsDatapacks GearsDatapacks force-pushed the warn-for-pure-unused-functions branch from c11707d to 2317c2f Compare April 20, 2025 19:18
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Fantastic! Thank you. I've left some notes inline.

Purity hasn't been included in the package interface, which I think is a good idea for now.

It could be even more useful if the compiler knows that functions such as list.reverse and dict.get are pure. Do we want to have a hard-coded list of pure stdlib functions?

@GearsDatapacks
Copy link
Member Author

I think having a list of pure stdlib functions is good, especially for things like dict.insert which in a non-functional language would not return anything and could definitely be a source of confusion.

@GearsDatapacks GearsDatapacks marked this pull request as draft April 20, 2025 20:18
@GearsDatapacks
Copy link
Member Author

I've now added the is_pure_standard_library_function function which matches against all the current standard library functions except:

  • gleam/io.* - those are inherently impure
  • The deprecated gleam/dynamic modules - they will soon be removed and be redundant

@GearsDatapacks GearsDatapacks force-pushed the warn-for-pure-unused-functions branch 2 times, most recently from 00872d9 to c447e81 Compare April 22, 2025 17:36
@GearsDatapacks
Copy link
Member Author

Based on our discussion on Discord, I've reverted some of my changes to the purity tracking system to make it simple but effective enough for the current use case.
I've also added a comment to the Purity enum explaining the limitations of this system and how it can be improved in future.

@GearsDatapacks GearsDatapacks marked this pull request as ready for review April 22, 2025 17:37
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Fantastic! Really liking this. I've left a few notes inline about some comments, just to make sure we understand this in future as it's conceptually quite complex.

@GearsDatapacks GearsDatapacks force-pushed the warn-for-pure-unused-functions branch from 6a43be4 to fe80441 Compare April 23, 2025 14:36
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Really fantastic documentation there! I love it. Thank you!

@lpil lpil force-pushed the warn-for-pure-unused-functions branch from fe80441 to 445747a Compare April 24, 2025 19:35
@lpil lpil enabled auto-merge (rebase) April 24, 2025 19:37
@lpil lpil merged commit 4513aca into gleam-lang:main Apr 24, 2025
12 checks passed
@GearsDatapacks GearsDatapacks deleted the warn-for-pure-unused-functions branch April 24, 2025 19:50
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.

Add a warning for pure functions whose return values are not used
3 participants