Enable Travis for Clang, and enable -Werror for all Travis builds.

Also added an extra Clang-only warning flag.

.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

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 \

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<upb::FieldDef::DescriptorType>(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;
 

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}

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);

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);

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

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);