Skip to content

Commit

Permalink
Add warnings related to custom default values
Browse files Browse the repository at this point in the history
Summary:
# Changes Breakdown
1. Changes `check_initializer()` (in `check_initializer.cc`) to return a boolean indicating whether any error was detected.
2. Use newly obtained boolean value to skip further validation of custom default values (in `standard_validat.cc::validate_field_default_value()`) if the initializer is not valid to begin with
3. If the initializer is valid, issue warnings if:
   1. the custom default value is the same as the intrinsic default (i.e., specifying a default value explicitly is therefore redundant).
   2. the field for which an explicit default value is specified is `optional` or `terse`.
      * In the `optional` case, a custom default value is nonsensical: it is simply ignored.
      * In the `terse` case, it is undesirable because it makes the logic/optimization of terse fields much more complex than it needs to be, and in practice we've noticed that there is only a very small number of use cases of custom defaults with terse fields. Removing them would allow us to significantly improve the logic and code (and better formalize terse fields in the Thrift Object Model - and allow easier general rollout).

Reviewed By: praihan, iahs

Differential Revision: D68197595

fbshipit-source-id: 2db9e68620ed783164fcc0fd6880d5d085020b9f
  • Loading branch information
Aristidis Papaioannou authored and facebook-github-bot committed Jan 17, 2025
1 parent 7b151ee commit c8ae7a9
Show file tree
Hide file tree
Showing 4 changed files with 296 additions and 59 deletions.
Loading

0 comments on commit c8ae7a9

Please sign in to comment.