aaron.ballman added a comment. In https://reviews.llvm.org/D45702#1086802, @shuaiwang wrote:
> In https://reviews.llvm.org/D45702#1086250, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D45702#1085890, @shuaiwang wrote: > > > > > In https://reviews.llvm.org/D45702#1085224, @aaron.ballman wrote: > > > > > > > > Have you run this over any large code bases to see whether the check > > > > > triggers in practice? > > > > > > > > I'm still curious about this, btw. > > > > > > > > > Yes it triggers in Google's code base. > > > > > > Were there any false positives that you saw? > > > From randomly checking several triggerings no I didn't find any false > positives. I feel the check should be pretty safe in terms of false positives > because we only trigger on configured types. Good to hear, thank you for the information. ================ Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45 + anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)))))), + callee(namedDecl(hasName("data")))) + .bind("call")))), ---------------- shuaiwang wrote: > aaron.ballman wrote: > > Eugene.Zelenko wrote: > > > aaron.ballman wrote: > > > > Should this check apply equally to `std::string::c_str()` as well as > > > > `std::string::data()`? > > > readability-redundant-string-cstr do both. > > Yup! But that makes me wonder if the name "redundant-data-call" is an > > issue. Perhaps the check name should focus more on the array subscript in > > the presence of an operator[]()? > How about "readability-circumlocutionary-subscript"? > "readability-circumlocutionary-element-access"? > "circumlocutionary" -> "verbose"? hah, I think circumlocutionary might be a bit too much. ;-) I think `readability-simplify-array-subscript-expr` might be reasonable, however. Right now, the simplification is just for `foo.data()[0]` but it seems plausible that there are other array subscript simplifications that could be added in the future, like `a[1 + 1]` being converted to `a[2]` or `x ? a[200] : a[400]` going to `a[x ? 200 : 400]` (etc). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45702 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
