Fix ODR issues around weak symbols for bundled runtime schema

Summary:
This diff removes the default implementation for the weak symbol used for finding runtime schema information of transitively included Thrift files.

If a thrift file is included in more than one file, then we have an ODR violation for the default implementation provided by the weak symbols. In practice, I have witnessed that this causes in some cases, for the default (empty) implementation returning `""` to be picked instead of the strongly linked symbol.

The bug has manifested in weird ways such as different implementations being picked depending on which header files are `#include`-ed, even though they do not cause any changes in the transitively linked-to targets in BUCK.

The fix is to only have forward declarations for the weak symbols with no definitions. This passes ODR since declarations are not definitions, and so we introduce a helper function to deal with the fact that the symbol can now be `nullptr`. Now, build are working again.

Reviewed By: pranavtbhat

Differential Revision: D65635942

fbshipit-source-id: 012e3942c7894c4a35d323a0ffb127dfcf16d23e

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

@@ -519,7 +519,7 @@ class cpp_mstch_program : public mstch_program {
       includes->emplace(
           include,
           fmt::format(
-              "{}::{}_constants::{}()",
+              "::apache::thrift::detail::mc::readSchema({}::{}_constants::{})",
               t_mstch_cpp2_generator::get_cpp2_namespace(include),
               include->name(),
               schematizer::name_schema(sm_, *include)));
@@ -533,7 +533,7 @@ class cpp_mstch_program : public mstch_program {
           includes->emplace(
               recursive_include,
               fmt::format(
-                  "{}::{}_constants::{}_includes()[{}]",
+                  "::apache::thrift::detail::mc::readSchemaInclude({}::{}_constants::{}_includes, {})",
                   t_mstch_cpp2_generator::get_cpp2_namespace(include),
                   include->name(),
                   schematizer::name_schema(sm_, *include),

third-party/thrift/src/thrift/compiler/generate/templates/cpp2/module_constants.cpp.mustache

@@ -32,11 +32,8 @@
 // {{program:autogen_path}}
 {{ > common/namespace_cpp2_begin}}
 namespace {{program:name}}_constants {
-FOLLY_ATTR_WEAK ::std::string_view {{program:schema_name}}() { return ""; }
-FOLLY_ATTR_WEAK ::folly::Range<const ::std::string_view*> {{program:schema_name}}_includes() {
-  static const ::std::array<::std::string_view, {{program:num_transitive_thrift_includes}}> empty;
-  return ::folly::range(empty);
- }
+FOLLY_ATTR_WEAK ::std::string_view {{program:schema_name}}();
+FOLLY_ATTR_WEAK ::folly::Range<const ::std::string_view*> {{program:schema_name}}_includes();
 }{{ > common/namespace_cpp2_end}}
 
 {{/program:thrift_includes}}

third-party/thrift/src/thrift/compiler/test/fixtures/service-schema/out/cpp2/gen-cpp2/module_constants.cpp

@@ -15,11 +15,8 @@
 
 // include.thrift
 namespace cpp2 {namespace include_constants {
-FOLLY_ATTR_WEAK ::std::string_view _fbthrift_schema_8569dfae849b43aa() { return ""; }
-FOLLY_ATTR_WEAK ::folly::Range<const ::std::string_view*> _fbthrift_schema_8569dfae849b43aa_includes() {
-  static const ::std::array<::std::string_view, 1> empty;
-  return ::folly::range(empty);
- }
+FOLLY_ATTR_WEAK ::std::string_view _fbthrift_schema_8569dfae849b43aa();
+FOLLY_ATTR_WEAK ::folly::Range<const ::std::string_view*> _fbthrift_schema_8569dfae849b43aa_includes();
 }} // namespace cpp2
 
 #endif
@@ -37,7 +34,7 @@ ::folly::Range<const ::std::string_view*> _fbthrift_schema_b747839c13cb3aa5_incl
 #if FBTHRIFT_CAN_POPULATE_SCHEMA_LIST
   static const ::std::array<::std::string_view, 2> includes = {
     _fbthrift_schema_b747839c13cb3aa5(),
-    ::cpp2::include_constants::_fbthrift_schema_8569dfae849b43aa(),
+    ::apache::thrift::detail::mc::readSchema(::cpp2::include_constants::_fbthrift_schema_8569dfae849b43aa),
   };
   return ::folly::range(includes);
 #else

third-party/thrift/src/thrift/compiler/test/fixtures/service-schema/out/cpp2_sinit/gen-cpp2/module_constants.cpp

@@ -15,11 +15,8 @@
 
 // include.thrift
 namespace cpp2 {namespace include_constants {
-FOLLY_ATTR_WEAK ::std::string_view _fbthrift_schema_8569dfae849b43aa() { return ""; }
-FOLLY_ATTR_WEAK ::folly::Range<const ::std::string_view*> _fbthrift_schema_8569dfae849b43aa_includes() {
-  static const ::std::array<::std::string_view, 1> empty;
-  return ::folly::range(empty);
- }
+FOLLY_ATTR_WEAK ::std::string_view _fbthrift_schema_8569dfae849b43aa();
+FOLLY_ATTR_WEAK ::folly::Range<const ::std::string_view*> _fbthrift_schema_8569dfae849b43aa_includes();
 }} // namespace cpp2
 
 #endif
@@ -37,7 +34,7 @@ ::folly::Range<const ::std::string_view*> _fbthrift_schema_b747839c13cb3aa5_incl
 #if FBTHRIFT_CAN_POPULATE_SCHEMA_LIST
   static const ::std::array<::std::string_view, 2> includes = {
     _fbthrift_schema_b747839c13cb3aa5(),
-    ::cpp2::include_constants::_fbthrift_schema_8569dfae849b43aa(),
+    ::apache::thrift::detail::mc::readSchema(::cpp2::include_constants::_fbthrift_schema_8569dfae849b43aa),
   };
   return ::folly::range(includes);
 #else

third-party/thrift/src/thrift/lib/cpp2/gen/module_constants_cpp.cpp

@@ -0,0 +1,31 @@
+/*
+ * 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.
+ */
+
+#include <thrift/lib/cpp2/gen/module_constants_cpp.h>
+
+namespace apache::thrift::detail::mc {
+
+::std::string_view readSchema(::std::string_view (*access)()) {
+  return access == nullptr ? ::std::string_view() : access();
+}
+
+::std::string_view readSchemaInclude(
+    ::folly::Range<const ::std::string_view*> (*access)(),
+    ::std::size_t index) {
+  return access == nullptr ? ::std::string_view() : access()[index];
+}
+
+} // namespace apache::thrift::detail::mc

third-party/thrift/src/thrift/lib/cpp2/gen/module_constants_cpp.h

@@ -17,9 +17,12 @@
 #pragma once
 
 #include <array>
+#include <cstdint>
+#include <string_view>
 
 #include <folly/CPortability.h>
 #include <folly/Indestructible.h>
+#include <folly/Range.h>
 #include <folly/lang/Exception.h>
 
 FOLLY_GNU_DISABLE_WARNING("-Woverlength-strings")
@@ -28,3 +31,12 @@ FOLLY_GNU_DISABLE_WARNING("-Wtrigraphs")
 // Schema constant depends on weak symbols to work around legacy include
 // resolution behavior.
 #define FBTHRIFT_CAN_POPULATE_SCHEMA_LIST FOLLY_HAVE_WEAK_SYMBOLS
+
+namespace apache::thrift::detail::mc {
+
+::std::string_view readSchema(::std::string_view (*access)());
+
+::std::string_view readSchemaInclude(
+    ::folly::Range<const ::std::string_view*> (*access)(), ::std::size_t index);
+
+} // namespace apache::thrift::detail::mc