New issue
Advanced search Search tips

Issue 747643 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS , Mac
Pri: 3
Type: Bug



Sign in to add a comment

third_party/google_toolbox_for_mac/src/iPhone doesn't build with new clang

Project Member Reported by thakis@chromium.org, Jul 22 2017

Issue description

I'm doing a clang roll (https://chromium-review.googlesource.com/c/582074/). It breaks the iOS build like so: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.mac%2Fios-simulator%2F264415%2F%2B%2Frecipes%2Fsteps%2Fcompile__with_patch_%2F0%2Fstdout

FAILED: ios_clang_x86/obj/third_party/google_toolbox_for_mac/google_toolbox_for_mac/GTMUIImage+Resize.o 
export DEVELOPER_DIR=/b/build/slave/ios-simulator/build/src/build/ios_files/Xcode.app;  /b/build/slave/cache/goma_client/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang -MMD -MF ios_clang_x86/obj/third_party/google_toolbox_for_mac/google_toolbox_for_mac/GTMUIImage+Resize.o.d -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DDISABLE_NACL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_CLANG_REVISION=\"308728-1\" -DCR_XCODE_VERSION=0821 -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -I../.. -Iios_clang_x86/gen -I../../third_party/google_toolbox_for_mac -I../../third_party/google_toolbox_for_mac/src -I../../third_party/google_toolbox_for_mac/src/AppKit -I../../third_party/google_toolbox_for_mac/src/DebugUtils -I../../third_party/google_toolbox_for_mac/src/Foundation -Wno-deprecated-declarations -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -arch i386 -O0 -fno-omit-frame-pointer -gdwarf-2 -isysroot /b/build/slave/ios-simulator/build/src/build/ios_files/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator10.2.sdk -stdlib=libc++ -mios-simulator-version-min=9.0 -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-auto-raw-pointer -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Werror -Wall -Wno-unused-variable -Wundeclared-selector -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -std=c11 -std=c99 -c ../../third_party/google_toolbox_for_mac/src/iPhone/GTMUIImage+Resize.m -o ios_clang_x86/obj/third_party/google_toolbox_for_mac/google_toolbox_for_mac/GTMUIImage+Resize.o
../../third_party/google_toolbox_for_mac/src/iPhone/GTMUIImage+Resize.m:155:56: error: enum values with underlying type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead [-Werror,-Wformat]
      _GTMDevAssert(false, @"Invalid orientation %zd", orientation);
                                                 ~~~   ^~~~~~~~~~~
                                                       (long)
../../third_party/google_toolbox_for_mac/src/GTMDefines.h:139:37: note: expanded from macro '_GTMDevAssert'
                        description:__VA_ARGS__];                             \
                                    ^~~~~~~~~~~
1 error generated.


This should be fixed in GTM. gtm OWNERS all look more like mac than ios folks though.
 

Comment 1 by mark@chromium.org, Jul 22 2017

Is there a way to do a -k0 build through the tryserver? The code is DEPSed in, so if there are more of these to fix, I’d rather get them all taken care of upstream in one fell swoop.

Comment 2 by thakis@chromium.org, Jul 22 2017

I did a build with the warning just suppressed in t_p/gtm, and that was green: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.mac%2Fios-simulator%2F264445%2F%2B%2Frecipes%2Fsteps%2Fcompile__with_patch_%2F0%2Fstdout

(patch here: https://chromium-review.googlesource.com/c/582074/10)

It's possible but unlikely that another of the ios-only files in GTM trigger this new warning too; I didn't check.

Comment 3 by mark@chromium.org, Jul 24 2017

Owner: mark@chromium.org
Status: Started (was: Untriaged)
https://github.com/google/google-toolbox-for-mac/pull/147
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1c688af81fcfa4c072dcca79e698641839e8a42f

commit 1c688af81fcfa4c072dcca79e698641839e8a42f
Author: Nico Weber <thakis@chromium.org>
Date: Mon Jul 24 11:25:28 2017

Roll clang 307486:308728.

Ran `tools/clang/scripts/upload_revision.py 308728`.

This changes the clang version from 5.0.0 to 6.0.0, so simplify
some things that dealt with both numbers.

This changes clang to no longer look for libc++ headers in the
Fuchsia SDK.  To keep things building, use "our" libc++, like
on linux (use_custom_libcxx).  This in turn means we pass
-nodefaultlibs to the linker, which sadly also disables the
automatic linking of clang_rt.builtins, so do that manually now.
(In exchange, we no longer need to pass in -resource-dir.)

This makes -Wformat fire in Google Toolbox for Mac in iOS builds,
so disable -Wformat in that config for now.

Bug:  746303 , 746505 , 747638 , 747643 , 724204 

Change-Id: I6196a2a173a1b4871f22d0ce92436d0197fd7845
Reviewed-on: https://chromium-review.googlesource.com/582074
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488946}
[modify] https://crrev.com/1c688af81fcfa4c072dcca79e698641839e8a42f/build/config/c++/BUILD.gn
[modify] https://crrev.com/1c688af81fcfa4c072dcca79e698641839e8a42f/build/config/c++/c++.gni
[modify] https://crrev.com/1c688af81fcfa4c072dcca79e698641839e8a42f/build/config/fuchsia/BUILD.gn
[modify] https://crrev.com/1c688af81fcfa4c072dcca79e698641839e8a42f/build/toolchain/toolchain.gni
[modify] https://crrev.com/1c688af81fcfa4c072dcca79e698641839e8a42f/third_party/google_toolbox_for_mac/BUILD.gn
[modify] https://crrev.com/1c688af81fcfa4c072dcca79e698641839e8a42f/tools/clang/scripts/update.py

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ed1431620c445d5e6c3551d34b8d282b360966f

commit 9ed1431620c445d5e6c3551d34b8d282b360966f
Author: Mark Mentovai <mark@chromium.org>
Date: Mon Jul 24 19:04:47 2017

Update GTM to 3c3111d3aefe and re-enable -Wformat

Since clang r308067, -Wformat is able to check that an argument's type
is appropriate for the %zd format specifier. Some uses in GTM were not,
and needed to be adjusted. -Wformat was disabled in GTM in 1c688af81fcf
and can now be re-enabled.

3c3111d3aefe Cast NSInteger to long for formatting as %ld

Bug:  747643 
Change-Id: I873e9771a83f95d32e186ef3779966d6ebfe2718
Reviewed-on: https://chromium-review.googlesource.com/583407
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489040}
[modify] https://crrev.com/9ed1431620c445d5e6c3551d34b8d282b360966f/DEPS
[modify] https://crrev.com/9ed1431620c445d5e6c3551d34b8d282b360966f/third_party/google_toolbox_for_mac/BUILD.gn

Comment 7 by mark@chromium.org, Jul 24 2017

Status: Fixed (was: Started)

Sign in to add a comment