Skip to content
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

Reduce compiler warnings (-Wall -Wpedantic) #880

Open
wants to merge 3 commits into
base: dev2
Choose a base branch
from

Conversation

gruenich
Copy link
Contributor

No description provided.

@DrTimothyAldenDavis
Copy link
Owner

Looks good. I haven't had time to try these out but will do so when my schedule allows.

My organization requires a minor revision to my Contributor Agreement, for all new contributions. Can you sign the new one I just posted? It's here in the dev branch:

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/dev/Contributor_License/SuiteSparse%20Individual%20Contributor%20License%20Agreement%20(20241011).pdf

@DrTimothyAldenDavis
Copy link
Owner

Also, I need to revise the PR to go to the dev2 branch, not dev. The two branches are in sync so that shouldn't be a problem.

@DrTimothyAldenDavis DrTimothyAldenDavis changed the base branch from dev to dev2 November 4, 2024 20:49
Extra semicolons are forbidden outside a function.
Found by GCC 15 (Wpedantic)
@gruenich gruenich force-pushed the feature/reduce-compiler-warnings branch from 99b1cda to 2f0d463 Compare November 9, 2024 08:57
@gruenich
Copy link
Contributor Author

I think this is ready to be merged.

@DrTimothyAldenDavis
Copy link
Owner

I haven't had a chance to get to it yet. Each of these changes looks fine but I need to evaluate each one carefully. Sometimes they point to a deeper issue in the code, which I have to think about.

I'm in the process of doing the same for GraphBLAS. I just encountered a bug where I failed to return a value from a non-void function. It worked, surprisingly, with -g but failed with -O3. Took a while to track down. So I've turned on -Wreturn-type as an error, so the compiler will refuse to compile any code that does that.

I may need to do things like that in the rest of SuiteSparse, and some of these might be just those cases.

Unused variables are different. Sometimes I want to keep them, for code readability, as a comment. In other words, it might be a line of code I want to see but don't actually need to compile. But in that case I should comment them out, not delete them.

@DrTimothyAldenDavis
Copy link
Owner

And sometimes unused variables are actually used if debugging is enabled with either #undef NDEBUG or my own debug structure. I have to double check to make sure which ones are used for debugging. In that case, the variables need to be placed inside and #ifdef so they are available when debugging is enabled.

So I need to check each one.

This is a useful process but it takes some time to evaluate each of these cases.

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.

3 participants