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

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

In D58578#1475863 <https://reviews.llvm.org/D58578#1475863>, @yln wrote:

> Overall changes: LGTM, thanks!
>
> As already noted, I would probably change the is_valid_apple_platform to:
>  `is_apple_platform` -> just yes/no, even for the empty string.
>  or rename to `assert_valid_apple_platform()` -> no return value, aborts if not.
>
> An `is_xxx` "getter" having 3 kinds of "results": true, false, or abort; is unexpected to me.


One way of looking at this is the empty platform name check being a precondition for the `is_valid_apple_platform` function.
It seems reasonable (to me at least) that if you violate the pre-condition that we should abort. In a language like C/C++ I probably
wouldn't have a precondition like this but in a language like CMake where it's incredibly easy to accidently pass the empty string
it seems worth having the check.

> I don't feel too strongly strongly about this though, so land this at your discretion.

Okay. I've landed it as is. We can revisit this later if it really becomes a problem.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D58578