From 58792dcebd27119f8626f665cd7220e35927e145 Mon Sep 17 00:00:00 2001 From: Satish Kumar Date: Thu, 9 Jan 2025 07:00:07 -0800 Subject: [PATCH] Delete arrays as a compiler option. 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 --- .../src/thrift/compiler/generate/t_hack_generator.cc | 8 -------- .../compiler/test/fixtures/array_shape_arraykey/cmd | 2 +- .../src/thrift/compiler/test/fixtures/hack-arrays/cmd | 2 +- .../test/fixtures/hack_field_wrapper_with_arrays/cmd | 8 ++++---- .../src/thrift/compiler/test/fixtures/lazy_constants/cmd | 2 +- .../thrift/compiler/test/fixtures/shape_hack_arrays/cmd | 2 +- 6 files changed, 8 insertions(+), 16 deletions(-) diff --git a/third-party/thrift/src/thrift/compiler/generate/t_hack_generator.cc b/third-party/thrift/src/thrift/compiler/generate/t_hack_generator.cc index bcfe6e67bcb178..aea942f8acc383 100644 --- a/third-party/thrift/src/thrift/compiler/generate/t_hack_generator.cc +++ b/third-party/thrift/src/thrift/compiler/generate/t_hack_generator.cc @@ -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"); @@ -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 */ @@ -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" diff --git a/third-party/thrift/src/thrift/compiler/test/fixtures/array_shape_arraykey/cmd b/third-party/thrift/src/thrift/compiler/test/fixtures/array_shape_arraykey/cmd index e0fa22a6007771..6717efb1123703 100644 --- a/third-party/thrift/src/thrift/compiler/test/fixtures/array_shape_arraykey/cmd +++ b/third-party/thrift/src/thrift/compiler/test/fixtures/array_shape_arraykey/cmd @@ -1 +1 @@ -hack: hack:arrays=1,shapes=1,shape_arraykeys=1 src/module.thrift +hack: hack:shapes=1,shape_arraykeys=1 src/module.thrift diff --git a/third-party/thrift/src/thrift/compiler/test/fixtures/hack-arrays/cmd b/third-party/thrift/src/thrift/compiler/test/fixtures/hack-arrays/cmd index be298251507167..90aeeecbb9728e 100644 --- a/third-party/thrift/src/thrift/compiler/test/fixtures/hack-arrays/cmd +++ b/third-party/thrift/src/thrift/compiler/test/fixtures/hack-arrays/cmd @@ -1 +1 @@ -hack: hack:arrays=1 src/module.thrift +hack: hack src/module.thrift diff --git a/third-party/thrift/src/thrift/compiler/test/fixtures/hack_field_wrapper_with_arrays/cmd b/third-party/thrift/src/thrift/compiler/test/fixtures/hack_field_wrapper_with_arrays/cmd index 80cd14fdd62f40..fccce128489c54 100644 --- a/third-party/thrift/src/thrift/compiler/test/fixtures/hack_field_wrapper_with_arrays/cmd +++ b/third-party/thrift/src/thrift/compiler/test/fixtures/hack_field_wrapper_with_arrays/cmd @@ -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 diff --git a/third-party/thrift/src/thrift/compiler/test/fixtures/lazy_constants/cmd b/third-party/thrift/src/thrift/compiler/test/fixtures/lazy_constants/cmd index 8bac4bf2fdf910..90bae6cfd67dad 100644 --- a/third-party/thrift/src/thrift/compiler/test/fixtures/lazy_constants/cmd +++ b/third-party/thrift/src/thrift/compiler/test/fixtures/lazy_constants/cmd @@ -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 diff --git a/third-party/thrift/src/thrift/compiler/test/fixtures/shape_hack_arrays/cmd b/third-party/thrift/src/thrift/compiler/test/fixtures/shape_hack_arrays/cmd index a2e759688f6a41..884e5f082bccc5 100644 --- a/third-party/thrift/src/thrift/compiler/test/fixtures/shape_hack_arrays/cmd +++ b/third-party/thrift/src/thrift/compiler/test/fixtures/shape_hack_arrays/cmd @@ -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