Add an option to specify wich style must be followed when choosing the new index name.
Details
Diff Detail
Event Timeline
clang-tidy/modernize/LoopConvertUtils.cpp | ||
---|---|---|
816 | This can be a StringRef to avoid some copies. | |
821 | Once ContainerName is a StringRef, this can be replaced with if (ContainerName.endswith("s")) | |
829 | The underscore here is not needed in the LLVM style. | |
833 | This should also handle LLVM style. | |
837 | What will that be? <container>_elem_elem? Doesn't make sense to me. | |
841 | 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). | |
845 | 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 | 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 | Please add tests for all naming conventions. | |
448 | 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 | The variable can be const, the one above as well. | |
clang-tidy/modernize/LoopConvertUtils.cpp | ||
822 | No need to check the length. endswith handles this itself. | |
831 | 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 | ||
447 | 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. |
The variable can be const, the one above as well.