Qualify enclosing-struct param in adapted_field_t storage type (unblock frozen2 + field @cpp.Adapter)

Summary:
Field-level `cpp.Adapter` storage type `adapted_field_t<Adapter, id, ThriftType, EnclosingStruct>` emitted `EnclosingStruct` unqualified. Under `namespace apache::thrift::frozen` (frozen2 layouts) the bare name doesn't resolve, so `*_layouts.h` didn't compile and such structs couldn't be frozen.

Adds `get_qualified_storage_type` / `get_qualified_native_type` to qualify only the enclosing-struct param, opt-in via a new `cpp_frozen2_storage_type` property. Default `get_storage_type` stays unqualified, so `*_types.h` is unchanged.

Reviewed By: vitaut

Differential Revision: D115490480

fbshipit-source-id: 5a144467edf9d22e670fe38a6010590b62eee062

Author: Uday Garikipati

third-party/thrift/src/thrift/compiler/generate/cpp/name_resolver.cc

@@ -66,6 +66,25 @@ std::string gen_template_type(
 
 const std::string& cpp_name_resolver::get_native_type(
     const t_field& field, const t_structured& parent) {
+  return detail::get_or_gen(field_type_cache_, &field, [&]() {
+    return gen_field_native_type(field, parent, /*qualify_parent=*/false);
+  });
+}
+
+const std::string& cpp_name_resolver::get_qualified_native_type(
+    const t_field& field, const t_structured& parent) {
+  // Only a field-level @cpp.Adapter names its enclosing struct; every other
+  // field resolves identically to get_native_type.
+  if (field.find_structured_annotation_or_null(kCppAdapterUri) == nullptr) {
+    return get_native_type(field, parent);
+  }
+  return detail::get_or_gen(qualified_field_type_cache_, &field, [&]() {
+    return gen_field_native_type(field, parent, /*qualify_parent=*/true);
+  });
+}
+
+std::string cpp_name_resolver::gen_field_native_type(
+    const t_field& field, const t_structured& parent, bool qualify_parent) {
   const t_type& type = *field.type();
 
   // Handle @cpp.Adapter on field.
@@ -78,50 +97,51 @@ const std::string& cpp_name_resolver::get_native_type(
     }
     const auto& adapter_on_field =
         annotation->get_value_from_structured_annotation("name").get_string();
-    return detail::get_or_gen(field_type_cache_, &field, [&]() {
-      // When the field also has @cpp.Type, use that as the base type for the
-      // adapter wrapping instead of resolving from the type node.
-      if (auto* cpp_type_annot =
-              field.find_structured_annotation_or_null(kCppTypeUri)) {
-        if (auto* name =
-                cpp_type_annot->get_value_from_structured_annotation_or_null(
-                    "name")) {
-          return gen_adapted_type(
-              &adapter_on_field, field.id(), name->get_string(), parent);
-        }
-        if (auto* tmpl =
-                cpp_type_annot->get_value_from_structured_annotation_or_null(
-                    "template")) {
-          return gen_adapted_type(
-              &adapter_on_field,
-              field.id(),
-              gen_container_type(
-                  type.get_true_type()->as<t_container>(),
-                  &cpp_name_resolver::get_native_type,
-                  &tmpl->get_string()),
-              parent);
-        }
+    // When the field also has @cpp.Type, use that as the base type for the
+    // adapter wrapping instead of resolving from the type node.
+    if (auto* cpp_type_annot =
+            field.find_structured_annotation_or_null(kCppTypeUri)) {
+      if (auto* name =
+              cpp_type_annot->get_value_from_structured_annotation_or_null(
+                  "name")) {
+        return gen_adapted_type(
+            &adapter_on_field,
+            field.id(),
+            name->get_string(),
+            parent,
+            qualify_parent);
       }
-      return gen_field_type(field.id(), type, parent, &adapter_on_field);
-    });
+      if (auto* tmpl =
+              cpp_type_annot->get_value_from_structured_annotation_or_null(
+                  "template")) {
+        return gen_adapted_type(
+            &adapter_on_field,
+            field.id(),
+            gen_container_type(
+                type.get_true_type()->as<t_container>(),
+                &cpp_name_resolver::get_native_type,
+                &tmpl->get_string()),
+            parent,
+            qualify_parent);
+      }
+    }
+    return gen_field_type(
+        field.id(), type, parent, &adapter_on_field, qualify_parent);
   }
 
   // Handle @cpp.Type on field.
   if (auto* annotation =
           field.find_structured_annotation_or_null(kCppTypeUri)) {
     if (auto name =
             annotation->get_value_from_structured_annotation_or_null("name")) {
-      return detail::get_or_gen(
-          field_type_cache_, &field, [&]() { return name->get_string(); });
+      return name->get_string();
     } else {
       auto& tmplate =
           annotation->get_value_from_structured_annotation("template");
-      return detail::get_or_gen(field_type_cache_, &field, [&]() {
-        return gen_container_type(
-            type.get_true_type()->as<t_container>(),
-            &cpp_name_resolver::get_native_type,
-            &tmplate.get_string());
-      });
+      return gen_container_type(
+          type.get_true_type()->as<t_container>(),
+          &cpp_name_resolver::get_native_type,
+          &tmplate.get_string());
     }
   }
 
@@ -322,6 +342,18 @@ const std::string& cpp_name_resolver::get_storage_type(
   });
 }
 
+const std::string& cpp_name_resolver::get_qualified_storage_type(
+    const t_field& field, const t_structured& parent) {
+  auto ref_type = gen::cpp::find_ref_type(field);
+  const std::string& native_type = get_qualified_native_type(field, parent);
+  if (ref_type == cpp_reference_type::none) {
+    return native_type;
+  }
+  return detail::get_or_gen(qualified_storage_type_cache_, &field, [&]() {
+    return gen_storage_type(native_type, ref_type);
+  });
+}
+
 const std::string& cpp_name_resolver::get_reference_type(const t_field& node) {
   return detail::get_or_gen(field_reference_type_cache_, &node, [&]() {
     return gen_reference_type(node);
@@ -609,17 +641,18 @@ std::string cpp_name_resolver::gen_adapted_type(
     const std::string* adapter,
     int16_t field_id,
     const std::string& standard_type,
-    const t_structured& parent) {
-  return adapter == nullptr
-      ? standard_type
-      : detail::gen_template_type(
-            "::apache::thrift::adapt_detail::adapted_field_t",
-            {
-                *adapter,
-                std::to_string(field_id),
-                standard_type,
-                get_underlying_name(parent),
-            });
+    const t_structured& parent,
+    bool qualify_parent) {
+  if (adapter == nullptr) {
+    return standard_type;
+  }
+  return detail::gen_template_type(
+      "::apache::thrift::adapt_detail::adapted_field_t",
+      {*adapter,
+       std::to_string(field_id),
+       standard_type,
+       qualify_parent ? get_underlying_namespaced_name(parent)
+                      : get_underlying_name(parent)});
 }
 
 std::string cpp_name_resolver::gen_type_tag(

third-party/thrift/src/thrift/compiler/generate/cpp/name_resolver.h

@@ -66,6 +66,10 @@ class cpp_name_resolver {
   // Returns C++ type name for the given Thrift field.
   const std::string& get_native_type(
       const t_field& field, const t_structured& parent);
+  // Like get_native_type(field, parent) but namespace-qualifies a field-level
+  // @cpp.Adapter's enclosing-struct parameter (see get_qualified_storage_type).
+  const std::string& get_qualified_native_type(
+      const t_field& field, const t_structured& parent);
 
   const std::string& get_native_type(const t_const& cnst);
 
@@ -158,6 +162,12 @@ class cpp_name_resolver {
   const std::string& get_storage_type(
       const t_field& field, const t_structured& parent);
 
+  // Like get_storage_type, but namespace-qualifies a field-level @cpp.Adapter's
+  // enclosing-struct parameter so the storage type is valid when emitted
+  // outside the struct's own namespace.
+  const std::string& get_qualified_storage_type(
+      const t_field& field, const t_structured& parent);
+
   // Returns the C++ reference type of the field.
   const std::string& get_reference_type(const t_field& node);
 
@@ -195,6 +205,8 @@ class cpp_name_resolver {
   std::unordered_map<const t_stream*, std::string> stream_cache_;
   std::unordered_map<const t_const*, std::string> const_cache_;
   std::unordered_map<const t_field*, std::string> field_type_cache_;
+  std::unordered_map<const t_field*, std::string> qualified_field_type_cache_;
+  std::unordered_map<const t_field*, std::string> qualified_storage_type_cache_;
   std::unordered_map<const t_type*, std::string> standard_type_cache_;
   std::unordered_map<const t_field*, std::string> field_standard_type_cache_;
   std::unordered_map<const t_type*, std::string> underlying_type_cache_;
@@ -240,9 +252,16 @@ class cpp_name_resolver {
       int16_t field_id,
       const t_type& type,
       const t_structured& parent,
-      const std::string* adapter) {
-    return gen_adapted_type(adapter, field_id, gen_type(type), parent);
+      const std::string* adapter,
+      bool qualify_parent = false) {
+    return gen_adapted_type(
+        adapter, field_id, gen_type(type), parent, qualify_parent);
   }
+  // Shared body behind get_native_type / get_qualified_native_type. When
+  // qualify_parent is set, a field-level @cpp.Adapter's enclosing-struct param
+  // is namespace-qualified.
+  std::string gen_field_native_type(
+      const t_field& field, const t_structured& parent, bool qualify_parent);
   std::string gen_standard_type(const t_type& node);
   std::string gen_standard_type(const t_type& node, type_resolve_fn resolve_fn);
   std::string gen_standard_type(const t_field& node);
@@ -254,11 +273,18 @@ class cpp_name_resolver {
       const std::string* templte = nullptr);
   static std::string gen_adapted_type(
       const std::string* adapter, const std::string& standard_type);
-  static std::string gen_adapted_type(
+
+  // Generates the storage type for a field-level @cpp.Adapter
+  // (adapted_field_t<Adapter, FieldId, ThriftType, EnclosingStruct>). The
+  // enclosing-struct parameter is unqualified by default (readable where the
+  // type is emitted inside the struct's own namespace); set qualify_parent to
+  // namespace-qualify it for emission elsewhere.
+  std::string gen_adapted_type(
       const std::string* adapter,
       int16_t field_id,
       const std::string& standard_type,
-      const t_structured& parent);
+      const t_structured& parent,
+      bool qualify_parent = false);
 
   std::string gen_thrift_type_tag(const t_type&);
   // TODO(dokwon): Remove ignored_cpp_type once cpp.type lowering migration is

third-party/thrift/src/thrift/compiler/generate/t_mstch_cpp2_generator.cc

@@ -1741,6 +1741,15 @@ class t_mstch_cpp2_generator : public t_whisker_generator {
       assert(parent != nullptr);
       return cpp_context_->resolver().get_storage_type(field, *parent);
     });
+    // Storage type for frozen2 codegen (*_layouts.h): namespace-qualifies a
+    // field-level @cpp.Adapter's enclosing-struct parameter so it resolves
+    // under namespace apache::thrift::frozen.
+    def.property("cpp_frozen2_storage_type", [this](const t_field& field) {
+      const t_structured* parent = context().get_field_parent(&field);
+      assert(parent != nullptr);
+      return cpp_context_->resolver().get_qualified_storage_type(
+          field, *parent);
+    });
     def.property("cpp_standard_type", [this](const t_field& field) {
       return cpp_context_->resolver().get_standard_type(field);
     });

third-party/thrift/src/thrift/compiler/generate/templates/cpp2/module_layouts.h.whisker

@@ -37,11 +37,11 @@ FROZEN_EXCLUDE_TYPE({{def.cpp_fullname}})
       {{#if (not def.union?)}}
 FROZEN_TYPE({{def.cpp_fullname}},{{!
 }}{{#each def.fields as |field|}}
-  FROZEN_FIELD{{#partial module_layouts.field_suffix field=field}}({{field.cpp_name}}, {{field.id}}, {{field.cpp_storage_type}}){{!
+  FROZEN_FIELD{{#partial module_layouts.field_suffix field=field}}({{field.cpp_name}}, {{field.id}}, {{field.cpp_frozen2_storage_type}}){{!
 }}{{/each}}
   FROZEN_VIEW({{!
 }}{{#each def.fields as |field|}}
-    FROZEN_VIEW_FIELD{{#partial module_layouts.field_suffix field=field}}({{field.cpp_name}}, {{field.cpp_storage_type}}){{!
+    FROZEN_VIEW_FIELD{{#partial module_layouts.field_suffix field=field}}({{field.cpp_name}}, {{field.cpp_frozen2_storage_type}}){{!
 }}{{/each}})
   FROZEN_SAVE_INLINE({{!
 }}{{#each def.fields as |field|}}
@@ -56,7 +56,7 @@ FROZEN_TYPE({{def.cpp_fullname}},{{!
 }}
   FROZEN_FIELD(fbthrift_type_, 0, {{def.cpp_fullname}}::Type){{!
 }}{{#each def.fields as |field|}}
-  FROZEN_FIELD{{#partial module_layouts.frozen2_union_field_suffix field=field}}({{field.cpp_name}}, {{field.id}}, {{field.cpp_storage_type}}){{!
+  FROZEN_FIELD{{#partial module_layouts.frozen2_union_field_suffix field=field}}({{field.cpp_name}}, {{field.id}}, {{field.cpp_frozen2_storage_type}}){{!
 }}{{/each}}
   struct View : public ViewBase<View, LayoutSelf, {{def.cpp_fullname}}> {
     {{def.cpp_fullname}}::Type fbthrift_type_;

third-party/thrift/src/thrift/compiler/test/cpp_name_resolver_test.cc

@@ -83,6 +83,10 @@ class CppNameResolverTest : public ::testing::Test {
     return resolver_.get_storage_type(node, struct_);
   }
 
+  const std::string& get_qualified_storage_type(const t_field& node) {
+    return resolver_.get_qualified_storage_type(node, struct_);
+  }
+
   const std::string& get_reference_type(const t_field& node) {
     return resolver_.get_reference_type(node);
   }
@@ -572,6 +576,40 @@ TEST_F(CppNameResolverTest, adapted_field_type) {
       "MyAdapter, 42, ::std::int64_t, ThriftStruct>");
 }
 
+TEST_F(CppNameResolverTest, adapted_field_qualified_storage_type) {
+  // get_qualified_storage_type namespace-qualifies the enclosing-struct
+  // parameter so the storage type is valid when emitted outside the struct's
+  // own namespace, while the default (get_storage_type) keeps the readable
+  // unqualified name.
+  const auto& i64 = t_primitive_type::t_i64();
+  auto cpp_ref = gen::cpp_ref_builder(program_);
+  auto adapter = gen::adapter_builder(program_, "cpp");
+
+  auto field = t_field(i64, "n", 42);
+  field.add_structured_annotation(adapter.make("MyAdapter"));
+  EXPECT_EQ(
+      get_storage_type(field),
+      "::apache::thrift::adapt_detail::adapted_field_t<"
+      "MyAdapter, 42, ::std::int64_t, ThriftStruct>");
+  EXPECT_EQ(
+      get_qualified_storage_type(field),
+      "::apache::thrift::adapt_detail::adapted_field_t<"
+      "MyAdapter, 42, ::std::int64_t, ::path::to::ThriftStruct>");
+
+  // Reference wrapping is applied around the qualified adapted type too.
+  auto unique_field = t_field(i64, "n", 42);
+  unique_field.add_structured_annotation(adapter.make("MyAdapter"));
+  unique_field.add_structured_annotation(cpp_ref.unique());
+  EXPECT_EQ(
+      get_qualified_storage_type(unique_field),
+      "::std::unique_ptr<::apache::thrift::adapt_detail::adapted_field_t<"
+      "MyAdapter, 42, ::std::int64_t, ::path::to::ThriftStruct>>");
+
+  // A field with no adapter resolves identically to get_storage_type.
+  auto plain_field = t_field(i64, "n", 42);
+  EXPECT_EQ(get_qualified_storage_type(plain_field), "::std::int64_t");
+}
+
 TEST_F(CppNameResolverTest, adapted_field_storage_type) {
   const auto& i64 = t_primitive_type::t_i64();
   auto cpp_ref = gen::cpp_ref_builder(program_);

third-party/thrift/src/thrift/lib/cpp2/frozen/test/Adapter.thrift

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * Licensed 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.
+ */
+
+package "facebook.com/thrift/test/frozen/adapter"
+
+include "thrift/annotation/cpp.thrift"
+
+cpp_include "thrift/lib/cpp2/frozen/test/FrozenAdapterTestUtil.h"
+
+// Field-level @cpp.Adapter storage types name their enclosing struct, so the
+// layouts header (emitted under namespace apache::thrift::frozen) can only
+// compile these structs if that name is fully qualified.
+struct AdaptedNested {
+  @cpp.Adapter{name = "::apache::thrift::test::PackedIntListAdapter"}
+  1: binary ids;
+}
+
+struct AdaptedFields {
+  @cpp.Adapter{name = "::apache::thrift::test::PackedIntListAdapter"}
+  1: binary ids;
+
+  @cpp.Adapter{name = "::apache::thrift::test::WidenToI64FieldAdapter"}
+  2: i32 widened;
+
+  @cpp.Adapter{name = "::apache::thrift::test::PackedIntListAdapter"}
+  3: optional binary optionalIds;
+
+  4: AdaptedNested nested;
+
+  5: list<AdaptedNested> nestedList;
+
+  6: string plain;
+}