JonasToth added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:20
+namespace {
+AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); }
+} // namespace
----------------
This matcher already exists either as global matcher or in some other check.
please refactor it out.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:39
+ << MatchedDecl;
+ diag(MatchedDecl->getLocation(), "insert 'const '", DiagnosticIDs::Note)
+ << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "const ");
----------------
`const` fixing is a complicated issue. I would rather not do this now, as there
is currently a patch in flight that does the general case.
If you really want it, there is a util for that in `clang-tidy/util/` that is
able to do some const-fixing.
================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:97
+- New :doc:`cppcoreguidelines-avoid-non-const-global-variables
+ <clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables>`
check.
----------------
Please describe the check with one sentence and sync that with the check
documentation.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst:6
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
----------------
Please adjust the documentation here as well :)
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp:3
+
+int nonConstVariable = 0;
+// CHECK-MESSAGES: warning: variable 'nonConstVariable' is non-const and
global [cppcoreguidelines-avoid-non-const-global-variables]
----------------
Please add test-cases for pointers and references of builtins (`int` and the
like) and classes/structs/enums/unions, function pointers and member
function/data pointers.
We need to consider template variables as well (i think a c++14 feature).
I am not sure about the `consteval` support in clang for c++20, but for each
feature from a newer standard its better to have a additional test-file that
will use this standard.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70265/new/
https://reviews.llvm.org/D70265
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
