commit a5f3a76e14799c68b5e8f74e375baa5f6f6ab4dc breaks build of digikam (again, see bug 133525), as not is used as keyword here. Other packages might be affected too, at least libkdcraw is. Additional, not is used in boost too, so just fixing the KDE packages is not a solution.
Those failing builds can be fixed with the following cmake trick: https://phabricator.kde.org/D3850#78322 Boost should do something similar in their build system, I suppose...
Note: All issues in Digikam + libkdcraw are resolved already. I think there's a real benefit to use this flag globally (as it inhibits us writing non-cross-platform code). I'll keep this open for now, for other opinions
Okay, external projects (here: boost) are indeed a problem...: /usr/include/boost/graph/transitive_reduction.hpp:108:25: error: unknown type name 'not' if( not edge_in_closure[i][j] ) { ^ /usr/include/boost/graph/transitive_reduction.hpp:108:29: error: variable declaration in condition must have an initializer if( not edge_in_closure[i][j] ) { ^ /usr/include/boost/graph/transitive_reduction.hpp:110:29: error: unknown type name 'not' if( not edge_in_closure[i][k] ) { ^ /usr/include/boost/graph/transitive_reduction.hpp:110:33: error: variable declaration in condition must have an initializer if( not edge_in_closure[i][k] ) { ^ 4 errors generated.
Ah right, boost likes code in headers... But then again, if a cmake project is using boost, it can just remove -fno-operator-names from the CMAKE_CXX_FLAGS variable.
I've fixed the individual problems.
(In reply to Kevin Funk from comment #5) > I've fixed the individual problems. Don't you think that projects should be able to use this syntax if their developers consider it justified? I'm not arguing against disabling support for them by default even if as argued on phab, compilers ought to support them by now. However, the CMake trick to remove the argument may work but is ugly easy to get wrong. I think it would be much more user-friendly if the ECM would either 1) provide a way to NOT add the argument or 2) provide a macro to do the replacement. And maybe a modified FindBoost package could be included that unsets the flag?
Is there a problem you're trying to fix or is this just a theoretical concern?
A bit of both. I ran into this issue rebuilding the previous digiKam release (why is not relevant here). Applications that use boost are not a theoretical concern, they exist. The ECM are supposed to be a collection of convenience macros and modules, allowing among others to write more readable CMake files that are easier to maintain. As such I think they shouldn't oblige software to use rarely used constructs (and a bit alien to CMake's principle too, IIUC). If there is really no other solution here, the least that could be done is provide something that hides the syntax and offloads the obligation for developers to be aware of a rarely used compiler argument. Ideally that would be a *documented* mechanism with the effect that the compiler argument is never added to CXXFLAGS but it could also be a macro of the form `ECM_PROJECT_USES_NAMED_LOGICAL_OPERATORS`. I'm not very well versed in writing CMake code but I could put a simple prototype up on Phabricator and see what we can come up with. Maybe a sort of attribute system where you can declare specific, less common needs of the project before including the ECM modules that set up the build environment (= the SET_PROJECT_PROPERTIES feature CMake forgot to implement ;)). --- To be very honest, I wasn't even aware that the C/C++ standard define named logical operators. So I googled a bit and came across a number of good arguments why one might prefer them. Not compelling enough to change my own practice but I can very well understand why complex projects like boost use them - and I doubt that boost has any anti-MSVC agenda (didn't check, though :)). What I think the ECM should do is set the options required to make software compile on all supported/major platforms. Setting things up so that software fails to build everywhere because there exists a platform where certain practices cause build failure feels like caressing the proverbial cat against its fur. Esp. if it happens in a minor release, possibly even without documentation other than a commit message. No matter how big MS Windows is compared to all other platforms it's still a platform that's very different from KDE's main platform (Unix/Linux and derivatives). It would be much more in line with the other ECM features to figure out how to get all platforms to accept what they should be accepting in the first place. FWIW, should getting MSVC to accept named logical operators not be feasible, isn't this something that should be implemented as a CMake policy?
I really don't like having to spend my time defending a change which actually helps us developers writing better code, but here we go... If there are some compiler switches that help us ensuring we're writing cross-platform code, then there are good reasons to use it. We've come across this problem: some Linux dev accidentally (freudian slip most of time) uses one of the operator keywords, and then just later we've noticed it broke the MSVC build. So this is addressing some real world issues. > Applications that use boost are not a theoretical concern, they exist. The ECM are supposed to be a collection of convenience macros and modules, allowing among others to write more readable CMake files that are easier to maintain. > (...) Yes, and also to ensure we write proper C++ code, by enabling several compiler warnings, etc. pp. > To be very honest, I wasn't even aware that the C/C++ standard define named logical operators. So I googled a bit and came across a number of good arguments why one might prefer them. Not compelling enough to change my own practice but I can very well understand why complex projects like boost use them - and I doubt that boost has any anti-MSVC agenda (didn't check, though :)). I just grepped the Boost source code (for 'not' and 'and') from the 15+ Boost libraries I have installed and it looks like there's indeed just occurrences of them in 'transitive_reduction.hpp', which looks to me like a freudian slip as well (all other files in that subdir use '!' and '&&' as usual). > What I think the ECM should do is set the options required to make software compile on all supported/major platforms. Setting things up so that software fails to build everywhere because there exists a platform where certain practices cause build failure feels like caressing the proverbial cat against its fur. Not really. If we can enable compiler warnings/error in a way it helps us writing cross-platform code, that's good. Please see this from a developer perspective. > Esp. if it happens in a minor release, possibly even without documentation other than a commit message. And a review, and with a discussion on the review... > No matter how big MS Windows is compared to all other platforms it's still a platform that's very different from KDE's main platform (Unix/Linux and derivatives). And b/c it's different we shouldn't make it easier to develop for it? > It would be much more in line with the other ECM features to figure out how to get all platforms to accept what they should be accepting in the first place. Not sure what you mean here. > FWIW, should getting MSVC to accept named logical operators not be feasible, isn't this something that should be implemented as a CMake policy? There's really no need to touch CMake here. There's an option to make MSVC adhere more to compiler standards (/Za) but I do not know the side effects of it. Please, let's not waste time discussing this change if there are no real world issues with it. The two or three individual problems with this patch have been sorted out, and since then no developer complained. We're really better off spending our time elsewhere.
> helps us developers writing better code That's a bit the point, using named logical operators can easily be defended as "helping write better code". "not" is more visible than "!", and confusion/mistakes between the logical and bitwise versions become much less likely and probably easier to spot. > There's an option to make MSVC adhere more to compiler standards (/Za) but I do not know the side effects of it. That too is one of the points: there appear to be side-effects. If that is not the case, then the ECM should add that option when MSVC is used instead of using -fno-operator-names everywhere else. Actually, q quick read of the /Za description suggests that might be a good idea anyway: https://msdn.microsoft.com/en-us/library/aa278577(v=vs.60).aspx Sadly I cannot test it. > The two or three individual problems with this patch have been sorted out, and since then no developer complained. We're really better off spending our time elsewhere. No wonder no developer has complained about a fix that was made for them. Look, this doesn't have to be a drawn-out discussion. I've brought it up because I saw you have been hesitating yourself. Let me just say 1 more thing: KDE_ENABLE_EXCEPTIONS and I'll work on an equivalent and put it up for review after testing it (hopefully with MSVC too).
> That's a bit the point, using named logical operators can easily be defended as "helping write better code". "not" is more visible than "!", and confusion/mistakes between the logical and bitwise versions become much less likely and probably easier to spot. That's off-topic/orthogonal. Or do you plan to port all code of KDE/Qt to named operators? We've never used them in KDE projects, nor will we in any near future. s/better/cross-platform/ if you prefer.
> Look, this doesn't have to be a drawn-out discussion. I've brought it up because I saw you have been hesitating yourself. Let me just say 1 more thing: > KDE_ENABLE_EXCEPTIONS Sorry, forgot: +1 on that one. Patches welcome :)
> We've never used them in KDE projects, nor will we in any near future. Define "we"? ;) > Sorry, forgot: +1 on that one. Patches welcome :) Whew... :)
> Define "we"? ;) ... you asked for it: "All but you?" But I'll stop filling the bug tracker with useless spam now.