aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:55-71 + if (const auto *Reference = + Result.Nodes.getNodeAs<VarDecl>("reference_to_non-const")) { + diag(Reference->getLocation(), + "variable %0 provides global access to non-const type, consider " + "making the referenced data const") + << Reference; // FIXME: Add fix-it hint to Reference + return; ---------------- I think these cases should be combined to avoid duplication. ``` if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("whatever")) { diag(VD->getLocation(), "variable %0 provides global access to a non-const object; considering making the %select{referenced|pointed-to}1 data 'const'") << VD << VD->getType()->isReferenceType(); return; } ``` the matcher needs to be changed to bind to the same id for both cases. Note, this rewords the diagnostic slightly to avoid type/object confusion (the variable provides access to an object of a non-const type, not to the type itself). ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst:20 + + char * c_ptr1 = &c; // Warns! + char *const c_const_ptr = &c; // Warns! ---------------- This isn't legal code. ================ Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst:33 + char h = 0; + } + ---------------- This isn't legal code either. You may want to run the example through the compiler to catch these sort of things. 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