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

Enable and Resolve [-Wzero-as-null-pointer-constant] #11709

Closed

Conversation

Minty-Meeo
Copy link
Contributor

This completes a small TODO in our CMakeLists. This PR is dependent on #11706, and is based on #11687 which will surely get reviewed any minute now. I have made an attempt to upstream the changes to cpp-optparse and PicoJSON, but haven't gotten a response yet. Regardless, the changes are incredibly minor if they must be maintained downstream.

@Minty-Meeo Minty-Meeo force-pushed the Wzero-as-null-pointer-constant branch from 1702935 to ee681ae Compare March 30, 2023 21:11
@Minty-Meeo
Copy link
Contributor Author

I still have to fix imgui/implot. It will be more difficult.

@iwubcode
Copy link
Contributor

I think it'd be more appropriate to ignore warnings in externals... (include them as system dirs). Kinda surprised we aren't doing that.

@Minty-Meeo
Copy link
Contributor Author

I hadn't considered changing the includes to be system directories. That sure beats putting a diagnostic ignored pragma guard around the includes all the time.

@Rumi-Larry
Copy link

I recommend dropping the SoundTouch commit from this PR. If these fixes are required to update Sound Touch, then the PR that updates SoundTouch should be based on this one, not backwards.

@Minty-Meeo
Copy link
Contributor Author

On the contrary. The Sound Touch PR fixes zero-as-a-null-pointer-constant warnings. It does not depend on this PR at all.

@Minty-Meeo
Copy link
Contributor Author

Minty-Meeo commented Apr 9, 2023

I am unfamiliar with the MSVC build system. How might I add this warning for MSVC?
https://learn.microsoft.com/en-us/cpp/code-quality/c26477
Preferably in a way that doesn't trigger a warning-turned-error, since there are some places that NULL is leaked into macros we use.
Edit: I now know this is a code analyzer feature, not a warning.

@Minty-Meeo Minty-Meeo force-pushed the Wzero-as-null-pointer-constant branch 2 times, most recently from 9760a96 to da13586 Compare April 14, 2023 06:48
@Minty-Meeo Minty-Meeo force-pushed the Wzero-as-null-pointer-constant branch from da13586 to 5829a44 Compare April 15, 2023 05:30
@Minty-Meeo
Copy link
Contributor Author

I have shuffled this PR around a bit. While the goal is still enabling -Wzero-as-null-pointer-constant, the steps taken are a bit different. The changes to Dolphin itself are now over on PR #11760, which this PR is obviously now dependent on. Given the inactivity of the Implot project, as well as the potential for future Externals triggering warnings, this PR is also now dependent on #11716 to solve warnings leaking from External headers.

@JosJuice JosJuice closed this Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants