This is an archive of the discontinued LLVM Phabricator instance.

[clang] Switch to LLVM_ENABLE_IDE
ClosedPublic

Authored by smeenai on Feb 15 2019, 8:06 AM.

Details

Summary

r344555 switched LLVM to guarding install targets with LLVM_ENABLE_IDE
instead of CMAKE_CONFIGURATION_TYPES, which expresses the intent more
directly and can be overridden by a user. Make the corresponding change
in clang. LLVM_ENABLE_IDE is computed by HandleLLVMOptions, so it should
be available for both standalone and integrated builds.

Diff Detail

Repository
rL LLVM

Event Timeline

smeenai created this revision.Feb 15 2019, 8:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 15 2019, 8:06 AM

CC @lebedev.ri, since I believe you raised some issues during the corresponding LLVM change but those were addressed.

CC @lebedev.ri, since I believe you raised some issues during the corresponding LLVM change but those were addressed.

Yeah, i don't think i have any comments here.
The logic to pick the default of LLVM_ENABLE_IDE (based on the existence of CMAKE_CONFIGURATION_TYPES) seems hacky,
but it indeed does not currently detect QtCreator's CMake integration as "IDE", so i don't have any additional concerns,
this looks like a straight-forward cleanup.

What i do have concerns about, is that LLVM_ENABLE_IDE is not documented in https://llvm.org/docs/CMake.html

CC @lebedev.ri, since I believe you raised some issues during the corresponding LLVM change but those were addressed.

Yeah, i don't think i have any comments here.
The logic to pick the default of LLVM_ENABLE_IDE (based on the existence of CMAKE_CONFIGURATION_TYPES) seems hacky,
but it indeed does not currently detect QtCreator's CMake integration as "IDE", so i don't have any additional concerns,
this looks like a straight-forward cleanup.

What i do have concerns about, is that LLVM_ENABLE_IDE is not documented in https://llvm.org/docs/CMake.html

Good point. Added documentation in D58286.

phosek accepted this revision.Feb 20 2019, 3:04 PM

LGTM

This revision is now accepted and ready to land.Feb 20 2019, 3:04 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2019, 3:08 PM