aaron.ballman added a comment. The functionality is looking good, aside from a few small nits remaining. However, I'm wondering how this should integrate with other const-correctness efforts like `readability-non-const-parameter`? Also, I'm wondering how this check performs over a large code base like LLVM -- how chatty are the diagnostics, and how bad is the false positive rate (roughly)?
================ Comment at: clang-tidy/cppcoreguidelines/ConstCheck.cpp:32 + * + * Handle = either a pointer or reference + * Value = everything else (Type variable_name;) ---------------- JonasToth wrote: > aaron.ballman wrote: > > Do you intend to support Obj-C object pointers as well? > For now not, because I have no experience nor knowledge with Obj-C. Okay, then please add a comment mentioning that they're explicitly not handled yet (perhaps with a FIXME). ================ Comment at: clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:147 + // Example: `int i = 10`, `int i` (will be used if program is correct) + const auto LocalValDecl = varDecl(unless(anyOf( + isLocal(), hasInitializer(anything()), unless(ConstType), ---------------- JonasToth wrote: > @aaron.ballman The change was not valid for some reason. I leave it like it > is if thats ok with you. That's.... really odd. I am fine leaving it as-is for this patch, but it would be good to understand why that code fails as it seems like a reasonable exposition. ================ Comment at: clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:183 + // TODO Implement automatic code transformation to add the 'const'. + diag(Variable->getLocStart(), "variable %0 of type %1 can be declared const") + << Variable << Variable->getType(); ---------------- Still missing the single quotes around `const` in the diagnostic. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
