Skip to content

Fix some MSVC warnings #500

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kdchambers
Copy link

Minor changes to reduce the number of compile warnings emitted. I'm using VMA in a unity build with MSVC and fairly strict compile settings in Debug mode. I saw it mentioned before that fixing compile warnings isn't considered important (Understandably), so it's fine to close the PR if the changes are considered against the preferences of the project.

Overview:

  • Mark unused variables with (void)var;
  • In VmaVector<T, AllocatorT>::resize remove ternary branch as it should not be possible for newCapacity to be 0. Apart from removing an unnecessary branch and clearing up intent, it removes a warning about potentially passing NULL as input to memcpy, which is undefined behavior.
  • Hoist compile time known macros out of runtime expressions

Notes:

  • There were 1 or 2 places where marking a variable as unused wasn't perfectly clean, for example if it's just used in a macro such as VMA_LEAK_LOG_FORMAT which may not may not expand to an expression. I figured it was still better to include them if warnings about unused variables aren't considered useful anyways, at least it reduces build noise.
  • Probably intentional, but VmaVector<T, AllocatorT>::resize will allocate a new buffer and copy over the contents even if the new capacity is less than the current. I don't know if there's an easy way to just shrink the allocation in such a case to avoid the overhead of allocating a new buffer and doing a memcpy. I didn't look into it for this PR.

The branch to reallocate the buffer should never be taken with
newCapacity = 0. So assert that and remove branch that leads the
compiler to believe that the following memcpy could be passed
a NULL as input
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.

1 participant