aaron.ballman added inline comments.
================ Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:107 + diag(Def->getInnerLocStart(), "return type %0 is 'const'-qualified " + "hindering compiler optimizations") + << Def->getReturnType(); ---------------- ymandel wrote: > aaron.ballman wrote: > > It seems strange to me that this is in the readability module but the > > diagnostic here is about compiler optimizations. Should this be in the > > performance module instead? Or should this diagnostic be reworded about the > > readability concerns? > Good point. The readability angle is that the const clutters the code to no > benefit. That is, even if it was performance neutral, I'd still want to > discourage this practice. But, it does seem like the primary impact is > performance. > > I'm fine either way -- moving it to performance or emphasizing the clutter of > the unhelpful "const". I'm inclined to moving it, but don't have good sense > of how strict these hierarchies are. What do you think? I'm not sold that `const`-qualified return types always pessimize optimizations. However, I'm also not sold that it's *always* a bad idea to have a top-level cv-qualifier on a return type either (for instance, this is one way to prevent callers from modifying a temp returned by a function). How about leaving this in readability and changing the diagnostic into: "top-level 'const' qualifier on a return type may reduce code readability with limited benefit" or something equally weasely? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53025 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
