[PATCH] D86379: [sanitizer] When building tests on Darwin, always pass -arch and other common flags

Kuba (Brecka) Mracek via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 31 13:28:12 PDT 2020


kubamracek added inline comments.


================
Comment at: compiler-rt/cmake/config-ix.cmake:221
 macro(get_test_cc_for_arch arch cc_out cflags_out)
-  if(ANDROID OR ${arch} MATCHES "arm|aarch64")
+  if(NOT APPLE AND (ANDROID OR ${arch} MATCHES "arm|aarch64"))
     # This is only true if we are cross-compiling.
----------------
delcypher wrote:
> delcypher wrote:
> > The CMake documentation (https://cmake.org/cmake/help/v3.1/command/if.html) makes it look like (I've not verified this) the operator precedence is
> > 
> > ```
> > NOT (APPLE AND (ANDROID OR ${arch} MATCHES "arm|aarch64"))
> > ```
> > 
> > which is wrong because the second condition has been flipped compared to what existed prior to this patch.
> > 
> > Please explicitly use parentheses here to remove any ambiguity.
> > 
> > I'm guessing you wanted:
> > 
> > ```
> > (NOT APPLE) AND (ANDROID OR ${arch} MATCHES "arm|aarch64"))
> > ```
> Sorry I meant
> 
> ```
> (NOT APPLE) AND (ANDROID OR ${arch} MATCHES "arm|aarch64")
> ```
No, "NOT" behaves sensibly in CMake, i.e. `if (NOT A AND B)` actually means `if ((NOT A) AND B)`. Given that (and the fact that we use this short form in many other places in compiler-rt CMake), do you still want me to add the parens?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86379



More information about the llvm-commits mailing list