[PATCH] D153651: [gn] Add check-lsan target for Mac

[Leonard Grey via Phabricator via llvm-commits]

lgrey added inline comments.


================
Comment at: llvm/utils/gn/build/toolchain/target_flags.gni:57
+      "x86_64",
+    ]
+  }
----------------
thakis wrote:
> iOS above also sets `target_ldflags`, don't we need that for mac? (If not, why not?)
> 
> Can we share code with the iOS path?
I didn't add them because it just doubled them up (https://github.com/llvm/llvm-project/blob/main/llvm/utils/gn/secondary/compiler-rt/test/test.gni#L13) but it probably can't hurt!

Had Mac as a separate clause to reduce nesting and I wasn't sure if sharing was what we wanted here. Now combined.


================
Comment at: llvm/utils/gn/secondary/BUILD.gn:28
+      "//compiler-rt/test/asan",
+      "//compiler-rt/test/lsan",
+    ]
----------------
thakis wrote:
> You add this here in the `linux or win or amc` branch, but currently i'll only have an effect on mac due to supported_toolchains being non-empty only on mac, right? (i'm wondering if things will start failing on http://45.33.8.238/ once this goes in, but I think it should be fine?)
Ooh good catch. Done.


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

https://reviews.llvm.org/D153651