[CMake] Unclear warning

[Nils Gladitz]

> Hi Nils:
>
> I have never understood CMP0054 until now when I read your above
> clear explanation.  So thanks very much for that!
>
> My problem was the bad/ambiguous documentation of CMP0054 at
> <http://www.cmake.org/cmake/help/v3.1/policy/CMP0054.html> which
> states
>
> "Only interpret if() arguments as variables or keywords when unquoted."
>
> This wording implies (if taken rigourously) that
>
> "${CMAKE_C_COMPILER_ID}"
>
> is that exact literal string under NEW CMP0054 which I am sure (now
> after reading your above explanation) was not
> meant by the author of that documentation.

I am the author of that documentation :)
The policy influences how if() itself dereferences arguments.

It has no effect on explicit ${} expansion which happens before if() 
gets or handles its arguments.

Instead, of course,
> ${CMAKE_C_COMPILER_ID} always explicitly dereferences the variable
> even if inside quotes.  I think this is all covered beautifully by
> your above "protected from implicit expansion" wording.  So could you
> please change
> <http://www.cmake.org/cmake/help/v3.1/policy/CMP0054.html> to reflect
> that much clearer wording?

I don't think it is actually ambiguous given that if() itself only 
performs implicit expansion and not ${} expansion but I'll think about 
revising it.

>> The warning was fixed in
>> http://www.cmake.org/gitweb?p=stage/cmake.git;a=commit;h=4d52cd36 .
>
> This brings up the "ugly x" issue.
>
> There, effectively
>
> if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
>
> is replaced by the extraordinarily ugly (in my opinion because it
> reminds me of that sort of logic I used to see in shell scripts
> configured by autotools [!])
>
> if("x${CMAKE_C_COMPILER_ID}" STREQUAL "xMSVC")
>
> According to your above explanation when CMAKE_C_COMPILER_ID
> derefences to MSVC and MSVC dereferences to 1, this boils down to
>
> if("x1" STREQUAL "x1")

This is not the case.

After explicit expansion you've got:
   if("xMSVC" STREQUAL "xMSVC")

if() with OLD CMP0054 behavior would implicitly expand xMSVC if it were 
a variable but the assumption of the workaround is that while MSVC is 1, 
xMSVC is unset.

Since xMSVC is unset if() will not implicitly dereference it and will 
interpret it as a literal string instead.

>
> for OLD CMP0054 and
>
> if("xMSVC" STREQUAL "xMSVC")
>
> for NEW CMP0054 (since MSVC will not implicitly dereference for NEW).
> So this
> style of logic will work regardless of whether the user is using OLD
> CMP0054, NEW CMP0054 or even a a CMake version which does not define
> CMP0054.
>
> That's great, but could use of these ugly "x"'s, be accompanied by
> the following comment?
>
> # When OLD CMP0054 is no longer supported can drop the x.
>
> Such self documentation of these ugly x's will avoid the situation
> where users are inspired to propagate x's to user code (even if that
> already assumes the latest CMake version with CMP0054 NEW).
> Furthermore, such self-documentation insures these x's can all be
> easily found and removed from the distributed
> Modules/CMakeFindBinUtils.cmake when support for CMP0054 OLD gets
> dropped from CMake (in the far distant feature).
>
> In sum, I think a change to use much less ambiguous language
> in <http://www.cmake.org/cmake/help/v3.1/policy/CMP0054.html> is
> essential, and an extra line in distributed modules saying
>
> # When OLD CMP0054 is no longer supported can drop the x.
>
> is a "would be nice" wherever this ugly x logic is currently being
> deployed.

I'd defer those kind of decisions to Brad though I'd personally not vote 
for adding those comments.

I think that if CMP0054 OLD behavior does get removed at some point the 
if() calls that can be cleaned up are more easily and thoroughly found 
by looking for the actual call pattern than spurious comments.

Users getting inspired by code in cmake shipped modules might be a 
problem that goes beyond just CMP0054.

The requirements for writing those modules might be quite different from 
requirements a developer might have for his own modules or cmake setup.

The effects of CMP0054 might be more evident but in general find modules 
are intended to work irregardless of a project's current policy settings.

Best practice advice should be part of the actual documentation rather 
than comments in internal modules.

Nils