Details
- Reviewers
aaronpuchert - Group Reviewers
KDevelop - Commits
- R32:416cd8152ddf: Enable warnings zero-as-null-pointer-constant & suggest-override
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.
| CMakeLists.txt | ||
|---|---|---|
| 129 | This typo fix would be done as separate commit. So far it had prevented -pedantic to be enabled with GCC :) | |
| CMakeLists.txt | ||
|---|---|---|
| 134 | With Clang you could add -Winconsistent-missing-override, although it's not exactly the same. (It's a bit weaker.) | |
| plugins/astyle/3rdparty/libastyle/CMakeLists.txt | ||
| 10–11 | Maybe you can remember in add_compile_flag_if_supported if a warning flag was enabled and check that here? That would seem more consistent to me. | |
| CMakeLists.txt | ||
|---|---|---|
| 134 | A bit weaker might be still better than no strength at all? If so, that one would be a candidate also for https://phabricator.kde.org/source/extra-cmake-modules/browse/master/kde-modules/KDEFrameworkCompilerSettings.cmake (where I took the base snippets from). | |
| plugins/astyle/3rdparty/libastyle/CMakeLists.txt | ||
| 10–11 | Agreed, had the same idea. Though for now I was lazy and just copied the approach done for the -Wdocumentation flag. Left that improvement though for a latter separate patch, until the flag addition in general are accepted. | |
Left that improvement though for a latter separate patch, until the flag addition in general are accepted.
The flags are definitely fine, there is no reason not to use nullptr and override these days.
| CMakeLists.txt | ||
|---|---|---|
| 134 | The docs say the diagnostic "-Winconsistent-missing-override" is "enabled by default.": Other places claim it is part of "-Wall" at least since clang 3.5, but I could not find anything related in the release notes, so far unsure when this was added and when per default enabled. So would you think it is still needed here to try to enable it as well? I would assume disabling it for the 3rd-party makes sense still. | |
| CMakeLists.txt | ||
|---|---|---|
| 134 | If -Winconsistent-missing-override is on by default, there is nothing you need to do. (I haven't seen the warning for third-party code, so disabling it might also not be necessary.) | |
- add macro add_target_compile_flag_if_supported for use with the target-specific flags
- move both macros into new file KDevelopMacrosInternal.cmake
| CMakeLists.txt | ||
|---|---|---|
| 134 | Okay, could also not see any traces for such warnings with freebsd clang builds on CI. | |
| plugins/astyle/3rdparty/libastyle/CMakeLists.txt | ||
| 10–11 | After I had experimentally added support for -Winconsistent-missing-override and its inversion, I found being able to drop any version checks would be nice to have already now, so went and extended the macro side of things. As I am not sure if one can rely on the "no-" version to be present for a flag, I simply have gone to propose testing the actual flag to be used to be present, so we are on the safe side. | |
Given only positive feedback (here and on irc, thanks everyone), going to push now :)