Using bools instead of integers better conveys the expected value of the option.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst | ||
---|---|---|
59–60 | 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. | |
129–130 | "An integer true value" is a bit of a strange beast. How about "The value true specifies that the target environment..."? | |
clang-tools-extra/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst | ||
50 | Drive-by since we're changing this line already: If option is set -> If the option is set | |
clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst | ||
185–186 | Drive-by since we're already changing the line: If option -> If the option |
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst | ||
---|---|---|
59–60 | Having looked throughout the NotNullTerminatedResultCheck header/impl files, I can't find any reference to AreSafeFunctionsAvailable. 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: 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. |
LGTM, thank you for this cleanup!
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst | ||
---|---|---|
59–60 |
I think so, but that can be done in an NFC followup if you'd like.
Oh, that's a good reason to support those spellings, thank you for clarifying. |
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst | ||
---|---|---|
59–60 | 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 |
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst | ||
---|---|---|
59–60 | No worries, I just wanted to make sure that was your initial intention :) |
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.