New issue
Advanced search Search tips

Issue 855881 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

xcode clang diag'd -Wtautological-compare while chromium clang didn't

Project Member Reported by thakis@chromium.org, Jun 23 2018

Issue description

https://logs.chromium.org/v/?s=chromium%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8942978656263239232%2F%2B%2Fsteps%2Fcompile%2F0%2Fstdout

FAILED: obj/components/autofill/core/browser/browser/form_structure.o 
/b/s/w/ir/cache/goma/client/gomacc clang++ -MMD -MF obj/components/autofill/core/browser/browser/form_structure.o.d -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_XCODE_VERSION=0920 -DCR_CLANG_REVISION=\"335091-1\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DWEBP_EXTERN=extern -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_SUPPORT_GPU=0 -DI18N_ADDRESS_VALIDATION_DATA_URL=\"https://chromium-i18n.appspot.com/ssl-aggregate-address/\" -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -DI18N_PHONENUMBERS_USE_ICU_REGEXP=1 -DI18N_PHONENUMBERS_USE_ALTERNATE_FORMATS=1 -DI18N_PHONENUMBERS_NO_THREAD_SAFETY=1 -I../.. -Igen -I../../third_party/libwebp/src -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/protobuf/src -I../../third_party/boringssl/src/include -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/encode -I../../third_party/skia/include/gpu -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/libaddressinput/src/cpp/include -I../../third_party/ced/src -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/cacheinvalidation/overrides -I../../third_party/cacheinvalidation/src -I../../third_party/libphonenumber/dist/cpp/src -Igen/third_party/libphonenumber -I../../third_party/re2/src -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -fmerge-all-constants -no-canonical-prefixes -arch x86_64 -Wall -Werror -Wextra -Wimplicit-fallthrough -Wthread-safety -Wunguarded-availability -Wundeclared-selector -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-address-of-packed-member -O0 -fno-omit-frame-pointer -g1 -isysroot /b/s/w/ir/cache/xcode_ios_9c40b.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.2.sdk -stdlib=libc++ -mios-simulator-version-min=10.0 -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=c++14 -fno-exceptions -fno-rtti -fvisibility-inlines-hidden -c ../../components/autofill/core/browser/form_structure.cc -o obj/components/autofill/core/browser/browser/form_structure.o
../../components/autofill/core/browser/form_structure.cc:1320:60: error: comparison of unsigned expression >= 0 is always true [-Werror,-Wtautological-compare]
  for (auto current_index = field_index - 1; current_index >= 0;
                                             ~~~~~~~~~~~~~ ^  ~
1 error generated.


Not immediately clear why chromium clang didn't emit this; seems like a good warning here.
 

Comment 1 by thakis@chromium.org, Jun 27 2018

Regular clang finds this if we pass -Wtautological-unsigned-zero-compare

Comment 2 by thakis@chromium.org, Jun 27 2018

Owner: thakis@chromium.org
This is probably due to the trainwreck in r315614 / r321691 / r322901. I think before r315614, the compare-to-0 warnings were default-on, then they got moved to a new group, and then I didn't reenable them when I tried to revert that all.

Comment 3 by thakis@chromium.org, Jun 27 2018

r312750 made it so that tautological compares with 0 (signed) instead of just with 0u fired. That new mode then got moved behind its own flag in Wtautological-unsigned-zero-compare r312792 / Wtautological-unsigned-enum-zero-compare r313745.

r315614 then moved TautologicalUnsignedZeroCompare into TautologicalConstantCompare, r321691 then made at least one TautologicalUnsignedZeroCompare DefaultIgnore (not sure why), and then when I did r322901 I thought TautologicalUnsignedZeroCompare had always been DefaultIgnored.

Comment 4 by thakis@chromium.org, Jun 27 2018

Either the commit message of r312750 isn't correct, or xcode clang branched between r312750 and r312792, which seems a bit unlikely...

Comment 5 by thakis@chromium.org, Jun 27 2018

Bleh, this is also machine-dependent, due to android using unsigned chars by default.
Status: Assigned (was: Untriaged)
This bug has an owner, thus, it's been triaged. Changing status to "assigned".

Sign in to add a comment