[llvm-dev] Extra space in LLVM_DEFINITIONS causes CMake 3.1 to fail

[Mueller-Roemer, Johannes Sebastian via llvm-dev]

LLVM_DEFINITIONS used to be defined as

 

set(LLVM_DEFINITIONS "-D__STDC_LIMIT_MACROS" "-D__STDC_CONSTANT_MACROS")

 

Now it is defined as

 

set(LLVM_DEFINITIONS " -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS")

 

(note that it is no longer a cmake list but a string, and the string contains a leading space!)

 

This causes CMake 3.1 to emit

 

g++  -D -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS […]

 

when LLVM_DEFINITIONS is used via target_compile_definitions. Which, unsurprisingly, fails.

 

Although this issue is fixed in current CMake versions (3.2+), do we really want to require these versions just to have LLVM_DEFINITIONS defined as a string instead of a list? A simple check if LLVM_DEFINITIONS is empty in add_llvm_definitions might solve the issue.

 

--

Johannes S. Mueller-Roemer, MSc

Wiss. Mitarbeiter - Interactive Engineering Technologies (IET)

 

Fraunhofer-Institut für Graphische Datenverarbeitung IGD

Fraunhoferstr. 5  |  64283 Darmstadt  |  Germany

Tel +49 6151 155-606  |  Fax +49 6151 155-139

johannes.mu...@igd.fraunhofer.de  |  www.igd.fraunhofer.de

 

[Mueller-Roemer, Johannes Sebastian via llvm-dev]

Correction, I was accidentally testing with an old version for my CMake 3.2+ tests. They fail just the same with the current version.

 

--

Johannes S. Mueller-Roemer, MSc

Wiss. Mitarbeiter - Interactive Engineering Technologies (IET)

 

Fraunhofer-Institut für Graphische Datenverarbeitung IGD

Fraunhoferstr. 5  |  64283 Darmstadt  |  Germany

Tel +49 6151 155-606  |  Fax +49 6151 155-139

johannes.mu...@igd.fraunhofer.de  |  www.igd.fraunhofer.de

 

[John Brawn via llvm-dev]

I made this change (not specifically changing from a list to a string, but changing from a hardcoded value to the value

that was actually decided by cmake), and I checked that it works with cmake 2.8.12.2 which is the minimum version

set in CMakeLists.txt. So if it doesn’t work with cmake 3.1/3.2 then that looks like something new that was introduced

after 2.8.12.2.

 

I’ll have a look at this to try and figure out what’s going on.

 

John

[Paweł Bylica]

I've fixed that: http://reviews.llvm.org/D13432

_______________________________________________
LLVM Developers mailing list
llvm...@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

[Mueller-Roemer, Johannes Sebastian via llvm-dev]

The proper correction would probably be to use a list instead. As using a string with target_compile_definitions is not correct anyways. In any case, maybe we should instead use target_compile_definitions instead of add_llvm_definitions, it is supported by our minimum version: https://cmake.org/cmake/help/v2.8.12/cmake.html#command:target_compile_definitions

 

 

--

Johannes S. Mueller-Roemer, MSc

Wiss. Mitarbeiter - Interactive Engineering Technologies (IET)

 

Fraunhofer-Institut für Graphische Datenverarbeitung IGD

Fraunhoferstr. 5  |  64283 Darmstadt  |  Germany

Tel +49 6151 155-606  |  Fax +49 6151 155-139

johannes.mu...@igd.fraunhofer.de  |  www.igd.fraunhofer.de

 

[Chris Bieneman via llvm-dev]

I don’t think there is any reason why we would want to push people to any particular version of CMake. Personally I’d love to make CMake 3.2 our minimum version, but at the moment our minimum required (and supported) version of CMake is 2.8.12.2.

It looks to me like the issue you’re reporting was an unintended side effect of r248884. We will of course welcome patches to resolve the issue.

-Chris

[Mueller-Roemer, Johannes Sebastian via llvm-dev]

I share the oppinion that a newer CMake version would be better. In any case, D13432 resolved the issue. As I mentioned in the comments on it, LLVM_DEFINITIONS can also contain non-definition flags, so for now I switched my code to

 

separate_arguments(LLVM_DEFINITIONS_LIST UNIX_COMMAND ${LLVM_DEFINITIONS})

set(LLVM_DEFINITIONS)

set(LLVM_FLAGS)

foreach(DEF ${LLVM_DEFINITIONS_LIST})

                string(REGEX MATCH "^[-/]D" DEF_MATCH ${DEF})

                if(DEF_MATCH)

                               string(SUBSTRING ${DEF} 2 -1 DEF)

                               list(APPEND LLVM_DEFINITIONS ${DEF})

                else()

                               list(APPEND LLVM_FLAGS ${DEF})

                endif()

endforeach()

 

[…]

 

target_compile_definitions(

                ${PROJECT_NAME}

                PUBLIC

                ${LLVM_DEFINITIONS}

)

target_compile_options(

                ${PROJECT_NAME}

                PUBLIC

                ${LLVM_FLAGS}

)

 

This solves both the issue of any extraneous flags that are in there as well as separating everything properly into separate list entries (each list entry in compile definitions is either prefixed with –D or /D depending on compiler unless it begins with –D or /D and the remainder is copied verbatim, which explains the breakage if an entry starts with a space).

 

--

Johannes S. Mueller-Roemer, MSc

Wiss. Mitarbeiter - Interactive Engineering Technologies (IET)

 

Fraunhofer-Institut für Graphische Datenverarbeitung IGD

Fraunhoferstr. 5  |  64283 Darmstadt  |  Germany

Tel +49 6151 155-606  |  Fax +49 6151 155-139

johannes.mu...@igd.fraunhofer.de  |  www.igd.fraunhofer.de

 

From: cbie...@apple.com [mailto:cbie...@apple.com] On Behalf Of Chris Bieneman

Sent: Wednesday, October 07, 2015 18:29
To: Mueller-Roemer, Johannes Sebastian
Cc: llvm...@lists.llvm.org