C++ accessor (and similar) member functions that return a value computed from one or more private data members without modifying the owning object can and should be declared const. Doing so not only improves the const-correctness of code that relies on objects of the type, it also aids its analyzability. It's easy (and not uncommon) to forget to declare accessors const. GCC could help detect the missing const by issuing a warning on such accessors, similarly to how it helps detect candidates for attribute const and pure. This is an enhancement to request to add such a warning. The test case below illustrates where it would be issued and shows the similar -Wsuggest-attribute warning that the new one could be modeled on. $ cat t.C && gcc -O2 -S -Wall -Wextra -Wsuggest-attribute=pure t.C class Int { public: int get () { return val; } // suggested warning: function can be declared const private: int val; }; int get_int (Int &i) { return i.get (); } t.C: In function ‘int get_int(Int&)’: t.C:10:5: warning: function might be candidate for attribute ‘pure’ [-Wsuggest-attribute=pure] int get_int (Int &i) ^~~~~~~
Good idea.
Also useful would be to warn for members that don't access any state at all: struct indirect_cmp { bool operator()(const X* l, const X* r) { return *l < *r; } }; This comparison object should have a const-qualified member function to be usable with associative containers such as std::set (see PR 83102 for example).
There is a warning like that in the middle-end: -Wsuggest-attribute=pure. Unfortunately, it's only good for functions that are actually emitted (i.e., not for C++ inline functions). $ cat t.C && gcc -O2 -S -Wall -Wsuggest-attribute=pure t.C typedef int X; struct indirect_cmp { bool operator()(const X* l, const X* r); }; bool indirect_cmp::operator()(const X* l, const X* r) { return *l < *r; } t.C: In member function ‘bool indirect_cmp::operator()(const X*, const X*)’: t.C:7:6: warning: function might be candidate for attribute ‘pure’ [-Wsuggest-attribute=pure] bool indirect_cmp::operator()(const X* l, const X* r) { return *l < *r; } ^~~~~~~~~~~~
*** Bug 94997 has been marked as a duplicate of this bug. ***
(In reply to Martin Sebor from comment #3) > There is a warning like that in the middle-end: -Wsuggest-attribute=pure. Whether a function is pure is a slightly different thing to whether it is a C++ const member function. I did a test build of current gcc with the warning flag and libgomp has Werror set, so that prevents a full build. However, for the partial build I did, the warning was produced about 160 times. Is it worth mentioning that pure functions are a GNU only extension but const member functions are full ISO C++ ?
You're right, there is a substantial difference between attributes const/pure and constness in the C/C++ sense. A warning that detects missing const on member functions (i.e., this request) is implementable in the C++ front end, while one that also considers accesses to the state of the object would have to be implemented in the middle end (like -Wsuggest-attribute). Only there is it possible to tell if a pointer dereference accesses *this. As an aside, a risk with enabling -Wsuggest-attribute= is that GCC doesn't detect accesses that are invalid in pure functions (like to globals), or any other misuses in functions declared with any of the suggested attributes. Since what is and isn't safe isn't always clear to everyone, it's easy to get it wrong. Once the detection is implemented, hopefully for GCC 11 (pr18487), using attributes pure and const will be a lot safer. But that's independent of improving const-correctness.
(In reply to Jonathan Wakely from comment #2) > Also useful would be to warn for members that don't access any state at all: > > struct indirect_cmp { > bool operator()(const X* l, const X* r) { return *l < *r; } > }; > > This comparison object should have a const-qualified member function to be > usable with associative containers such as std::set (see PR 83102 for > example). If I extend this to modify global state it's not pure, but should still be const-qualified: struct indirect_cmp { static int counter; bool operator()(const X* l, const X* r) { ++counter; return *l < *r; } }; int indirect_cmp::counter = 0; So the pure attribute isn't the right property.
(In reply to Jonathan Wakely from comment #7) > struct indirect_cmp { > static int counter; > bool operator()(const X* l, const X* r) { > ++counter; > return *l < *r; > } > }; > > int indirect_cmp::counter = 0; > > So the pure attribute isn't the right property. I tried your code on cppcheck and much to my surprise it found the problem: $ /home/dcb/cppcheck/trunk//cppcheck --enable=all --inconclusive may9a.cc may9a.cc:4:8: style:inconclusive: Technically the member function 'indirect_cmp::operator()' can be const. [functionConst] bool operator()(const X* l, const X* r) { ^ $ My working assumption had been that cppcheck was looking for C++ member functions that consist of a return statement only but clearly it is doing more than that. If there is any interest, I could probably figure out the pattern of C++ code it is looking for. My opinion is that a first approximation at implementation in gcc would merely look for C++ member functions that are return statements only. More fancy things could be done later in a second version.
(In reply to David Binderman from comment #8) > My opinion is that a first approximation at implementation in gcc would > merely look for C++ member functions that are return statements only. > More fancy things could be done later in a second version. No, that won't work: struct Iota { int i = 0; int operator()() { return i++; } };
(In reply to Jonathan Wakely from comment #9) > (In reply to David Binderman from comment #8) > > My opinion is that a first approximation at implementation in gcc would > > merely look for C++ member functions that are return statements only. > > More fancy things could be done later in a second version. > > No, that won't work: Of course. Some false positives are to be expected with a first approximation. Perhaps I've been slightly less than clear. A first approximation isn't the final solution. It should be a step towards a final solution.
I would expect no false positives for a warning like this.
A first approximation that is implemented in the wrong part of the compiler, using the wrong logic, giving the wrong answers, is not a step in the right direction because it would need to be completely redone. Might as well just start the right version and skip that first approximation.
(In reply to Jonathan Wakely from comment #12) > Might as well just start the right version and skip that first approximation. It sounds to me like you are somewhat keen to implement. Feel free to go right ahead. I'd be glad to help with some testing of the new feature. Do you plan a test driven development, perhaps ?