[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

[Nathan James via Phabricator via cfe-commits]

njames93 removed a subscriber: Charusso.
njames93 added inline comments.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:60
 - If copy to the destination array can overflow [1] and
-  ``AreSafeFunctionsAvailable`` is set to ``Yes``, ``y`` or non-zero and it is
+  ``AreSafeFunctionsAvailable`` is set to `true` and it is
   possible to obtain the capacity of the destination array then the new function
----------------
Charusso wrote:
> aaron.ballman wrote:
> > njames93 wrote:
> > > aaron.ballman wrote:
> > > > This edit loses information about also accepting `Yes` and `y` -- is that intentional (or were those unsupported before)?
> > > > 
> > > > FWIW, I'd be fine dropping support for alternate spellings of `true`.
> > > Having looked throughout the NotNullTerminatedResultCheck header/impl files, I can't find any reference to `AreSafeFunctionsAvailable`.
> > > I can only guess this is meant to say WantToUseSafeFunctions. If that is the case, `Yes` and `y` were never supported spellings.
> > > 
> > > Should this be changed to use that option name instead? cc @Charusso
> > > 
> > > FWIW I intend (in the near future) to extend boolean parsing for check options to:
> > > `y|Y|yes|Yes|YES|true|True|TRUE|on|On|ON`
> > > `n|N|no|No|NO|false|False|FALSE|off|Off|OFF`.
> > > 
> > > Reason for this is we claim to use YAML for config format and according to its specification, this is what is accepted as a boolean value. Ref https://yaml.org/type/bool.html.
> > > Still need to keep the old integer method of specifying bools for backwards compatibility reasons.
> > > 
> > > Should this be changed to use that option name instead? cc @Charusso
> > 
> > I think so, but that can be done in an NFC followup if you'd like.
> > 
> > > Reason for this is we claim to use YAML for config format and according to its specification, this is what is accepted as a boolean value.
> > 
> > Oh, that's a good reason to support those spellings, thank you for clarifying.
> Hey, nice catch and cool patch! Sorry for the extra work. `WantToUseSafeFunctions` is wanted to be here as a zero or non-zero value [1]. It would be great if you could rewrite this variable name because I do not write LLVM code any more. Thanks!
> 
> [1] https://clang.llvm.org/extra/clang-tidy/checks/bugprone-not-null-terminated-result.html#options
No worries, I just wanted to make sure that was your initial intention :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92652/new/

https://reviews.llvm.org/D92652