From fa232ac544f89e33e04bc8477d9f0a63a7eb4038 Mon Sep 17 00:00:00 2001 From: Jack Chistyakov Date: Wed, 17 May 2023 12:34:15 -0700 Subject: [PATCH] Legacy/compat setter for containers Summary: Support container (map/list/set) setters in legacy/compat mode - for complete forwards/backwards compatibility with the legacy Go generator. Reviewed By: leoleovich Differential Revision: D45838418 fbshipit-source-id: 495b1928682da8f5d66c3d75137225a3e2987841 --- .../compiler/generate/t_mstch_go_generator.cc | 41 ++++++++++++++----- .../go/types/field_method_set.mustache | 10 ++++- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/third-party/thrift/src/thrift/compiler/generate/t_mstch_go_generator.cc b/third-party/thrift/src/thrift/compiler/generate/t_mstch_go_generator.cc index 07bf89ae2a3d80..a5c431c475ee11 100644 --- a/third-party/thrift/src/thrift/compiler/generate/t_mstch_go_generator.cc +++ b/third-party/thrift/src/thrift/compiler/generate/t_mstch_go_generator.cc @@ -322,6 +322,8 @@ class mstch_go_field : public mstch_field { {"field:pointer?", &mstch_go_field::is_pointer}, {"field:compat_setter_pointer?", &mstch_go_field::is_compat_setter_pointer}, + {"field:compat_setter_value_op", + &mstch_go_field::compat_setter_value_op}, {"field:nilable?", &mstch_go_field::is_nilable}, {"field:dereference?", &mstch_go_field::should_dereference}, {"field:key_str", &mstch_go_field::key_str}, @@ -372,17 +374,17 @@ class mstch_go_field : public mstch_field { auto real_type = field_->type()->get_true_type(); return is_pointer_() && !go::is_type_go_struct(real_type); } - mstch::node is_compat_setter_pointer() { - // Whether this field's value should be a poitner in compat-setter. - // This is needed for a seamless migration from the legacy generator. - // - // Legacy generator would make optional fields with default values be - // impelemented as non-pointer fields. This is incorrect because such - // field cannot be unset (i.e set to nil) despite being optional. - // This ommision is fixed in the new generator, however, our compat - // setters must be backwards compatible to ensure graceful migration. - bool has_default_value = (field_->default_value() != nullptr); - return is_pointer_() && !(is_optional_() && has_default_value); + mstch::node is_compat_setter_pointer() { return is_compat_setter_pointer_(); } + mstch::node compat_setter_value_op() { + if (is_pointer_() && is_compat_setter_pointer_()) { + return std::string(""); + } else if (is_pointer_() && !is_compat_setter_pointer_()) { + return std::string("&"); + } else if (!is_pointer_() && !is_compat_setter_pointer_()) { + return std::string(""); + } else { // if (!is_pointer_() && is_compat_setter_pointer_()) + return std::string("*"); + } } mstch::node key_str() { // Legacy schemas may have negative tags - replace minus with an underscore. @@ -423,6 +425,23 @@ class mstch_go_field : public mstch_field { !go::is_type_nilable(real_type); } + bool is_compat_setter_pointer_() { + // Whether this field's value should be a pointer in compat-setter. + // This is needed for a seamless migration from the legacy generator. + // + // Legacy generator treats optional fields with default values differently + // compared to the new generator (pointer vs non-pointer). + // This method helps with backwards compatibility. + bool has_default_value = (field_->default_value() != nullptr); + if (is_optional_() && has_default_value) { + auto real_type = field_->type()->get_true_type(); + bool is_container = + (real_type->is_list() || real_type->is_map() || real_type->is_set()); + return is_container; + } + return is_pointer_(); + } + bool is_inside_union_() { // Whether field is part of a union return field_context_ != nullptr && field_context_->strct != nullptr && diff --git a/third-party/thrift/src/thrift/compiler/generate/templates/go/types/field_method_set.mustache b/third-party/thrift/src/thrift/compiler/generate/templates/go/types/field_method_set.mustache index 18e03cf738c895..5373e250ad37c6 100644 --- a/third-party/thrift/src/thrift/compiler/generate/templates/go/types/field_method_set.mustache +++ b/third-party/thrift/src/thrift/compiler/generate/templates/go/types/field_method_set.mustache @@ -26,7 +26,15 @@ func (x *{{struct:go_name}}) {{field:go_setter_name}}{{#program:compat_setters?} {{#program:compat_setters?}} func (x *{{struct:go_name}}) {{field:go_setter_name}}(value {{#field:compat_setter_pointer?}}*{{/field:compat_setter_pointer?}}{{#field:type}}{{> common/type}}{{/field:type}}) *{{struct:go_name}} { - x.{{field:go_name}} = {{^field:compat_setter_pointer?}}{{#field:pointer?}}&{{/field:pointer?}}{{/field:compat_setter_pointer?}}value + {{#field:compat_setter_pointer?}} + {{^field:pointer?}} + if (value == nil) { + x.{{field:go_name}} = nil + return x + } + {{/field:pointer?}} + {{/field:compat_setter_pointer?}} + x.{{field:go_name}} = {{field:compat_setter_value_op}}value return x } {{/program:compat_setters?}}