diff --git a/compiler/cpp/src/generate/t_cpp_generator.cc b/compiler/cpp/src/generate/t_cpp_generator.cc index 298096d3265..2bda4d0dc8b 100755 --- a/compiler/cpp/src/generate/t_cpp_generator.cc +++ b/compiler/cpp/src/generate/t_cpp_generator.cc @@ -42,6 +42,20 @@ using std::vector; static const string endl = "\n"; // avoid ostream << std::endl flushes +namespace { + + +enum TerseWrites { TW_DISABLED = 0, TW_SAFE = 1, TW_ALL = 2 }; + +template +TerseWrites parseTerseWrites(const Options& options) { + auto iter = options.find("terse_writes"); + return (iter == options.end()) ? TW_DISABLED : + (iter->second == "safe") ? TW_SAFE : TW_ALL; +} + +} // namespace + /** * C++ code generator. This is legitimacy incarnate. * @@ -81,6 +95,8 @@ class t_cpp_generator : public t_oop_generator { gen_templates_only_ = (iter != parsed_options.end() && iter->second == "only"); + terse_writes_ = parseTerseWrites(parsed_options); + out_dir_base_ = "gen-cpp"; } @@ -217,6 +233,8 @@ class t_cpp_generator : public t_oop_generator { std::string argument_list(t_struct* tstruct, bool name_params=true, bool start_comma=false); std::string type_to_enum(t_type* ttype); std::string local_reflection_name(const char*, t_type* ttype, bool external=false); + bool try_terse_write_predicate(ofstream& out, t_field* t, bool pointers, TerseWrites terse_writes, + string& predicate); void generate_enum_constant_list(std::ofstream& f, const vector& constants, @@ -291,6 +309,15 @@ class t_cpp_generator : public t_oop_generator { */ bool gen_no_default_operators_; + /* + * Should write function avoid emitting values that are unchanged + * from default, if not explicitly optional/required? + * Caveats, when whitelisted: + * - don't change default values when turned on. + * - read() should be done into fresh or __clear()ed objects. + */ + TerseWrites terse_writes_; + /** * Strings for namespace, computed once up front then used directly */ @@ -1379,6 +1406,70 @@ void t_cpp_generator::generate_struct_reader(ofstream& out, "}" << endl << endl; } +/** + * Generates a terse write predicate - checks if the value + * has changed from its initial value. + */ +bool t_cpp_generator::try_terse_write_predicate( + ofstream& out, t_field* tfield, bool pointers, TerseWrites terse_writes, + string& predicate) { + if (terse_writes == TW_DISABLED) { + return false; + } + + // Only do terse writes for fields where required/optional isn't specified. + if (tfield->get_req() == t_field::T_REQUIRED || + tfield->get_req() == t_field::T_OPTIONAL) { + return false; + } + t_type* type = get_true_type(tfield->get_type()); + t_const_value* tval = tfield->get_value(); + + // Terse write is unsafe to use without explicitly setting default value, + // as in PHP / Python that would change result of deserialization (comparing + // with the case when terse_writes is not used): field set in C++ to default + // value would be deserialized as null / None. + if (terse_writes == TW_SAFE && tval == nullptr) { + return false; + } + + if (type->is_struct() || type->is_xception() || + // no support for void. + (type->is_base_type() && ((t_base_type*)type)->is_void()) || + // only support string, if default empty. + (type->is_base_type() && ((t_base_type*)type)->is_string() && + tval != nullptr && !tval->get_string().empty()) || + // only support container, if default empty. + (type->is_container() && tval != nullptr && + ((tval->get_type() == t_const_value::CV_LIST && + !tval->get_list().empty()) || + (tval->get_type() == t_const_value::CV_MAP && + !tval->get_map().empty()))) + ) { + return false; + } + + // Containers -> "if (!x.empty())" + if (type->is_container() || + (type->is_base_type() && ((t_base_type*)type)->is_string())) { + predicate = "!this->" + tfield->get_name() + + (pointers ? "->empty()" : ".empty()"); + return true; + } + // ints, enum -> "if (x != default value) + if (type->is_base_type() || type->is_enum()) { + std::string const_value = "0"; + if (tval != NULL) { + const_value = render_const_value(out, "", type, tval); + } + predicate = (pointers ? "*(this->" : "this->") + + tfield->get_name() + (pointers ? ") != " : " != ") + + const_value; + return true; + } + return false; +} + /** * Generates the write function. * @@ -1391,6 +1482,9 @@ void t_cpp_generator::generate_struct_writer(ofstream& out, string name = tstruct->get_name(); const vector& fields = tstruct->get_sorted_members(); vector::const_iterator f_iter; + string predicate; + const TerseWrites terse_writes = + std::max(terse_writes_, parseTerseWrites(tstruct->annotations_)); if (gen_templates_) { out << @@ -1416,6 +1510,11 @@ void t_cpp_generator::generate_struct_writer(ofstream& out, if (check_if_set) { out << endl << indent() << "if (this->__isset." << (*f_iter)->get_name() << ") {" << endl; indent_up(); + } else if (try_terse_write_predicate(out, *f_iter, pointers, terse_writes, + predicate)) { + indent(out) << "if (" << predicate << ") {" << endl; + indent_up(); + check_if_set = true; } else { out << endl; } @@ -4682,5 +4781,6 @@ THRIFT_REGISTER_GENERATOR(cpp, "C++", " pure_enums: Generate pure enums instead of wrapper classes.\n" " dense: Generate type specifications for the dense protocol.\n" " include_prefix: Use full include paths in generated files.\n" +" terse_writes: Suppress writes for fields holding default values.\n" ) diff --git a/lib/cpp/test/Makefile.am b/lib/cpp/test/Makefile.am index d04cfb501c1..af211a0f024 100755 --- a/lib/cpp/test/Makefile.am +++ b/lib/cpp/test/Makefile.am @@ -55,6 +55,7 @@ check_PROGRAMS = \ DebugProtoTest \ JSONProtoTest \ OptionalRequiredTest \ + TerseWritesTest \ SpecializationTest \ AllProtocolsTest \ TransportTest \ @@ -167,6 +168,14 @@ OptionalRequiredTest_SOURCES = \ OptionalRequiredTest_LDADD = libtestgencpp.la +# +# TerseWritesTest +# +TerseWritesTest_SOURCES = \ + TerseWritesTest.cpp + +TerseWritesTest_LDADD = libtestgencpp.la + # # SpecializationTest # @@ -213,7 +222,7 @@ gen-cpp/Service.cpp gen-cpp/StressTest_types.cpp: $(top_srcdir)/test/StressTest. $(THRIFT) --gen cpp:dense $< gen-cpp/SecondService.cpp gen-cpp/ThriftTest_constants.cpp gen-cpp/ThriftTest.cpp gen-cpp/ThriftTest_types.cpp gen-cpp/ThriftTest_types.h: $(top_srcdir)/test/ThriftTest.thrift - $(THRIFT) --gen cpp:dense $< + $(THRIFT) --gen cpp:dense,terse_writes $< gen-cpp/ChildService.cpp: processor/proc.thrift $(THRIFT) --gen cpp:templates,cob_style $< diff --git a/lib/cpp/test/TerseWritesTest.cpp b/lib/cpp/test/TerseWritesTest.cpp new file mode 100644 index 00000000000..5c9a9b1f6eb --- /dev/null +++ b/lib/cpp/test/TerseWritesTest.cpp @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * Contains some contributions under the Thrift Software License. + * Please see doc/old-thrift-license.txt in the Thrift distribution for + * details. + */ + +#include "gen-cpp/ThriftTest_types.h" + +#include +#include +#include "gen-cpp/ThriftTest_types.h" + +using apache::thrift::transport::TMemoryBuffer; +using apache::thrift::protocol::TBinaryProtocol; +using boost::shared_ptr; +using namespace thrift::test; + +int main() { + shared_ptr buf(new TMemoryBuffer()); + shared_ptr prot(new TBinaryProtocol(buf)); + + BoolTest btest; + btest.__isset.b = false; + btest.__isset.s = false; + + BoolTest btestread; + + btest.write(prot.get()); + btestread.read(prot.get()); + assert(btest.__isset.b == false); + assert(btest.__isset.s == false); + assert(btest.s == "true"); + + // Try without optional keyword + Bonk bonk; + bonk.type = 1; + Bonk bonkread; + bonk.write(prot.get()); + bonkread.read(prot.get()); + assert(bonkread.__isset.message == false); + assert(bonkread.__isset.type == true); + assert(bonkread.__isset.type == 1); + + return 0; +}