Add an option to specify wich style must be followed when choosing the new index name.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
clang-tidy/modernize/LoopConvertUtils.cpp | ||
---|---|---|
815 ↗ | (On Diff #35362) | This can be a StringRef to avoid some copies. |
820 ↗ | (On Diff #35362) | Once ContainerName is a StringRef, this can be replaced with if (ContainerName.endswith("s")) |
828 ↗ | (On Diff #35362) | The underscore here is not needed in the LLVM style. |
832 ↗ | (On Diff #35362) | This should also handle LLVM style. |
836 ↗ | (On Diff #35362) | What will that be? <container>_elem_elem? Doesn't make sense to me. |
840 ↗ | (On Diff #35362) | The option with the trailing underscore doesn't make sense to me as well (unless we add a support for a style requiring trailing underscores for variable names). |
844 ↗ | (On Diff #35362) | I'd go with give_me_name{0,1,2,...} or something else that would make it more obvious that the automatic name selection failed and a user input is needed. |
clang-tidy/modernize/LoopConvertUtils.h | ||
418 ↗ | (On Diff #35362) | The IdentifierNamingCheck defines styles in a more generic way (CamelCase, camelBack, lower_case, UPPER_CASE). It seems reasonable to use a similar set of options. Maybe even move the enum from that check to a common place together with the conversion routines. |
418 ↗ | (On Diff #35362) | Please add tests for all naming conventions. |
448 ↗ | (On Diff #35362) | Please make it const and put it right after Context. |
Use CamelCase on the existing tests and add a test for every other naming convention.
A few minor comments.
clang-tidy/modernize/LoopConvertCheck.h | ||
---|---|---|
68 ↗ | (On Diff #35380) | The variable can be const, the one above as well. |
clang-tidy/modernize/LoopConvertUtils.cpp | ||
820 ↗ | (On Diff #35380) | No need to check the length. endswith handles this itself. |
841 ↗ | (On Diff #35380) | Automatic captures always look suspicious and require more attention. I'd avoid them unless there's a significant improvement in readability. I'd also prefer a more generic function, say AppendSuffixWithStyle, which could then be placed next to the styles enumeration and other functions to work with banking styles. |
clang-tidy/modernize/LoopConvertUtils.h | ||
453 ↗ | (On Diff #35380) | nit: please remove the empty line. |
No need to check the length. endswith handles this itself.
I am checking that it is strictly greater than one, because we don't want an empty identifier after removing the "s".
Looks good with a comment. Thank you!
test/clang-tidy/modernize-loop-convert-uppercase.cpp | ||
---|---|---|
48 ↗ | (On Diff #35608) | Please add a test for GIVE_ME_NAME_{0,1}. Same for other styles. |