From eace8e32954eb6152e8df06f5a18905c235f0156 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 13 May 2015 16:23:35 -0700 Subject: [PATCH] Enable Travis for Clang, and enable -Werror for all Travis builds. Also added an extra Clang-only warning flag. --- .travis.yml | 5 +-- Makefile | 40 ++++++++++++++--------- tests/bindings/googlepb/test_vs_proto2.cc | 4 ++- travis.sh | 9 +++++ upb/bindings/googlepb/bridge.cc | 2 +- upb/bindings/lua/upb/table.c | 4 +-- upb/descriptor/reader.c | 2 +- upb/pb/compile_decoder_x64.c | 4 +-- 8 files changed, 46 insertions(+), 24 deletions(-) diff --git a/.travis.yml b/.travis.yml index b1a0cc765d..105bf1ca47 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,7 @@ language: cpp -compiler: gcc +compiler: + - gcc + - clang install: ./travis.sh install script: ./travis.sh script after_success: ./travis.sh after_success @@ -10,5 +12,4 @@ env: - UPB_TRAVIS_BUILD=withprotobuf - UPB_TRAVIS_BUILD=lua - UPB_TRAVIS_BUILD=coverage - - UPB_TRAVIS_BUILD=warnings - UPB_TRAVIS_BUILD=genfiles diff --git a/Makefile b/Makefile index 7d1ef11202..d3ccfc0d03 100644 --- a/Makefile +++ b/Makefile @@ -38,17 +38,18 @@ USER_CPPFLAGS= # Build with "make WITH_JIT=yes" (or anything besides "no") to enable the JIT. WITH_JIT=no -# Build with "make WITH_MAX_WARNINGS=yes" (or anything besides "no") to enable -# with strict warnings and treat warnings as errors. -WITH_MAX_WARNINGS=no +# Build with "make UPB_FAIL_WARNINGS=yes" (or anything besides "no") to turn +# warnings into errors. +UPB_FAIL_WARNINGS?=no # Basic compiler/flag setup. -CC=cc -CXX=c++ +CC?=cc +CXX?=c++ CFLAGS=-std=c99 CXXFLAGS=-Wno-unused-private-field INCLUDE=-I. -WARNFLAGS=-Wall -Wextra -Wno-sign-compare +WARNFLAGS=-Wall -Wextra -Wpointer-arith +WARNFLAGS_CXX=-Wall -Wextra -Wpointer-arith CPPFLAGS=$(INCLUDE) -DNDEBUG $(USER_CPPFLAGS) LUA=lua # 5.1 and 5.2 should both be supported @@ -57,8 +58,17 @@ ifneq ($(WITH_JIT), no) CPPFLAGS += -DUPB_USE_JIT_X64 endif -ifneq ($(WITH_MAX_WARNINGS), no) - WARNFLAGS=-Wall -Wextra -Wpointer-arith -Werror +ifeq ($(CC), clang) + WARNFLAGS += -Wconditional-uninitialized +endif + +ifeq ($(CXX), clang++) + WARNFLAGS_CXX += -Wconditional-uninitialized +endif + +ifneq ($(UPB_FAIL_WARNINGS), no) + WARNFLAGS += -Werror + WARNFLAGS_CXX += -Werror endif # Build with "make Q=" to see all commands that are being executed. @@ -215,15 +225,15 @@ obj/upb/%.o: upb/%.c | $$(@D)/. obj/upb/%.o: upb/%.cc | $$(@D)/. $(E) CXX $< - $(Q) $(CXX) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $< + $(Q) $(CXX) $(OPT) $(WARNFLAGS_CXX) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $< obj/upb/%.lo: upb/%.c | $$(@D)/. $(E) 'CC -fPIC' $< $(Q) $(CC) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CFLAGS) -c -o $@ $< -fPIC obj/upb/%.lo: upb/%.cc | $$(@D)/. - $(E) CXX $< - $(Q) $(CXX) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $< -fPIC + $(E) CXX -fPIC $< + $(Q) $(CXX) $(OPT) $(WARNFLAGS_CXX) $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $< -fPIC # Note: mkdir -p is technically susceptible to races when used with make -j. %/.: @@ -278,7 +288,7 @@ tests: $(TESTS) tests/testmain.o: tests/testmain.cc $(E) CXX $< - $(Q) $(CXX) $(OPT) $(WARNFLAGS) $(CXXFLAGS) $(CPPFLAGS) -c -o $@ $< + $(Q) $(CXX) $(OPT) $(WARNFLAGS_CXX) $(CXXFLAGS) $(CPPFLAGS) -c -o $@ $< $(C_TESTS): % : %.c tests/testmain.o $$(LIBS) $(E) CC $< @@ -286,7 +296,7 @@ $(C_TESTS): % : %.c tests/testmain.o $$(LIBS) $(CC_TESTS): % : %.cc tests/testmain.o $$(LIBS) $(E) CXX $< - $(Q) $(CXX) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CXXFLAGS) -Wno-deprecated -o $@ tests/testmain.o $< $(LIBS) + $(Q) $(CXX) $(OPT) $(WARNFLAGS_CXX) $(CPPFLAGS) $(CXXFLAGS) -Wno-deprecated -o $@ tests/testmain.o $< $(LIBS) # Several of these tests don't actually test these libs, but use them # incidentally to load a descriptor @@ -385,7 +395,7 @@ tests/bindings/googlepb/test_vs_proto2.googlemessage1: $(GOOGLEPB_TEST_DEPS) \ tests/google_message1.h \ tests/google_message2.h $(E) CXX $< '(benchmarks::SpeedMessage1)' - $(Q) $(CXX) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CXXFLAGS) -o $@ $< \ + $(Q) $(CXX) $(OPT) $(WARNFLAGS_CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $< \ -DMESSAGE_CIDENT="benchmarks::SpeedMessage1" \ -DMESSAGE_DATA_IDENT=message1_data \ tests/google_messages.pb.cc tests/testmain.o -lprotobuf -lpthread \ @@ -395,7 +405,7 @@ tests/bindings/googlepb/test_vs_proto2.googlemessage2: $(GOOGLEPB_TEST_DEPS) \ tests/google_message1.h \ tests/google_message2.h $(E) CXX $< '(benchmarks::SpeedMessage2)' - $(Q) $(CXX) $(OPT) $(WARNFLAGS) $(CPPFLAGS) $(CXXFLAGS) -o $@ $< \ + $(Q) $(CXX) $(OPT) $(WARNFLAGS_CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $< \ -DMESSAGE_CIDENT="benchmarks::SpeedMessage2" \ -DMESSAGE_DATA_IDENT=message2_data \ tests/google_messages.pb.cc tests/testmain.o -lprotobuf -lpthread \ diff --git a/tests/bindings/googlepb/test_vs_proto2.cc b/tests/bindings/googlepb/test_vs_proto2.cc index 8e68791685..06bea87d5e 100644 --- a/tests/bindings/googlepb/test_vs_proto2.cc +++ b/tests/bindings/googlepb/test_vs_proto2.cc @@ -47,7 +47,7 @@ void compare_metadata(const google::protobuf::Descriptor* d, d->FindFieldByNumber(upb_f->number()); ASSERT(upb_f); ASSERT(proto2_f); - ASSERT(upb_f->number() == proto2_f->number()); + ASSERT(upb_f->number() == (uint32_t)proto2_f->number()); ASSERT(std::string(upb_f->name()) == proto2_f->name()); ASSERT(upb_f->descriptor_type() == static_cast(proto2_f->type())); @@ -124,6 +124,8 @@ extern "C" { int run_tests(int argc, char *argv[]) { UPB_UNUSED(argc); UPB_UNUSED(argv); + UPB_UNUSED(message1_data); + UPB_UNUSED(message2_data); size_t len = sizeof(MESSAGE_DATA_IDENT); const char *str = (const char*)MESSAGE_DATA_IDENT; diff --git a/travis.sh b/travis.sh index 93a5935dc3..a09ed2eac7 100755 --- a/travis.sh +++ b/travis.sh @@ -94,6 +94,15 @@ if [ "$1" == "after_success" ] && [ "$UPB_TRAVIS_BUILD" != "coverage" ]; then exit fi +if [ "$CC" != "gcc" ] && [ "$UPB_TRAVIS_BUILD" == "coverage" ]; then + # coverage build only works for GCC. + exit +fi + set -e set -x + +# Make any compiler warning fail the build. +export UPB_FAIL_WARNINGS=true + eval ${UPB_TRAVIS_BUILD}_${1} diff --git a/upb/bindings/googlepb/bridge.cc b/upb/bindings/googlepb/bridge.cc index 2a2bbbfc4d..394f7b4c71 100644 --- a/upb/bindings/googlepb/bridge.cc +++ b/upb/bindings/googlepb/bridge.cc @@ -99,7 +99,7 @@ const MessageDef* DefBuilder::GetMaybeUnfrozenMessageDef( fields.push_back(d->field(i)); } - for (int i = 0; i < fields.size(); i++) { + for (size_t i = 0; i < fields.size(); i++) { const goog::FieldDescriptor* proto2_f = fields[i]; assert(proto2_f); md->AddField(NewFieldDef(proto2_f, m), &status); diff --git a/upb/bindings/lua/upb/table.c b/upb/bindings/lua/upb/table.c index 0907f581aa..e574ab0faf 100644 --- a/upb/bindings/lua/upb/table.c +++ b/upb/bindings/lua/upb/table.c @@ -88,7 +88,7 @@ static void lupbtable_pushtable(lua_State *L, const upb_table *t, bool inttab) { lupbtable_setnum(L, -1, "size_lg2", t->size_lg2); lua_newtable(L); - for (int i = 0; i < upb_table_size(t); i++) { + for (size_t i = 0; i < upb_table_size(t); i++) { lupbtable_pushent(L, &t->entries[i], inttab, t->ctype); lua_rawseti(L, -2, i + 1); } @@ -102,7 +102,7 @@ static void lupbtable_pushinttable(lua_State *L, const upb_inttable *t) { lupbtable_setnum(L, -1, "array_count", t->array_count); lua_newtable(L); - for (int i = 0; i < t->array_size; i++) { + for (size_t i = 0; i < t->array_size; i++) { lua_newtable(L); if (upb_arrhas(t->array[i])) { lupbtable_pushval(L, t->array[i], t->t.ctype); diff --git a/upb/descriptor/reader.c b/upb/descriptor/reader.c index 0b289c0092..29a21886c4 100644 --- a/upb/descriptor/reader.c +++ b/upb/descriptor/reader.c @@ -319,7 +319,7 @@ static bool parse_default(char *str, upb_fielddef *f) { break; } case UPB_TYPE_UINT32: { - long val = strtoul(str, &end, 0); + unsigned long val = strtoul(str, &end, 0); if (val > UINT32_MAX || errno == ERANGE || *end) success = false; else diff --git a/upb/pb/compile_decoder_x64.c b/upb/pb/compile_decoder_x64.c index a0cb3c925f..51b9b9e5c4 100644 --- a/upb/pb/compile_decoder_x64.c +++ b/upb/pb/compile_decoder_x64.c @@ -330,7 +330,7 @@ static void patchdispatch(jitcompiler *jc) { // Secondary slot. Since we have 64 bits for the value, we use an // absolute offset. int mcofs = machine_code_ofs2(jc, method, val); - newval = (uint64_t)(jc->group->jit_code + mcofs); + newval = (uint64_t)((char*)jc->group->jit_code + mcofs); } bool ok = upb_inttable_replace(dispatch, key, upb_value_uint64(newval)); UPB_ASSERT_VAR(ok, ok); @@ -339,7 +339,7 @@ static void patchdispatch(jitcompiler *jc) { // Update entry point for this method to point at mc base instead of bc // base. Set this only *after* we have patched the offsets // (machine_code_ofs2() uses this). - method->code_base.ptr = jc->group->jit_code + machine_code_ofs(jc, method); + method->code_base.ptr = (char*)jc->group->jit_code + machine_code_ofs(jc, method); upb_byteshandler *h = &method->input_handler_; upb_byteshandler_setstartstr(h, upb_pbdecoder_startjit, NULL);