mgorny added inline comments.

================
Comment at: lldb/cmake/modules/LLDBConfig.cmake:33
+    set(maybe_required)
+  elseif("${${variable}}" STREQUAL "ON")
+    set(maybe_required REQUIRED)
----------------
JDevlieghere wrote:
> mgorny wrote:
> > Now you broke it the other way around. Is there any reason you can't just 
> > `${${variable}}` here?
> Can you please describe the desired behavior rather than the symptom? I must 
> be missing what you're trying to achieve. :-) 
> 
> Maybe there's a gap in my CMake knowledge, but wouldn't `if(${${variable}})` 
> evaluate to `true` for **any** string value? 
The desired behavior is to keep standard CMake behavior for any value other 
than `AUTO`, and by standard behavior I mean: 
https://cmake.org/cmake/help/v3.0/command/if.html the first definition on the 
list.

Particularly, in Gentoo we default to passing `yes`/`no` for historical 
reasons. I would find it irritating if different places in LLVM accepted 
different constants.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71306/new/

https://reviews.llvm.org/D71306



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to