More JUCE suggestions #61
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Cheers, this looks sensible to me, will merge it.. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Well just as I got those other fixes through, more builds were added to our CI 😆
For the pragma ignore, I'm struggling to see how to avoid an ignore of some description. The ignore in this PR is required because of the use of
__trywhen building with clang on windows (I think with pedantic warnings enabled, I would need to check exactly what triggered it). If we don't use__trythen MSVC will complain, suggesting to use_mallocainstead, but that will allocate on the heap for anything over 1024 bytes by default. We could instead ignore the MSVC warning and drop the try/except?The other issue is fairly simple
_M_X64is defined for arm64ec builds (but not arm64). In theory using_umul128should work but because there is no equivalent intrinsic for arm64 a warning is triggeredC4163: 'UnsignedMultiply128': not available as an intrinsic function._M_ARM64ECis only defined for am64ec builds so we can easily detect the edge case.