[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

I'm holding off on reviewing the code until we figure out what the rules are.



================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst:10
+The check conservatively tries to preserve logical costness in favor of
+physical costness. The only operations on ``this`` that this check considers
+to preserve logical constness are
----------------
Sorry, it is unclear to me what it means: "the check [...] tries to do X in favor of Y"

Also unclear what logical/physical constness mean.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst:17
+* returning const-qualified ``this``
+* passing const-qualified ``this`` as a parameter.
+
----------------
These rules need a justification; if not for the users, but for future maintainers.

For example, why isn't reading a private member variable OK? Why isn't calling a private member function OK?



================
Comment at: clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp:311
+  // This member function must stay non-const, even though
+  // it only calls other private const member functions.
+  int &get() {
----------------
Is it because the const version already exists? Then that should be the rule (not calling a private helper function).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68074/new/

https://reviews.llvm.org/D68074