[PATCH] D112356: [NFC] Tidy up spelling, grammar, and inconsistencies in clang-tidy documentation
Arthur O'Dwyer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 23 07:22:03 PDT 2021
Quuxplusone added a comment.
Definitely an improvement! I looked at all the changed places and found some more improvements you can make. I don't need to see this again, though.
The only disimprovement I found was "Jaro–Winkler" becoming "Jaro-Winkler".
================
Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:444-445
``run-clang-tidy.py clang-tidy/.*Check\.cpp`` will only analyze clang-tidy
- checks. It may also be necessary to restrict the header files warnings are
- displayed from using the ``-header-filter`` flag. It has the same behavior
+ checks. It may also be necessary to restrict the header files that warnings
+ are displayed from using the ``-header-filter`` flag. It has the same behavior
as the corresponding :program:`clang-tidy` flag.
----------------
Naïve grammar fix:
```
It may also be necessary to restrict the header files from which warnings are displayed using the ``-header-filter`` flag.
```
But, better, active-voice fix:
```
You can also use the ``-header-filter`` flag to silence warnings from certain header files.
```
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/abseil-no-internal-dependencies.rst:9
+it because it's an implementation detail. They cannot friend it, include it,
you mention it or refer to it in any way. Doing so violates Abseil's
compatibility guidelines and may result in breakage. See
----------------
What is "you mention it"? Should this just say "mention it"? but even then, I don't know the technical definition of "mention" in this context.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:6-7
-This checks ensures that pipe2() is called with the O_CLOEXEC flag. The check also
+This check ensures that pipe2() is called with the O_CLOEXEC flag. The check also
adds the O_CLOEXEC flag that marks the file descriptor to be closed in child processes.
Without this flag a sensitive file descriptor can be leaked to a child process,
----------------
Should `pipe2()` and `O_CLOEXEC` be double-backticked here? I think so.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/boost-use-to-string.rst:10
It doesn't replace conversion from floating points despite the ``to_string``
+overloads, because it would change the behavior.
----------------
`s/floating points/floating-point types/`
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:122
By default, the following parameter names, and their Uppercase-initial
variants are ignored:
`""` (unnamed parameters), `iterator`, `begin`, `end`, `first`, `last`,
----------------
`s/variants/variants,/`
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:157
parameter of that function, of the same overload.
- E.g. ``f(a, 1)`` and ``f(b, 2)`` to some ``f(T, int)``.
+ e.g. ``f(a, 1)`` and ``f(b, 2)`` to some ``f(T, int)``.
----------------
This letter is at the beginning of a sentence, so `E.g.` is correct. Arguably. It //is// a sentence fragment. ;)
Perhaps:
```
The heuristics suppress the easily-swappable-parameters warning for pairs of parameters that:
* Are used together in the same expression, such as ``f(a, b)`` or ``a < b``.
* Are passed in the same argument position to the same function, such as ``f(a, 1)`` and ``f(b, 1)``, as long as both of those expressions call the same overload ``f(SomeT, int)``.
```
This section definitely needs better examples.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-implicit-widening-of-multiplication-result.rst:11
-This is mainly useful when operating on a very large buffers.
+This is mainly useful when operating on very large buffers.
For example, consider:
----------------
`This is useful mainly when`
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:64
-- If the new function is could be safe version and C++ files are analysed and
+- If the new function is could be safe version and C++ files are analyzed and
the destination array is plain ``char``/``wchar_t`` without ``un/signed`` then
----------------
Here and line 68: "is could be safe" doesn't make sense, but I'm not sure what is meant. My guess is that the writer is trying to refer to a set of functions, the "could-be-safe functions," like:
```
If we're in C++, and the new function is a could-be-safe function, and the destination array is...
```
but I haven't found whether the notion of "could-be-safe" functions is actually defined anywhere.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-string-compare.rst:17
-Checks that compare function results (i,e, ``strcmp``) are compared to valid
+Checks that compare function results (i.e., ``strcmp``) are compared to valid
constant. The resulting value is
----------------
```
Checks that the results of comparison functions (e.g. ``strcmp``) are...
```
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst:28-29
-This algorithm works for small amount of objects, but will lead to freeze for a
+This algorithm works for a small amount of objects, but will lead to freeze for
a larger user input.
----------------
```
The ``doSomething`` function works for ``items.size() < 32767``, but has undefined behavior for larger vectors.
```
Also, `const std::vector&` needs to be `const std::vector<int>&`; CTAD isn't allowed on function parameters.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/concurrency-mt-unsafe.rst:15
would be clobbered by subsequent (non-parallel, but concurrent) calls to
-a related function. E.g. the following code suffers from unprotected
+a related function. e.g. the following code suffers from unprotected
accesses to a global state:
----------------
Sentence-initial letter should be capitalized. Again, sentence fragment.
```
Note that using some thread-unsafe functions may be still valid in
concurrent programming if only a single thread is used; for example, ``setenv(3)``.
However, some functions may track global state which
would be clobbered by subsequent (non-parallel, but concurrent) calls to
a related function. For example, the following code suffers from unprotected
accesses to global state:
```
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/hicpp-signed-bitwise.rst:7
Finds uses of bitwise operations on signed integer types, which may lead to
-undefined or implementation defined behaviour.
+undefined or implementation defined behavior.
----------------
`implementation-defined`
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst:19
+ idiomatic.
* Catching character pointers (``char``, ``wchar_t``, unicode character types)
will not be flagged to allow catching sting literals.
----------------
`Unicode`
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:139
Specify the function used to reverse an iterator pair, the function should
accept a class with ``rbegin`` and ``rend`` methods and return a
----------------
`pair. The function`
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-loop-convert.rst:142
+ class with ``begin`` and ``end`` methods that call the ``rbegin`` and
``rend`` methods respectively. Common examples are ``ranges::reverse_view``
and ``llvm::reverse``.
----------------
`s/ranges::reverse_view/std::views::reverse/`
C++20 `ranges::reverse_view` is not a function, and won't (quite) do the right thing (compared to `views::reverse`) if you pass it something that's already reversed.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-replace-disallow-copy-and-assign-macro.rst:17
When running this check on a code like this:
----------------
`s/a code/code/`
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/modernize-use-auto.rst:119
+``new`` expression. In this case, the declaration type can be replaced with
``auto`` improving readability and maintainability.
----------------
`s/improving/to improve/` here and line 152
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst:21
- // ``for`` directive can not have ``default`` clause, no diagnostics.
+ // ``for`` directive cannot have ``default`` clause, no diagnostics.
void n0(const int a) {
----------------
`clause; no diagnostic.` ?
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-magic-numbers.rst:12-13
- * `Rule ES.45: Avoid “magic constants”; use symbolic constants in C++ Core Guidelines <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-magic>`_
+ * `Rule ES.45: Avoid "magic constants"; use symbolic constants in C++ Core Guidelines <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-magic>`_
* `Rule 5.1.1 Use symbolic names instead of literal values in code in High Integrity C++ <http://www.codingstandard.com/rule/5-1-1-use-symbolic-names-instead-of-literal-values-in-code/>`_
* Item 17 in "C++ Coding Standards: 101 Rules, Guidelines and Best
----------------
```
* `C++ Core Guideline ES.45: Avoid "magic constants"; use symbolic constants <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-magic>`_
* `High Integrity C++ Rule 5.1.1: Use symbolic names instead of literal values in code <http://www.codingstandard.com/rule/5-1-1-use-symbolic-names-instead-of-literal-values-in-code/>`_
```
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-suspicious-call-argument.rst:115
-The `Jaro–Winkler distance <http://en.wikipedia.org/wiki/Jaro–Winkler_distance>`_
+The `Jaro-Winkler distance <http://en.wikipedia.org/wiki/Jaro–Winkler_distance>`_
is an edit distance like the Levenshtein distance.
----------------
Here, "Jaro–Winkler" is correct and "Jaro-Winkler" is wrong.
Ditto "Sørensen–Dice" below.
If there's a markdown equivalent, that still renders an en-dash –, and you want to use that instead, OK. But just changing the punctuation to an ASCII hyphen-dash isn't right. (I notice you didn't change `Sørensen` to `Sorensen`, so the goal definitely wasn't a pure ASCII-fication of these files.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112356/new/
https://reviews.llvm.org/D112356
More information about the cfe-commits
mailing list