[PATCH] D23918: [clang-tidy docs] Add missing option docs.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 30 13:15:23 PDT 2016
aaron.ballman requested changes to this revision.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.
This revision now requires changes to proceed.
Thank you for working on this!
================
Comment at: docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst:24
@@ +23,3 @@
+
+ A string represents which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
----------------
s/represents/specifying
================
Comment at: docs/clang-tidy/checks/google-build-namespaces.rst:20
@@ +19,3 @@
+ A comma-separated list of filename extensions of header files (the filename
+ extensions should not contain "." prefix). "h,hh,hpp,hxx" by default. For
+ extension-less header files, using an empty string or leaving an empty string
----------------
s/not contain/not include the
================
Comment at: docs/clang-tidy/checks/google-build-namespaces.rst:21
@@ +20,3 @@
+ extensions should not contain "." prefix). "h,hh,hpp,hxx" by default. For
+ extension-less header files, using an empty string or leaving an empty string
+ between "," if there are other filename extensions.
----------------
How about:
```
For header files without an extension, use an empty string (if there are no other desired extensions) or leave an empty element in the list. e.g., "h,hh,hpp,hxx," (note the trailing comma).
```
(assuming my example actually works.)
================
Comment at: docs/clang-tidy/checks/google-runtime-int.rst:19
@@ +18,3 @@
+
+ A string represents unsigned type prefix. Default is `uint`.
+
----------------
A string specifying the unsigned type prefix.
================
Comment at: docs/clang-tidy/checks/google-runtime-int.rst:23
@@ +22,3 @@
+
+ A string represents singed type prefix. Default is `int`.
+
----------------
A string specifying the signed type prefix.
================
Comment at: docs/clang-tidy/checks/google-runtime-int.rst:27
@@ +26,2 @@
+
+ A string represents type suffix. Default is an empty string.
----------------
A string specifying the type suffix.
================
Comment at: docs/clang-tidy/checks/llvm-namespace-comment.rst:20
@@ +19,3 @@
+
+ An unsigned integer represents the maximal number of lines of the namespace
+ that is not required closing commments. Default is `1U`.
----------------
How about:
```
Requires the closing brace of the namespace definition to be followed by a closing comment if the body of the namespace has more than `ShortNamespaceLines` lines of code. The value is an unsigned integer that defaults to `1U`.
```
================
Comment at: docs/clang-tidy/checks/llvm-namespace-comment.rst:25
@@ +24,2 @@
+
+ An unsigned integer represents spaces before comments. Default is `1U`.
----------------
How about:
```
An unsigned integer specifying the number of spaces before the comment closing a namespace definition.
```
================
Comment at: docs/clang-tidy/checks/misc-definitions-in-headers.rst:83
@@ +82,3 @@
+ A comma-separated list of filename extensions of header files (the filename
+ extensions should not contain "." prefix). "h,hh,hpp,hxx" by default. For
+ extension-less header files, using an empty string or leaving an empty string
----------------
Same suggestion here as above.
================
Comment at: docs/clang-tidy/checks/misc-definitions-in-headers.rst:89
@@ +88,3 @@
+
+ When non-zero, the check will use use file extension to distinguish header
+ files. Default is `1`.
----------------
s/use use/use the
================
Comment at: docs/clang-tidy/checks/misc-move-constructor-init.rst:20
@@ +19,3 @@
+
+ A string represents which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
----------------
s/represents/specifying
================
Comment at: docs/clang-tidy/checks/misc-move-constructor-init.rst:25-26
@@ +24,3 @@
+
+ When non-zero, the check is also used to implement
+ `cert-oop11-cpp <cert-oop11-cpp.html>`_. Default is `0`.
----------------
cert-oop11-cpp redirects to this file, so the link is a bit circular. Perhaps a better link is:
How about:
```
When non-zero, the check conforms to the behavior expected by the CERT secure coding recommendation `OOP11-CPP <https://www.securecoding.cert.org/confluence/display/cplusplus/OOP11-CPP.+Do+not+copy-initialize+members+or+base+classes+from+a+move+constructor>`_. Default is `0` for misc-move-constructor-init and `1` for cert-oop11-cpp
```
================
Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:142
@@ +141,3 @@
+
+ When non-zero, the check will warn on expression like ``sizeof(CONSTANT)``.
+ Default is `1`.
----------------
s/on/on an
================
Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:147
@@ +146,3 @@
+
+ When non-zero, the check will warn on expression like ``sizeof(this)``.
+ Default is `1`.
----------------
s/on/on an
================
Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:152
@@ +151,3 @@
+
+ When non-zero, the check will warn on expression like ``sizeof(epxr) <= k``
+ for a suspicious constant 'k'. Default is `1`.
----------------
s/on/on an
================
Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:153
@@ +152,2 @@
+ When non-zero, the check will warn on expression like ``sizeof(epxr) <= k``
+ for a suspicious constant 'k'. Default is `1`.
----------------
It would be good to describe what a suspicious constant actually is.
================
Comment at: docs/clang-tidy/checks/misc-string-constructor.rst:39
@@ +38,3 @@
+
+ When non-zero, the check will warn on the large length string. Default is
+ `1`.
----------------
How about:
```
.. warn on a string with a length greater than `LargeLengthThreshold`.
```
================
Comment at: docs/clang-tidy/checks/misc-string-constructor.rst:44
@@ +43,2 @@
+
+ An integer represents the large length threshold. Default is `0x8000000`.
----------------
s/represents/specifying
Also, the value is wrong, the default is `0x800000` not `0x8000000`.
================
Comment at: docs/clang-tidy/checks/misc-suspicious-missing-comma.rst:48
@@ +47,3 @@
+
+ An unsigned integer represents minimal size of a string literals array to be
+ considered by the check. Default is `5U`.
----------------
s/represents minimal/specifying the minimum
s/literals array/literal
================
Comment at: docs/clang-tidy/checks/misc-suspicious-missing-comma.rst:53
@@ +52,3 @@
+
+ A string represents maximal threshold ratio of suspicious string literals to
+ be considered. Default is `.2`.
----------------
represents->specifying
maximal->maximum
Should actually state that the string represents a floating-point value between 0 and 1 (inclusive? exclusive?). Also, is it really a string? aka, does it need quotes, or is it okay to not use quotes (as the default suggests)?
================
Comment at: docs/clang-tidy/checks/misc-suspicious-missing-comma.rst:58
@@ +57,3 @@
+
+ An unsigned integer represents maximal number of concatenated tokens.
+ Default is `5U`.
----------------
s/represents maximal/specifying the maximum
================
Comment at: docs/clang-tidy/checks/misc-suspicious-string-compare.rst:53
@@ +52,3 @@
+
+ A string represents the name of detecting string compare function. Default is
+ an empty string.
----------------
How about:
```
A string specifying the comma-separated names of string comparison functions.
```
We should also list the default string comparison functions.
================
Comment at: docs/clang-tidy/checks/modernize-pass-by-value.rst:160
@@ +159,3 @@
+
+ A string represents which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
----------------
s/represents/specifying
================
Comment at: docs/clang-tidy/checks/modernize-replace-auto-ptr.rst:78
@@ +77,3 @@
+
+ A string represents which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
----------------
s/represents/specifying
================
Comment at: docs/clang-tidy/checks/performance-for-range-copy.rst:26
@@ +25,2 @@
+
+ When non-zero, the check will warn on all auto copies. Default is `0`.
----------------
What is an "auto copy"? I think this option just warns if `auto` is used as the type specifier in a range-based for loop, so perhaps this should just say "When non-zero, warns on any use of `auto` as the type of the range-based for loop variable."
================
Comment at: docs/clang-tidy/checks/performance-unnecessary-value-param.rst:62
@@ +61,3 @@
+
+ A string represents which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
----------------
s/represents/specifying
https://reviews.llvm.org/D23918
More information about the cfe-commits
mailing list