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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
