From 51cf616dab63ba65c30cc58f0e5a61724aa4f731 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 13 May 2015 14:51:35 -0700 Subject: Changes to Lua module loading, and file generation. This change has several parts: 1. Resurrected tools/upbc. The code was all there but the build was broken for open-source. Now you can type "make tools/upbc" and it will build all necessary Lua modules and create a robust shell script for running upbc. 2. Changed Lua module loading to no longer rely on OS-level .so dependencies. The net effect of this is that you now only need to set LUA_PATH and LUA_CPATH; setting LD_LIBRARY_PATH or rpaths is no longer required. Downside: this drops compatibility with Lua 5.1, since it depends on a feature that only exists in Lua >=5.2 (and LuaJIT). 3. Since upbc works again, I fixed the re-generation of the descriptor files (descriptor.upb.h, descriptor.upb.c). "make genfiles" will re-generate these as well as the JIT code generator. 4. Added a Travis test target that ensures that the checked-in generated files are not out of date. I would do this for the Ragel generated file also, but we can't count on all versions of Ragel to necessarily generate identical output. 5. Changed Makefile to no longer automatically run Ragel to regenerate the JSON parser. This is unfortuante, because it's convenient when you're developing the JSON parser. However, "git clone" sometimes skews the timestamps a little bit so that "make" thinks it needs to regenerate these files for a fresh "git clone." This would normally be harmless, but if the user doesn't have Ragel installed, it makes the build fail completely. So now you have to explicitly regenerate the Ragel output. If you want to you can uncomment the auto-generation during development. --- tests/bindings/lua/test_upb.pb.lua | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tests/bindings') diff --git a/tests/bindings/lua/test_upb.pb.lua b/tests/bindings/lua/test_upb.pb.lua index 02b443a..7c1c0d0 100644 --- a/tests/bindings/lua/test_upb.pb.lua +++ b/tests/bindings/lua/test_upb.pb.lua @@ -1,6 +1,8 @@ -local upb = require "upb" +-- Require "pb" first to ensure that the transitive require of "upb" is +-- handled properly by the "pb" module. local pb = require "upb.pb" +local upb = require "upb" local lunit = require "lunit" if _VERSION >= 'Lua 5.2' then -- cgit v1.2.3 From eace8e32954eb6152e8df06f5a18905c235f0156 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 13 May 2015 16:23:35 -0700 Subject: 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(-) (limited to 'tests/bindings') diff --git a/.travis.yml b/.travis.yml index b1a0cc7..105bf1c 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 7d1ef11..d3ccfc0 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 8e68791..06bea87 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 93a5935..a09ed2e 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 2a2bbbf..394f7b4 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 0907f58..e574ab0 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 0b289c0..29a2188 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 a0cb3c9..51b9b9e 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); -- cgit v1.2.3