aaron.ballman added inline comments.

================
Comment at: clang-tidy/readability/RedundantDataCallCheck.cpp:45
+                  anyOf(TypesMatcher, pointerType(pointee(TypesMatcher)))))),
+              callee(namedDecl(hasName("data"))))
+              .bind("call")))),
----------------
shuaiwang wrote:
> aaron.ballman wrote:
> > 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).
> Just `readability-simplify-subscript-expr`?
> Since after simplification the subscript operation is done by calling 
> overloaded `operator[]` on an object instead of built-in subscript operator 
> on an array.
> 
> Let me know if this name looks good to you and I'll do the actual renaming 
> (together with addressing other comments) after your confirmation.
I think `readability-simplify-subscript-expr` is a reasonable name.


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

Reply via email to