Skip to content

Commit

Permalink
Delete arrays as a compiler option.
Browse files Browse the repository at this point in the history
Summary:
#What?
Delete `arrays` as a compiler option.

#Why?
`arrays` are now the default in the absence of `legacy_arrays` and `hack_collections`, and cease to exist as an explicit option.

# Fixtures
- Delete `arrays` from `cmd` files.
- Generated fixtures must not change.

# Context
`arraysets`, `no_use_hack_collections`, `stricttypes`, `array_migration`, `shape_arraykeys`, `const_collections` are all compiler options that control the generation of container fields. To make code generator simpler and easier to reason about, identify which of these can be removed/merged.

The new options based on the ones that are currently in use:

`legacy_arrays` replaces `no_use_hack_collections`.
`hack_collections`, which was the implicit default in the absence of `arrays` and `no_use_hack_collections`, is now explicit.
`arrays` are now the default in the absence of `legacy_arrays` and `hack_collections`, and cease to exist as an explicit option.

# The steps
The item in bold corresponds to the current diff.
1. Use new compiler options based on the ones that already exist.
1. Make arrays the default option.
1. Use the legacy arrays + hack collections logical equivalent of arrays.
1. Introduce hack collections wherever necessary.
1. Replace no use hack collections with legacy arrays.
1. Remove arraysets if arrays present.
1. Rename `no_use_hack_collections` compiler option to `legacy_arrays`.
1. Add `hack_collections` compiler option.
1. Remove references to the `arrays` compiler option.
1. **Delete the `arrays` compiler option.**

Reviewed By: rmakheja

Differential Revision: D67618067

fbshipit-source-id: d6c20bd9343f868b1492497f8e41b72ec91c45a6
  • Loading branch information
Satish Kumar authored and facebook-github-bot committed Jan 9, 2025
1 parent 0e8c4a1 commit 35c947e
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 16 deletions.
8 changes: 0 additions & 8 deletions thrift/compiler/generate/t_hack_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ class t_hack_generator : public t_concat_generator {
shapes_allow_unknown_fields_ =
option_is_specified(options, "shapes_allow_unknown_fields");
array_migration_ = option_is_specified(options, "array_migration");
arrays_ = option_is_specified(options, "arrays");
legacy_arrays_ = option_is_specified(options, "legacy_arrays");
hack_collections_ = option_is_specified(options, "hack_collections");

Expand Down Expand Up @@ -1185,11 +1184,6 @@ class t_hack_generator : public t_concat_generator {
*/
bool array_migration_;

/**
* True to use Hack arrays instead of collections
*/
bool arrays_;

/**
* True to never use hack collection objects. Only used for migrations
*/
Expand Down Expand Up @@ -7632,8 +7626,6 @@ THRIFT_REGISTER_GENERATOR(
" shapes_allow_unknown_fields Allow unknown fields and implicit "
"subtyping for shapes \n"
" frommap_construct Generate fromMap_DEPRECATED method.\n"
" arrays Use Hack arrays for maps/lists/sets instead of "
"objects.\n"
" hack_collections Generate hack collections instead of hack arrays.\n"
" const_collections Use ConstCollection objects rather than their "
"mutable counterparts.\n"
Expand Down
2 changes: 1 addition & 1 deletion thrift/compiler/test/fixtures/array_shape_arraykey/cmd
Original file line number Diff line number Diff line change
@@ -1 +1 @@
hack: hack:arrays=1,shapes=1,shape_arraykeys=1 src/module.thrift
hack: hack:shapes=1,shape_arraykeys=1 src/module.thrift
2 changes: 1 addition & 1 deletion thrift/compiler/test/fixtures/hack-arrays/cmd
Original file line number Diff line number Diff line change
@@ -1 +1 @@
hack: hack:arrays=1 src/module.thrift
hack: hack src/module.thrift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
hack: hack:json,arrays,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/module.thrift
hack_include: hack:json,arrays,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/include.thrift
hack_annotations: hack:json,arrays,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/annotations.thrift
hack_constants: hack:json,arrays,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/constants.thrift
hack: hack:json,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/module.thrift
hack_include: hack:json,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/include.thrift
hack_annotations: hack:json,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/annotations.thrift
hack_constants: hack:json,shapes,shape_arraykeys,frommap_construct,typedef,server,shapes_use_pipe_structure src/constants.thrift
2 changes: 1 addition & 1 deletion thrift/compiler/test/fixtures/lazy_constants/cmd
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
hack: hack:lazy_constants=1,arrays=1 src/module.thrift
hack: hack:lazy_constants=1 src/module.thrift
java: mstch_java src/module.thrift
2 changes: 1 addition & 1 deletion thrift/compiler/test/fixtures/shape_hack_arrays/cmd
Original file line number Diff line number Diff line change
@@ -1 +1 @@
hack: hack:shapes=1,shapes_allow_unknown_fields=1,arrays=1,shapes_use_pipe_structure=1 src/module.thrift
hack: hack:shapes=1,shapes_allow_unknown_fields=1,shapes_use_pipe_structure=1 src/module.thrift

0 comments on commit 35c947e

Please sign in to comment.