Skip to content

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
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
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