| ivan |
| Frameworks | |
| Build System |
Disables alternative tokens for &&, ||, etc.. They're are not supported
by MSVC out of the box, thus using them will limit the portability of
the code. There *are* options to make alternative tokens available under
MSVC [1], but I think we shouldn't promote the usage of them.
From the GCC documentation:
-fno-operator-names: Do not treat the operator name keywords and,
bitand, bitor, compl, not, or and xor as synonyms as keywords.
| No Linters Available |
| No Unit Test Coverage |
Note: Just refreshing my complete KF5 build to test the change.
kactivities fails:
/home/kfunk/devel/src/kf5/kactivities/autotests/common/test.h:143:25: error: token is not a valid binary operator in a preprocessor subexpression
#if defined(Q_NO_DEBUG) or (not defined(Q_OS_LINUX))
~~~~~~~~~~~~~~~~~~~ ^
1 error generated.What about adding a way (cmake variable?) to opt-in if one wants to use the alternative operators? Personally I like and use them whenever I start something from scratch...
Hmm... You could overwrite the setting by just doing this after include(KDECompilerSettings):
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -foperator-names")But I just learned that clang++ (Clang 3.9) does not support -foperator-names. GCC does support it...
I'd like to hear some more opinions (from Ivan maybe) first. Making the use of alternative tokens opt-in is an option of course.
Isn't it better to use check_cxx_compiler_flag to see if the flag is supported and enable it in case it is?
-fno-operator-names is an ancient compiler flag, supported by GCC since at least 2000 [1]. I've also tested Clang, it's supported from at *least* Clang 3.5 there. Intel compiler also supports it since at least 2005 [2]. I don't think we need a check_cxx_compiler_flag here.
[1] https://gcc.gnu.org/ml/gcc-patches/2000-04/msg00333.html
[2] https://software.intel.com/en-us/forums/intel-c-compiler/topic/308561
For the record, if anyone wants to override this flag, the following works also with clang 3.9:
include(KDECompilerSettings) ... string(REPLACE "-fno-operator-names" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
Alternative tokens [lex.digraph] are standard c++ and have been since it's was standardized (and I don't know of any plan to deprecate them). They are a accessibility feature, since not every keyboard has those keys.
Disabling operator names makes it impossible to include a header file which uses them in an template, inline or constexpr function. Since it is also not possible to re-enable operator names in icc and clang this seems less like a 'discouragement' than a force.
Making three(!) compilers diverge from the standard and forcing users of ECM to diverge too because MSVC needs a switch to follow the standard is in my opinion a bad idea.
A standards conformant compiler is a requirement for portability, not a limit.
Please reconsider.
I start to agree that it's probably better to revert this patch, for the simple reason: We might break compilation of a project using boost if boost decides to add code which uses alternative tokens to any of its headers. This is not under our control.
On the other hand, alternative tokens are indeed *rarely* used (at least in KDE they are mostly used b/c of simple typos) and only cause harm when code is being ported to MSVC. I'd like to avoid running into issues like that whenever possible.
In general it'd be better to have a compiler *warning* instead when alternative tokens are used, but there isn't.
I'm undecided -- Apparently we've already fixed all issues in KDE land (/me didn't hear any more complaints so far), so we're good in that area. But problems from upgrading versions of dependent libraries can still be a problem -- though still easily fixed by stripping the flag inside CMake user code...
This might be better solved by a krazy-like check, instead of making the compilation fail...
Closing. Please just strip the flag from CMake flags if you're using a library making use of alternative tokens.
We've just seen compilation failing early because some lib in KF5 accidentally used alternative tokens => would have broken compilation on Windows again. So in one regard this is really good to have in place. I didn't receive any more complaints about this patch so far.