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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits