I recently came across a bug in one of my projects build system, and I feel that its a result of an inconsistent design philosophy between the generator expressions, and the other logical functions.
using the ExternalPackage_Add function, I’m building a few third party dependencies, based on boolean variables that I set in earlier scripts, BUILD_ZLIB, and so on. In all of the logic handling functions, I would use this variable as
if(BUILD_ZLIB)
...do stuff
endif(BUILD_ZLIB)
if() would automatically dereference BUILD_ZLIB to see if it were true or false.
In the ExternalProject_Add function, I have CMAKE_ARGS lines similar to
$<$<BOOL:BUILD_ZLIB>:-DZLIB_ROOT=<INSTALL_DIR>>
A casual user might expect that the same boolean variable is dereferenced here as well, especially since there’s a giant “BOOL” setting right there beside it. However, the generator expression expects a STRING variable here. BUILD_ZLIB is interpreted as a literal string, which evaluates to TRUE. It was a mistake that was hard to find, and only after carefully reading the docs did I track it down. Fortunately the fix was a good sed command across a bunch of files to add the ${ ... } characters , but in large projects, I’m sure it would go unnoticed.
Would it be possible to change the behavior here with a new cmake policy? IE, check to see if the RHS of the BOOL:VARIABLE is an existing variable, and dereference it, like the other logical functions? I can’t imagine it would break existing code, as the only time people would be using it this way, would be if they were making a mistake, just as I was. It would also be more consistent with the other functionality.
Not really. When the genex is evaluated, the variable scope is long gone and inaccessible. You need to bake in a value. We don’t even parse it until it is meant to be evaluated (set(foo "$<NOT:A,GENEX") is perfectly valid code), so there’s no indication that it’s even a variable name at that point (as it could really have been a value).
With this discrepancy, I’d argue that if() is broken rather than the generator expressions. This syntax:
if(BUILD_ZLIB)
# ...
endif(BUILD_ZLIB)
is a holdover from the very early days of CMake. If if() were being implemented today, it would probably look more like:
if(${BUILD_ZLIB})
# ...
endif()
with the variable expansion in if(), just like you would put in the generator expression: $<BOOL:${BUILD_ZLIB}>.
CMP0054 fixes this to some extent for quoted arguments:
if("${BUILD_ZLIB}")
# ...
endif()
I’ve long considered proposing a policy to extend this to unquoted arguments and do away with the if(VAR) syntax entirely, but if(VAR) is so heavily ingrained into the CMake ecosystem by now that it would be a very invasive change, even with a policy.
The problem is that with if (${BUILD_ZLIB}), a value of foo;bar;baz-NOTFOUND is false (well, this is still the case today), a value of EXISTS;"/some/path" is not what one expects, etc. Back in the Old Days, quoting was not tracked, so if had no idea if the value came in via an expansion or literally.
I actually like the unquoted behavior because it makes it clearer, to me, that a variable is being tested, but I’m also pretty rigorous about my CMake style guidelines for code I write.
I tend to agree with Ben on the style issue - I like the look of the if(variable) rather than the if(${variable}) scheme.
Any suggestions on how to proceed with future code? I know it’s probably a non-issue, but we’ve always been in the if(variable) endif(variable) camp, especially when they’re heavily nested.
I always leave endif (and other end* commands) with empty arguments. The reason is that else (cond) is really dumb and only confusing (and even more so in an elseif tree).
The only thing I don’t like is:
if (var_${param})
breaks my “quote all variable expansions that aren’t intended to be a list of arguments to the command” rule. But it happens rarely enough that I don’t worry about it too much.