[PATCH] D126247: `readability-indentifier-naming` resolution order and examples

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2742
     double   dValueDouble = 0.0;
     ULONG    ulValueUlong = 0;
----------------
Phabricator says there is no context available.  Did you generate this diff with `git diff -U999999 main`?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2750
+The full set of identifier classifications listed above includes some overlap,
+where an individual identifier can falling into several classifications. Bellow
+is a list of the classifications supported by ``readability-identifier-naming``
----------------
`fall`, not `falling`


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2750
+The full set of identifier classifications listed above includes some overlap,
+where an individual identifier can falling into several classifications. Bellow
+is a list of the classifications supported by ``readability-identifier-naming``
----------------
LegalizeAdulthood wrote:
> `fall`, not `falling`
`Below`, not `Bellow`


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2756
+matched. This occurs when the semantics of the identifier match and there is a
+valid option for the classification present in the ``.clang-tidy`` file - e.g.,
+``readability-identifier-naming.AbstractClassCase``.
----------------
Does it have to be in the `.clang-tidy` file, or is it just a matter of setting options?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2772
+   - Class ``[class, struct]``
+   - Struct ``[class]``
+   - Union ``[union]``
----------------
Why is `Struct`  listed twice?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2786
+ - <parameters>
+   - ConstexprVariable ``[constexpr]``
+   - ``[const]``
----------------
I would have expected `ConstExprVariable` instead?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2857
+attempt disambiguation within the context of this document. For example,
+``<member-variables>`` identifiers that clang tidy is currently looking only at
+member variables.
----------------
Eugene.Zelenko wrote:
> Ditto.
`identifies`, not `identifiers`


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2873
+classify the identifier in any other context. For example, in the 
+``<paramaters>`` semantic context, clang tidy will abort matching after failing
+to resolve the ``Parameter`` classification and a parameter will *not* be
----------------
`parameters` not `paramaters`


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2873
+classify the identifier in any other context. For example, in the 
+``<paramaters>`` semantic context, clang tidy will abort matching after failing
+to resolve the ``Parameter`` classification and a parameter will *not* be
----------------
LegalizeAdulthood wrote:
> `parameters` not `paramaters`
Clang-tidy, not clang tidy


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2880
+
+The code snippet below[1]_ serves as an exhaustive example of various
+identifiers the ``readability-identifier-naming`` check is able to classify.
----------------
Should we be linking to ephemeral gists that can disappear?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:3054
+    // @param param Parameter
+    int func(std::string *const str_ptr, const std::string &str, int *ptr_param, int param)
+    {
----------------
This covers "const pointer", but what about "pointer to const"?
e.g. `std::string const *str_ptr` how is this handled?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:3120
+    // GlobalConstantPointer, GlobalConstant, Constant, GlobalPointer, GlobalVariable, Variable
+    int *const global_const_ptr = nullptr;
+
----------------
Same, pointer to `const`?


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:3127-3129
+    // StaticConstant does not actually trip for this declaration despite the documentation indicating that it should.
+    // StaticConstant does not appear to trip for anything. Reading the code, it seems that StaticConstant logic is in the
+    // wrong place and the conditions cannot be met.
----------------
Is this really a bug report disguised as a doc comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126247