Repository: thrift Updated Branches: refs/heads/master 36628a28e -> b4c190b6e
THRIFT-4060 add better support in the cpp generator for custom ostream operators on structures Client: C++ This closes #1172 Project: http://git-wip-us.apache.org/repos/asf/thrift/repo Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/b4c190b6 Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/b4c190b6 Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/b4c190b6 Branch: refs/heads/master Commit: b4c190b6ea960c20a420089b1431042e435c73e9 Parents: 36628a2 Author: James E. King, III <jk...@apache.org> Authored: Mon Feb 13 16:39:59 2017 -0500 Committer: James E. King, III <jk...@apache.org> Committed: Mon Feb 13 16:39:59 2017 -0500 ---------------------------------------------------------------------- .gitignore | 1 + .../cpp/src/thrift/generate/t_cpp_generator.cc | 61 ++++++++++++++---- lib/cpp/test/AnnotationTest.cpp | 68 ++++++++++++++++++++ lib/cpp/test/CMakeLists.txt | 19 +++++- lib/cpp/test/Makefile.am | 20 +++++- test/AnnotationTest.thrift | 9 +++ 6 files changed, 163 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/thrift/blob/b4c190b6/.gitignore ---------------------------------------------------------------------- diff --git a/.gitignore b/.gitignore index 288f1b2..88bb9c6 100644 --- a/.gitignore +++ b/.gitignore @@ -102,6 +102,7 @@ project.lock.json /lib/cpp/src/thrift/stamp-h2 /lib/cpp/test/Benchmark /lib/cpp/test/AllProtocolsTest +/lib/cpp/test/AnnotationTest /lib/cpp/test/DebugProtoTest /lib/cpp/test/DenseProtoTest /lib/cpp/test/EnumTest http://git-wip-us.apache.org/repos/asf/thrift/blob/b4c190b6/compiler/cpp/src/thrift/generate/t_cpp_generator.cc ---------------------------------------------------------------------- diff --git a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc index cbe8da2..1c82e09 100644 --- a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc @@ -64,6 +64,8 @@ public: gen_templates_ = false; gen_templates_only_ = false; gen_moveable_ = false; + gen_no_ostream_operators_ = false; + for( iter = parsed_options.begin(); iter != parsed_options.end(); ++iter) { if( iter->first.compare("pure_enums") == 0) { gen_pure_enums_ = true; @@ -80,6 +82,8 @@ public: gen_templates_only_ = (iter->second == "only"); } else if( iter->first.compare("moveable_types") == 0) { gen_moveable_ = true; + } else if ( iter->first.compare("no_ostream_operators") == 0) { + gen_no_ostream_operators_ = true; } else { throw "unknown option cpp:" + iter->first; } @@ -260,6 +264,22 @@ public: void set_use_include_prefix(bool use_include_prefix) { use_include_prefix_ = use_include_prefix; } + /** + * The compiler option "no_thrift_ostream_impl" can be used to prevent + * the compiler from emitting implementations for operator <<. In this + * case the consuming application must provide any needed to build. + * + * To disable this on a per structure bases, one can alternatively set + * the annotation "cpp.customostream" to prevent thrift from emitting an + * operator << (std::ostream&). + * + * See AnnotationTest for validation of this annotation feature. + */ + bool has_custom_ostream(t_type* ttype) const { + return (gen_no_ostream_operators_) || + (ttype->annotations_.find("cpp.customostream") != ttype->annotations_.end()); + } + private: /** * Returns the include prefix to use for a file generated by program, or the @@ -289,6 +309,11 @@ private: bool gen_moveable_; /** + * True if we should generate ostream definitions + */ + bool gen_no_ostream_operators_; + + /** * True iff we should use a path prefix in our #include statements for other * thrift-generated header files. */ @@ -751,7 +776,11 @@ void t_cpp_generator::generate_cpp_struct(t_struct* tstruct, bool is_exception) if (gen_moveable_) { generate_move_assignment_operator(f_types_impl_, tstruct); } - generate_struct_print_method(f_types_impl_, tstruct); + + if (!has_custom_ostream(tstruct)) { + generate_struct_print_method(f_types_impl_, tstruct); + } + if (is_exception) { generate_exception_what_method(f_types_impl_, tstruct); } @@ -1094,7 +1123,7 @@ void t_cpp_generator::generate_struct_declaration(ofstream& out, } out << endl; - if (is_user_struct) { + if (is_user_struct && !has_custom_ostream(tstruct)) { out << indent() << "virtual "; generate_struct_print_method_decl(out, NULL); out << ";" << endl; @@ -1467,14 +1496,22 @@ void t_cpp_generator::generate_struct_swap(ofstream& out, t_struct* tstruct) { } void t_cpp_generator::generate_struct_ostream_operator(std::ofstream& out, t_struct* tstruct) { - out << "inline std::ostream& operator<<(std::ostream& out, const " - << tstruct->get_name() - << "& obj)" << endl; - scope_up(out); - out << indent() << "obj.printTo(out);" << endl - << indent() << "return out;" << endl; - scope_down(out); - out << endl; + if (has_custom_ostream(tstruct)) { + // the consuming implementation will define this behavior + out << "std::ostream& operator<<(std::ostream& out, const " + << tstruct->get_name() + << "& obj);" << endl; + } else { + // thrift defines this behavior + out << "inline std::ostream& operator<<(std::ostream& out, const " + << tstruct->get_name() + << "& obj)" << endl; + scope_up(out); + out << indent() << "obj.printTo(out);" << endl + << indent() << "return out;" << endl; + scope_down(out); + out << endl; + } } void t_cpp_generator::generate_struct_print_method_decl(std::ofstream& out, t_struct* tstruct) { @@ -4371,4 +4408,6 @@ THRIFT_REGISTER_GENERATOR( " templates: Generate templatized reader/writer methods.\n" " pure_enums: Generate pure enums instead of wrapper classes.\n" " include_prefix: Use full include paths in generated files.\n" - " moveable_types: Generate move constructors and assignment operators.\n") + " moveable_types: Generate move constructors and assignment operators.\n" + " no_ostream_operators:\n" + " Omit generation of ostream definitions.\n"); http://git-wip-us.apache.org/repos/asf/thrift/blob/b4c190b6/lib/cpp/test/AnnotationTest.cpp ---------------------------------------------------------------------- diff --git a/lib/cpp/test/AnnotationTest.cpp b/lib/cpp/test/AnnotationTest.cpp new file mode 100644 index 0000000..2e18840 --- /dev/null +++ b/lib/cpp/test/AnnotationTest.cpp @@ -0,0 +1,68 @@ +/* + * 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. + */ +#define BOOST_TEST_MODULE AnnotationTest +#include <boost/test/unit_test.hpp> +#include "gen-cpp/AnnotationTest_types.h" +#include <ostream> +#include <sstream> + +// Normally thrift generates ostream operators, however +// with the annotation "cpp.customostream" one can tell the +// compiler they are going to provide their own, and not +// emit operator << or printTo(). + +std::ostream& operator<<(std::ostream& os, const ostr_custom& osc) +{ + os << "{ bar = " << osc.bar << "; }"; + return os; +} + +BOOST_AUTO_TEST_SUITE(BOOST_TEST_MODULE) + +BOOST_AUTO_TEST_CASE(test_cpp_compiler_generated_ostream_operator) +{ + ostr_default def; + def.__set_bar(10); + + std::stringstream ssd; + ssd << def; + BOOST_CHECK_EQUAL(ssd.str(), "ostr_default(bar=10)"); +} + +BOOST_AUTO_TEST_CASE(test_cpp_customostream_uses_consuming_application_definition) +{ + ostr_custom cus; + cus.__set_bar(10); + + std::stringstream csd; + csd << cus; + BOOST_CHECK_EQUAL(csd.str(), "{ bar = 10; }"); +} + +/** + * Disabled; see THRIFT-1567 - not sure what it is supposed to do +BOOST_AUTO_TEST_CASE(test_cpp_type) { + // Check that the "cpp.type" annotation changes "struct foo" to "DenseFoo" + // This won't compile if the annotation is mishandled + DenseFoo foo; + foo.__set_bar(5); +} + */ + +BOOST_AUTO_TEST_SUITE_END() http://git-wip-us.apache.org/repos/asf/thrift/blob/b4c190b6/lib/cpp/test/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/lib/cpp/test/CMakeLists.txt b/lib/cpp/test/CMakeLists.txt index 74b0a09..b7a7798 100644 --- a/lib/cpp/test/CMakeLists.txt +++ b/lib/cpp/test/CMakeLists.txt @@ -30,6 +30,8 @@ include_directories("${CMAKE_CURRENT_BINARY_DIR}") # Create the thrift C++ test library set(testgencpp_SOURCES + gen-cpp/AnnotationTest_types.cpp + gen-cpp/AnnotationTest_types.h gen-cpp/DebugProtoTest_types.cpp gen-cpp/DebugProtoTest_types.h gen-cpp/EnumTest_types.cpp @@ -146,6 +148,13 @@ LINK_AGAINST_THRIFT_LIBRARY(ZlibTest thriftz) add_test(NAME ZlibTest COMMAND ZlibTest) endif(WITH_ZLIB) +add_executable(AnnotationTest AnnotationTest.cpp) +target_link_libraries(AnnotationTest + testgencpp + ${Boost_LIBRARIES} +) +LINK_AGAINST_THRIFT_LIBRARY(AnnotationTest thrift) +add_test(NAME AnnotationTest COMMAND AnnotationTest) add_executable(EnumTest EnumTest.cpp) target_link_libraries(EnumTest @@ -332,8 +341,16 @@ endif() # Common thrift code generation rules # +add_custom_command(OUTPUT gen-cpp/AnnotationTest_constants.cpp + gen-cpp/AnnotationTest_constants.h + gen-cpp/AnnotationTest_types.cpp + gen-cpp/AnnotationTest_types.h + gen-cpp/foo_service.cpp + gen-cpp/foo_service.h + COMMAND ${THRIFT_COMPILER} --gen cpp ${PROJECT_SOURCE_DIR}/test/AnnotationTest.thrift +) -add_custom_command(OUTPUT gen-cpp/DebugProtoTest_types.cpp gen-cpp/DebugProtoTest_types.h gen-cpp/EmptyService.cpp gen-cpp/EmptyService.h +add_custom_command(OUTPUT gen-cpp/DebugProtoTest_types.cpp gen-cpp/DebugProtoTest_types.h gen-cpp/EmptyService.cpp gen-cpp/EmptyService.h COMMAND ${THRIFT_COMPILER} --gen cpp ${PROJECT_SOURCE_DIR}/test/DebugProtoTest.thrift ) http://git-wip-us.apache.org/repos/asf/thrift/blob/b4c190b6/lib/cpp/test/Makefile.am ---------------------------------------------------------------------- diff --git a/lib/cpp/test/Makefile.am b/lib/cpp/test/Makefile.am index 6f46117..d387297 100755 --- a/lib/cpp/test/Makefile.am +++ b/lib/cpp/test/Makefile.am @@ -18,7 +18,8 @@ # AUTOMAKE_OPTIONS = subdir-objects serial-tests -BUILT_SOURCES = gen-cpp/DebugProtoTest_types.h \ +BUILT_SOURCES = gen-cpp/AnnotationTest_types.h \ + gen-cpp/DebugProtoTest_types.h \ gen-cpp/EnumTest_types.h \ gen-cpp/OptionalRequiredTest_types.h \ gen-cpp/Recursive_types.h \ @@ -31,6 +32,8 @@ BUILT_SOURCES = gen-cpp/DebugProtoTest_types.h \ noinst_LTLIBRARIES = libtestgencpp.la libprocessortest.la nodist_libtestgencpp_la_SOURCES = \ + gen-cpp/AnnotationTest_types.cpp \ + gen-cpp/AnnotationTest_types.h \ gen-cpp/DebugProtoTest_types.cpp \ gen-cpp/DebugProtoTest_types.h \ gen-cpp/EnumTest_types.cpp \ @@ -89,7 +92,8 @@ check_PROGRAMS = \ TFileTransportTest \ link_test \ OpenSSLManualInitTest \ - EnumTest + EnumTest \ + AnnotationTest if AMX_HAVE_LIBEVENT noinst_PROGRAMS += \ @@ -178,12 +182,19 @@ ZlibTest_LDADD = \ -lz EnumTest_SOURCES = \ - EnumTest.cpp + EnumTest.cpp EnumTest_LDADD = \ libtestgencpp.la \ $(BOOST_TEST_LDADD) +AnnotationTest_SOURCES = \ + AnnotationTest.cpp + +AnnotationTest_LDADD = \ + libtestgencpp.la \ + $(BOOST_TEST_LDADD) + TFileTransportTest_SOURCES = \ TFileTransportTest.cpp @@ -334,6 +345,9 @@ OpenSSLManualInitTest_LDADD = \ # THRIFT = $(top_builddir)/compiler/cpp/thrift +gen-cpp/AnnotationTest_constants.cpp gen-cpp/AnnotationTest_constants.h gen-cpp/AnnotationTest_types.cpp gen-cpp/AnnotationTest_types.h: $(top_srcdir)/test/AnnotationTest.thrift + $(THRIFT) --gen cpp $< + gen-cpp/DebugProtoTest_types.cpp gen-cpp/DebugProtoTest_types.h gen-cpp/EmptyService.cpp gen-cpp/EmptyService.h: $(top_srcdir)/test/DebugProtoTest.thrift $(THRIFT) --gen cpp $< http://git-wip-us.apache.org/repos/asf/thrift/blob/b4c190b6/test/AnnotationTest.thrift ---------------------------------------------------------------------- diff --git a/test/AnnotationTest.thrift b/test/AnnotationTest.thrift index 191995a..7e24e1c 100644 --- a/test/AnnotationTest.thrift +++ b/test/AnnotationTest.thrift @@ -57,6 +57,15 @@ senum seasons { "Winter" } ( foo = "bar" ) +struct ostr_default { + 1: i32 bar; +} + +struct ostr_custom { + 1: i32 bar; +} (cpp.customostream) + + service foo_service { void foo() ( foo = "bar" ) } (a.b="c")
