[cmake-developers] conditionals in generator expressions

[Stephen Kelly]

Brad King wrote:

> On 08/19/2012 05:43 PM, Stephen Kelly wrote:
>> Thanks, here are my findings so far, which relate to my extension of that
>> branch at
>> git at gitorious.org:~steveire/cmake/steveires-
cmake.git:
> 
> Thanks for working on this.

Thanks for the feedback. I've updated the branch.

> 
>> 1) The result string of the $<CONFIG:...> expression does not allow
>> lists. That means we can't do anything like this:
>> 
>> include_directories("$<$<CONFIG:Release>:/foo;/bar/bat>")
>> include_directories("$<$<CONFIG:Release>:${somedirs}>")
>> # somedirs might contain a list
>> 
>> I fixed that in my branch by modifying cmSystemTools::ExpandListArgument.
> 
> It's not the generator expression parsing that can't handle ';'.  It's
> that the division on ';' occurs before generator expressions are
> processed. 

<snip>

I've implemented it like that now. That will also work for 
compile_definitions, but I think it won't work for link_libraries. I'm also 
not sure about link_directories, compiler_flags and linker_flags. For 
link_libraries it is quite unfortunate, because it would make things like 

  $<${USE_FOO}:${FOO_LIBRARIES}>

not work as expected:

/usr/lib/icecc/bin/c++       CMakeFiles/fooex.dir/newex.cpp.o  -o fooex -
rdynamic libfoo.so libfoo.so $<1:Qt5::Widgets -lQt5::Sql> -Wl,-
rpath,/home/stephen/dev/src/playground/cmake/build 
g++: error: $<1:Qt5::Widgets: No such file or directory

I think this needs to be fixed, but I get the feeling it might require 
deeper redesign of the link libraries logic?

>> 2) Because of the way generator expressions are parsed, the > character
>> can't be part of the result string. This is an unlikely but possible
>> character to appear in include directories for example.
> 
> We could create a generator expression such as "$<RANGLE>" that expands
> to right-angle.  The expanded values are not re-processed by the parser
> so if it expands to '>' it would be accepted literally.

Done. It's also not ideal because it means the user has to potentially pre-
process the contents of all properties to replace the literal with the 
RANGLE. I don't see any better solution though.

>> include_directories("/tmp/$<$<CONFIG:Release>:foo/>bar")
>> # Could include /tmp/bar or /tmp/foo/bar, but not very readable.
>> 
>> Can this be changed?
> 
> I consider this a feature, and with my answer to (1) you would not have
> boundaries to test anyway.

Ok, I'm sure there can be a solution to the rest of (1).

>> 3) I can add something like $<TARGET_INTERFACE_INCLUDE_DIRS:tgt> in the
>> future to expand to the interface include directories of a target
>> (currently cmake has no such concept, but I'm thinking in the future).
>> 
>> However, I don't think something like
>> 
>> $<$<CONFIG:Release>:$<TARGET_INTERFACE_INCLUDE_DIRS:tgt>>
>> 
>> would work, would it? The 'right hand side' of the expressions must be
>> the final string?
> 
> It would work just fine.  The first "$<" token will be kept as a boundary
> on the stack of opening tokens by the parser until after the other two
> are expanded.  When the last ">" is reached then the expression will be
> either "$<0:/include/dirs>" or "$<1:/include/dirs>" and will be evaluated
> as one of those expressions.

Great, thanks.

> 
>> 4) I'm really not certain if I've implemented the patch for conditional
>> link libraries in the right place. I tried a few different locations. I'm
>> also not sure if there is logic at configure time which can't be
>> implemented in terms of the generator expressions which are not evaluated
>> by then. I tried using it with imported targets which had entries in
>> their IMPORTED_LINK_INTERFACE_LIBRARIES, which seemed to work, so that's
>> encouraging.
> 
> It should be in cmTarget::ComputeLinkImplementation at this line:
> 
>       // The entry is meant for this configuration.
>       impl.Libraries.push_back(item);
> 
> Process the value of the item string and use ExpandListArgument on the
> result to append to impl.Libraries.
> 
> Also in cmTarget::ComputeLinkInterface in the "if(explicitLibraries)"
> block between these two lines:
> 
>     // The interface libraries have been explicitly set.
>     cmSystemTools::ExpandListArgument(explicitLibraries, iface.Libraries);
> 
> Process the value of explicitLibraries before expanding as a list.
> 
> Generator expressions should be processed directly after reading the
> original string value from the user interface that provides it.

I tried this, but it made (5) below not work anymore. The generator 
expression was evaluated too early it seems for Qt5::Core, and at that point 
TYPE == SHARED_LIBRARY.

> 
>> 5) I think it's quite cool that I can add something like
>> 
>> set(_WIN32_EXE "$<AND:$<TARGET_PROPERTY_STREQUAL:TYPE,EXECUTABLE>,
>> $<TARGET_PROPERTY_BOOL:WIN32_EXECUTABLE>>")
>> target_link_libraries(Qt5::Core
>>   LINK_INTERFACE_LIBRARIES
>>    "$<${_WIN32_EXE}:Qt5::WinMain>"
>> )
>> 
>> and it will lie dormant and not have any effect on any shared libraries
>> until it is finally linked to an executable :). Several features come
>> together nicely there.
> 
> Nice.  BTW, I think the TARGET_PROPERTY_STREQUAL expression can use
> "==" instead of "," as the separator between property name and value.

Done.

>> 6) In my new implementation of cmTarget::GetIncludeDirectories, it is now
>> necessary for that method to only be called at generate-time. As far as I
>> can see, that is already true for that method, but for maintenance it is
>> a bit difficult. Are there other examples of similar requirements of when
>> an API can be meaningfully called? It seems to be something that could
>> benefit from refactoring throughout cmake if so.
> 
> This has been pretty messy in the past.  Recently I created the
> cmGeneratorTarget class to hold information/interfaces for a target
> that only make sense at generate time.  You could move the method
> over there.

I created a separate branch for that in my clone (use-generator-target). I 
think it's a good idea but a separate topic which should be run to 
completion.


Thanks,

Steve.