<div dir="ltr">Hi,<br><br>I'm seeking some opinions/advice on a clang-tidy check that I've been working on. It seemed to me to be a no-brainer check to add, but the more I work on it, the more questions arise, and I want to get a feel for what people think about whether it belongs in clang-tidy, and in what capacity, and how it should work.<br><br>The check basically flags code such as this:<br>struct A {<br>    std::string getS() const { return s; }<br>private:<br>    std::string s;<br>};<br>where `getS` is used as an accessor for the member `s`, but, because of its by-value return type, expensively copies the member on each access. A sensible and most often compatible change is to adjust `getS` so that it returns `const std::string&`. Code such as:<br>    const auto str = myA.getS();<br>continues to work identically as before, but code such as:<br>    const auto size = myA.getS().size();<br>saves a copy-construction of a std::string.<br><br>I have the basics of this working, but as I think about it more, a few concerns arise:<br><br>1) this breaks code that calls non-const member functions on the result of getS(), e.g. `myA.getS() += 'x';`. This code may be smelly, but it is legal. What's the stance of clang-tidy on issues like this? Are checks that potentially break call sites for the sake of performance welcome or unwelcome?<br><br>2) it becomes more difficult as you stretch the definition of what an 'accessor' is:<br>struct A {<br>    std::string getS(bool a, bool b) const { return a ? x : b ? y : z; }<br>private:<br>    std::string x, y, z;<br>};<br>The definition of `getS` can be arbitrarily complex, but as long as it always ends up returning a member, the check could kick in. In fact, why not just expand the check to apply to any function that returns a non-local? That seems perhaps too aggressive, but where is the line?<br><br>3) Why stop at `const&`? The struct `A` above could maybe benefit from another function overloaded on &&, that returns a std::string&& by std::move()ing `s`. That may create an undesired moved-from state for A, but it seems no less arbitrary than point (1) above. What is clang-tidy's general stance on being opinionated like this?<br><br>There might be more, but that's enough for now. Interested in any feedback anyone has, and interested in any info about how such design decisions have been addressed by clang-tidy in the past.<br><br>Thanks,<br>Logan R. Smith<br></div>