Skip to content

Conversation

joprice
Copy link
Contributor

@joprice joprice commented Jan 24, 2021

This applies the suggested fix that shows up when ASEnableVerboseLogging is enabled:

external/Texture/Source/ASNetworkImageNode.mm:891:144: error: enum values with underlying type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead [-Werror,-Wformat]
            as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ url: %@ cacheType: %@", self, [imageContainer asdk_image], URL, cacheType);```

@@ -888,7 +888,7 @@ - (void)_lazilyLoadImageIfNecessary
if (delegateDidLoadImageFromCache) {
[delegate imageNodeDidLoadImageFromCache:self];
}
as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ url: %@ cacheType: %@", self, [imageContainer asdk_image], URL, cacheType);
as_log_verbose(ASImageLoadingLog(), "Decached image for %@ img: %@ url: %@ cacheType: %ld", self, [imageContainer asdk_image], URL, (long) cacheType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sizeof(NSInteger) > sizeof(long)

Another way to do this is to keep the formatter the same but box the scalar ie @(cacheType).

@joprice
Copy link
Contributor Author

joprice commented Jan 31, 2021

I'm trying to verify this change by using build.sh, but I have a newer sdk. When I provide TEXTURE_BUILD_PLATFORM and TEXTURE_BUILD_SDK with my current versions, I get errors related to missing extern modifiers, etc. It seems that -Werror, and I'm not sure if that's intentional and the sdk version caused this, or if these warnings are expected and I'm supposed to ignore them somehow.

Up until now, I've been testing this by building it via bazel using PodToBuild, but I'd like to be able to test it directly and run the tests (or perhaps bazel support could be added directly?).

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.

2 participants