Skip to content

Commit

Permalink
Add mechanisms to disable schema const
Browse files Browse the repository at this point in the history
Summary:
In order to avoid needing weak symbols we would have to ensure the named symbol is always available. This diff handles the case where the path is not well-known, for example because the file is included using a file-relative path instead of a repo-relative path.
The file without a well-known path applies this annotation to its package statement, and is excluded from the list of transitive includes so its constant name does not have to be known.
Since every file using file-relative includes would need this annotation, we instead automatically disable schema const generation for files using such includes as there are a large number and we want to bring that number down but not block schema bundling on that.

Reviewed By: yoney

Differential Revision: D65977818

fbshipit-source-id: 02a1351a2b0fd52332ac068c4f53212b051623e6
  • Loading branch information
iahs authored and facebook-github-bot committed Nov 20, 2024
1 parent 3257c5e commit 730f090
Show file tree
Hide file tree
Showing 186 changed files with 8,239 additions and 157 deletions.
7 changes: 7 additions & 0 deletions third-party/thrift/src/thrift/annotation/scope.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,10 @@ struct RootDefinition {}
@EnumValue
@Transitive
struct Definition {}

/**
* Not a scope, just here for dependency cycle reasons.
* Disables schema const injection for the program.
*/
@Program
struct DisableSchemaConst {}
2 changes: 2 additions & 0 deletions third-party/thrift/src/thrift/compiler/ast/uri.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ inline constexpr auto kAllowReservedIdentifierUri =
"facebook.com/thrift/annotation/AllowReservedIdentifier";
inline constexpr auto kAllowReservedFilenameUri =
"facebook.com/thrift/annotation/AllowReservedFilename";
inline constexpr auto kDisableSchemaConstUri =
"facebook.com/thrift/annotation/DisableSchemaConst";

// Scope:
inline constexpr auto kScopeProgramUri =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,8 @@ class cpp_mstch_program : public mstch_program {
{"program:split_enums", &cpp_mstch_program::split_enums},
{"program:has_schema?", &cpp_mstch_program::has_schema},
{"program:schema_name", &cpp_mstch_program::schema_name},
{"program:schema_includes_const?",
&cpp_mstch_program::schema_includes_const},
{"program:needs_sinit?", &cpp_mstch_program::needs_sinit},
{"program:structs_and_typedefs",
&cpp_mstch_program::structs_and_typedefs}});
Expand Down Expand Up @@ -516,6 +518,10 @@ class cpp_mstch_program : public mstch_program {
auto includes = std::make_unique<transitive_include_map>();
auto local_includes = program->get_includes_for_codegen();
for (const auto* include : local_includes) {
if (include->find_structured_annotation_or_null(kDisableSchemaConstUri)) {
continue;
}

includes->emplace(
include,
fmt::format(
Expand Down Expand Up @@ -757,9 +763,35 @@ class cpp_mstch_program : public mstch_program {

mstch::node schema_name() { return schematizer::name_schema(sm_, *program_); }

mstch::node schema_includes_const() {
return supports_schema_includes(program_);
}

mstch::node needs_sinit() { return has_option("any"); }

private:
bool supports_schema_includes(const t_program* program) {
enum class strong_bool {};
strong_bool supports = context_.cache().get(*program, [&] {
bool ret =
// Opting out of schema const should disable all of its failure modes.
!program->find_structured_annotation_or_null(
kDisableSchemaConstUri) &&
// File-relative includes break schema const naming,
// so exclude programs with includes that don't contain a path
// separator.
std::all_of(
program->includes().begin(),
program->includes().end(),
[](const t_include* inc) {
auto path = inc->raw_path();
return std::find(path.begin(), path.end(), '/') != path.end();
});
return std::make_unique<strong_bool>(static_cast<strong_bool>(ret));
});
return static_cast<bool>(supports);
}

const std::optional<int32_t> split_id_;
const std::optional<std::vector<t_structured*>> split_structs_;
std::optional<std::vector<t_enum*>> split_enums_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ namespace {{program:name}}_constants {
}
{{/program:has_schema?}}
::folly::Range<const ::std::string_view*> {{program:schema_name}}_includes() {
{{#if program:schema_includes_const?}}
#if FBTHRIFT_CAN_POPULATE_SCHEMA_LIST
static const ::std::array<::std::string_view, {{program:num_transitive_thrift_includes}}> includes = {
{{#program:transitive_schema_initializers}}
Expand All @@ -84,6 +85,9 @@ namespace {{program:name}}_constants {
#else
return {};
#endif
{{else}}
return {};
{{/if}}
}

} // namespace {{program:name}}_constants
Expand Down
4 changes: 4 additions & 0 deletions third-party/thrift/src/thrift/compiler/sema/sema.cc
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,10 @@ void lower_type_annotations(
}

void inject_schema_const(sema_context& ctx, mutator_context&, t_program& prog) {
if (prog.find_structured_annotation_or_null(kDisableSchemaConstUri)) {
return;
}

detail::schematizer::options opts;
opts.only_root_program_ = true;
opts.use_hash = true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/**
* Autogenerated by Thrift
*
* DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING
* @generated
*/
package com.facebook.thrift.annotation_deprecated;

import java.util.List;
import java.util.ArrayList;
import java.util.Map;
import java.util.HashMap;
import java.util.Set;
import java.util.HashSet;
import java.util.Collections;
import java.util.BitSet;
import java.util.Arrays;
import com.facebook.thrift.*;
import com.facebook.thrift.annotations.*;
import com.facebook.thrift.async.*;
import com.facebook.thrift.meta_data.*;
import com.facebook.thrift.server.*;
import com.facebook.thrift.transport.*;
import com.facebook.thrift.protocol.*;

/**
* Not a scope, just here for dependency cycle reasons.
* Disables schema const injection for the program.
*/
@SuppressWarnings({ "unused", "serial" })
public class DisableSchemaConst implements TBase, java.io.Serializable, Cloneable {
private static final TStruct STRUCT_DESC = new TStruct("DisableSchemaConst");


public DisableSchemaConst() {
}

/**
* Performs a deep copy on <i>other</i>.
*/
public DisableSchemaConst(DisableSchemaConst other) {
}

public DisableSchemaConst deepCopy() {
return new DisableSchemaConst(this);
}

@Override
public boolean equals(Object _that) {
if (_that == null)
return false;
if (this == _that)
return true;
if (!(_that instanceof DisableSchemaConst))
return false;
DisableSchemaConst that = (DisableSchemaConst)_that;

return true;
}

@Override
public int hashCode() {
return Arrays.deepHashCode(new Object[] {});
}

// This is required to satisfy the TBase interface, but can't be implemented on immutable struture.
public void read(TProtocol iprot) throws TException {
throw new TException("unimplemented in android immutable structure");
}

public static DisableSchemaConst deserialize(TProtocol iprot) throws TException {
TField __field;
iprot.readStructBegin();
while (true)
{
__field = iprot.readFieldBegin();
if (__field.type == TType.STOP) {
break;
}
switch (__field.id)
{
default:
TProtocolUtil.skip(iprot, __field.type);
break;
}
iprot.readFieldEnd();
}
iprot.readStructEnd();

DisableSchemaConst _that;
_that = new DisableSchemaConst(
);
_that.validate();
return _that;
}

public void write(TProtocol oprot) throws TException {
validate();

oprot.writeStructBegin(STRUCT_DESC);
oprot.writeFieldStop();
oprot.writeStructEnd();
}

@Override
public String toString() {
return toString(1, true);
}

@Override
public String toString(int indent, boolean prettyPrint) {
return TBaseHelper.toStringHelper(this, indent, prettyPrint);
}

public void validate() throws TException {
// check for required fields
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var (
premadeCodecTypeSpec_scope_Interface *thrift.TypeSpec = nil
premadeCodecTypeSpec_scope_RootDefinition *thrift.TypeSpec = nil
premadeCodecTypeSpec_scope_Definition *thrift.TypeSpec = nil
premadeCodecTypeSpec_scope_DisableSchemaConst *thrift.TypeSpec = nil
)

// Premade codec specs initializer
Expand Down Expand Up @@ -192,6 +193,15 @@ var premadeCodecSpecsInitOnce = sync.OnceFunc(func() {
NewFunc: func() thrift.Struct { return NewDefinition() },
},

}
premadeCodecTypeSpec_scope_DisableSchemaConst = &thrift.TypeSpec{
FullName: "scope.DisableSchemaConst",
CodecStructSpec: &thrift.CodecStructSpec{
ScopedName: "scope.DisableSchemaConst",
IsUnion: false,
NewFunc: func() thrift.Struct { return NewDisableSchemaConst() },
},

}
})

Expand All @@ -214,6 +224,7 @@ var (
premadeStructSpec_Interface *thrift.StructSpec = nil
premadeStructSpec_RootDefinition *thrift.StructSpec = nil
premadeStructSpec_Definition *thrift.StructSpec = nil
premadeStructSpec_DisableSchemaConst *thrift.StructSpec = nil
)

// Premade struct specs initializer
Expand Down Expand Up @@ -421,6 +432,18 @@ var premadeStructSpecsInitOnce = sync.OnceFunc(func() {
},
FieldSpecNameToIndex: map[string]int{
},
}
premadeStructSpec_DisableSchemaConst = &thrift.StructSpec{
Name: "DisableSchemaConst",
ScopedName: "scope.DisableSchemaConst",
IsUnion: false,
IsException: false,
FieldSpecs: []thrift.FieldSpec{
},
FieldSpecIDToIndex: map[int16]int{
},
FieldSpecNameToIndex: map[string]int{
},
}
})

Expand All @@ -447,6 +470,7 @@ var premadeCodecSpecsMapOnce = sync.OnceValue(
fbthriftTypeSpecsMap[premadeCodecTypeSpec_scope_Interface.FullName] = premadeCodecTypeSpec_scope_Interface
fbthriftTypeSpecsMap[premadeCodecTypeSpec_scope_RootDefinition.FullName] = premadeCodecTypeSpec_scope_RootDefinition
fbthriftTypeSpecsMap[premadeCodecTypeSpec_scope_Definition.FullName] = premadeCodecTypeSpec_scope_Definition
fbthriftTypeSpecsMap[premadeCodecTypeSpec_scope_DisableSchemaConst.FullName] = premadeCodecTypeSpec_scope_DisableSchemaConst
return fbthriftTypeSpecsMap
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var (
premadeThriftType_scope_Interface *metadata.ThriftType = nil
premadeThriftType_scope_RootDefinition *metadata.ThriftType = nil
premadeThriftType_scope_Definition *metadata.ThriftType = nil
premadeThriftType_scope_DisableSchemaConst *metadata.ThriftType = nil
)

// Premade Thrift type initializer
Expand Down Expand Up @@ -109,6 +110,10 @@ var premadeThriftTypesInitOnce = sync.OnceFunc(func() {
metadata.NewThriftStructType().
SetName("scope.Definition"),
)
premadeThriftType_scope_DisableSchemaConst = metadata.NewThriftType().SetTStruct(
metadata.NewThriftStructType().
SetName("scope.DisableSchemaConst"),
)
})

// Helper type to allow us to store Thrift types in a slice at compile time,
Expand Down Expand Up @@ -142,6 +147,7 @@ var premadeThriftTypesMapOnce = sync.OnceValue(
thriftTypesWithFullName = append(thriftTypesWithFullName, thriftTypeWithFullName{ "scope.Interface", premadeThriftType_scope_Interface })
thriftTypesWithFullName = append(thriftTypesWithFullName, thriftTypeWithFullName{ "scope.RootDefinition", premadeThriftType_scope_RootDefinition })
thriftTypesWithFullName = append(thriftTypesWithFullName, thriftTypeWithFullName{ "scope.Definition", premadeThriftType_scope_Definition })
thriftTypesWithFullName = append(thriftTypesWithFullName, thriftTypeWithFullName{ "scope.DisableSchemaConst", premadeThriftType_scope_DisableSchemaConst })

fbthriftThriftTypesMap := make(map[string]*metadata.ThriftType, len(thriftTypesWithFullName))
for _, value := range thriftTypesWithFullName {
Expand Down Expand Up @@ -207,6 +213,9 @@ var structMetadatasOnce = sync.OnceValue(
SetIsUnion(false))
fbthriftResults = append(fbthriftResults, metadata.NewThriftStruct().
SetName("scope.Definition").
SetIsUnion(false))
fbthriftResults = append(fbthriftResults, metadata.NewThriftStruct().
SetName("scope.DisableSchemaConst").
SetIsUnion(false))
return fbthriftResults
},
Expand Down
Loading

0 comments on commit 730f090

Please sign in to comment.