Details
- Reviewers
kfunk - Group Reviewers
KDevelop - Commits
- R32:5e4a16d68df0: Default to QT_NO_CAST_FROM_ASCII/QT_NO_CAST_TO_ASCII/QT_NO_CAST_FROM_BYTEARRAY
Diff Detail
- Repository
- R32 KDevelop
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
There have been lots of commits across the whole codebase of all the plugins to use QLatin1String, QStringLiteral & Co.
Though the usage is not consistent, which leaves developer editing the code puzzled what to use.
Given that kdevelop is resource-heavy, default to the explicit classes with papercut-like performance effects might make sense in general, despite the more complex code to read for humans.
All plugins not yet building with the strict settings explicitly have to opt out, and would/could be ported one-by-one to follow the strict settings.
Test code should be always fine to not avoid the implicit conversions, so opt-out there would be always added.
As discussed on IRC.
I'd prefer this being fixed right away instead of introducing so much (temporary) CMake code.
Clazy has auto fixits for these kind of issues: https://github.com/KDE/clazy/blob/master/docs/checks/README-qstring-allocations.md
Tried that, but it seems most of the implicit casts were not detected, also many fixits were skipped due to strings being in macros and stuff. That needs more manual work :/
As discussed on IRC with Kevin, have now worked on fixing this right away, and this can go in now without all the temporary remove_definitions.
@volden Given you have mentioned you have a series of patches for the perforce plugin, I left that plugin out for now, so you do not run into conflicts. We would then complete the explicit-cast work here later :)
@kossebau I actually played around a little with the cast stuff myself. I now have it in a compiling/tests passing condition. Be happy to make a review request of that if you like.