[PATCH] D58578: [CMake] Fix the value of `config.target_cflags` for non-macOS Apple platforms.

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 25 10:48:40 PST 2019


delcypher marked an inline comment as done.
delcypher added inline comments.


================
Comment at: compiler-rt/cmake/config-ix.cmake:221
+
+function(is_valid_apple_platform platform is_valid_out)
+  set(is_valid FALSE)
----------------
yln wrote:
> Nitpick: is_valid_apple_platform can return true/false, or error.
> From the name I would not expect that it can error.
> I would prefer:
> is_valid_apple_platform -> true/false
> [assert/check/ensure/...]_is_valid_apple_platform -> no return value, just assert.
> 
The error is a misuse of the API. Passing an empty string is almost certain a bug and we should stop immediately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58578





More information about the llvm-commits mailing list