aaron.ballman added a comment. In D108893#2972651 <https://reviews.llvm.org/D108893#2972651>, @Eugene.Zelenko wrote:
> In D108893#2972634 <https://reviews.llvm.org/D108893#2972634>, @compnerd > wrote: > >> @Eugene.Zelenko I'd like to have @aaron.ballman weigh in on the naming, and >> assuming that he's okay with `modernize.container-data-pointer`, I'll change >> it to that. > > Sure, Aaron has much broader vision than me. Lies! I just wear glasses, that's all. ;-) I can see a case being made for either `modernize` or `readability`. For `modernize`, `data()` was newly added in C++11, so you could argue that this is used to modernize C++98 code. For `readability`, `data()` makes the semantics of the underlying code more clear. Also, `bugprone` makes sense too because doing `&v[0]` is UB if `v` is empty, but `v.data()` is not (immediately) UB. I think `bugprone` would be a tough place to put it because of false positive concerns. This is more of a blanket code rewriting utility than it is trying to catch bugs (the static analyzer and UBSan are better tools for that in that case). I think `modernize` would be reasonable, but given that C++11 is already over ten years old, I feel like this is losing its "modernization" aspects in some ways. Also, this only covers converting `&vec[0]` into a call to `data()`; for `modernize`, I'd expect it to convert `&vec[N]` into `vec.data() + N` more broadly. I think `readability` makes slightly more sense because then we can position this as a rewriting tool to make the code more readable (so there are no false positives -- it transforms all `&vec[0]` calls to `data()`). However, if someone wants to make a strong case for `modernize`, I think it'd be defensible. Also, if someone wanted to suggest we add it to both `readability` and `modernize`, that could also make sense (perhaps with an option to convert `&vec[N]` that's on by default for modernize and off by default for readability). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108893/new/ https://reviews.llvm.org/D108893 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits