Skip to content

fix warnings showing up with -Wall#2692

Merged
andresy merged 1 commit intomainfrom
wall-warnings
Oct 24, 2025
Merged

fix warnings showing up with -Wall#2692
andresy merged 1 commit intomainfrom
wall-warnings

Conversation

@andresy
Copy link
Copy Markdown
Member

@andresy andresy commented Oct 21, 2025

Proposed changes

Fix all warnings occurring when compiling with -Wall

Checklist

Put an x in the boxes that apply.

  • [x ] I have read the CONTRIBUTING document
  • [ x] I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • [ x] I have added tests that prove my fix is effective or that my feature works
  • [ x] I have updated the necessary documentation (if needed)

@andresy andresy requested review from angeloskath and awni and removed request for awni October 21, 2025 22:29
@awni
Copy link
Copy Markdown
Member

awni commented Oct 22, 2025

This is awesome, thanks! What do you think about adding -Wall as a build option by default so we stay warning free (that way it will be checked in circle).

@andresy
Copy link
Copy Markdown
Member Author

andresy commented Oct 23, 2025

I think with clang it would make sense.

With gcc one get a lot more warnings -- (possibly what you would get with -Wall -Wextra with clang), so I think we would not want to have this option enabled right now for gcc.

These warnings you get with gcc include type mismatch warnings (say a int for loop, with a size_t limit). I did not fix these yet, I was actually not sure we want to fix them

Copy link
Copy Markdown
Member

@awni awni left a comment

Choose a reason for hiding this comment

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

LGTM

@andresy andresy merged commit 8f8af61 into main Oct 24, 2025
6 checks passed
@andresy andresy deleted the wall-warnings branch October 24, 2025 18:43
faisalmemon pushed a commit to faisalmemon/mlx that referenced this pull request Oct 30, 2025
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.

2 participants