JonasToth marked an inline comment as done.
JonasToth added a comment.
addressed all outstanding review comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:107-108
+ int *np_local3[2] = {&np_local0[0], &np_local0[1]};
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'np_local3' of type 'int
*[2]' can be declared 'const'
+ // CHECK-FIXES: const
+ for (int *non_const_ptr : np_local3) {
----------------
aaron.ballman wrote:
> This is a false positive. Making `np_local3` be `const` will break the code:
> https://godbolt.org/z/EExKfsW4G
Thanks for spotting this!
The analyzer did not consider the `int * non_const_ptr: np_local3` to be a
mutation, as its not a reference type.
```
444 // The range variable is a reference to a builtin array. In that case
the
1 // array is considered **modified if the loop-variable is a non-const
reference**.
2 const auto DeclStmtToNonRefToArray =
declStmt(hasSingleDecl(varDecl(hasType(
3
hasUnqualifiedDesugaredType(referenceType(pointee(arrayType())))))));
4 const auto RefToArrayRefToElements = match(
5 findAll(stmt(cxxForRangeStmt(
6
hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
7 .bind(NodeID<Decl>::value)),
8 hasRangeStmt(DeclStmtToNonRefToArray),
9 hasRangeInit(canResolveToExpr(equalsNode(Exp)))))
10 .bind("stmt")),
11 Stm, Context);
```
Added the fix to the patch, is that ok with you?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:17
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
----------------
aaron.ballman wrote:
> Similarly, showing anonymous namespace variables would help, as those are
> like a static global and can also theoretically known to be const. (Note, I
> don't insist on catching those cases! Just would like test coverage with some
> comments on what to expect + docs if it seems important to tell users.)
Globals are not matched in any way. But there is
`cppcoreguidelines-avoid-non-const-global-variables` which warns for non-const
globals already.
The docs say `This check implements detection of local variables which could be
declared as`, so its implicit that globals are not caught. I think that its
enough. I will add a reference to
`cppcoreguidelines-avoid-non-const-global-variables` though.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54943/new/
https://reviews.llvm.org/D54943
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
