[PATCH] D101335: [ASan][Darwin] Fix `TestCases/Darwin/init_for_dlopen.cpp` when running in a standalone build.

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: compiler-rt/test/asan/lit.cfg.py:57
+def find_clang_compiler_rt_libdir(config):
+  import subprocess
+  clang_cmd = [
----------------
aralisza wrote:
> how come this import is inside the function?
To use the import only when it's needed. I don't have strong opinions here so I can make the import global if that's what you'd prefer.


================
Comment at: compiler-rt/test/asan/lit.cfg.py:70
+  except subprocess.CalledProcessError as e:
+    msg = 'Failed to run {}\nrc:{}\nstdout:{}\nstderr:{}\n'.format(
+        clang_cmd,
----------------
aralisza wrote:
> Is this python >= 3.6? What do you think about using f strings?
According to https://llvm.org/docs/GettingStarted.html#software python 3.6 is the minimum version so we should be able to use f strings.


================
Comment at: compiler-rt/test/asan/lit.cfg.py:87
+    # Standalone build of compiler-rt.
+    lit_config.warning('Path reported by clang ({}) != configured path ({})'.format(
+      path, config_crt_libdir_real))
----------------
aralisza wrote:
> super nit picky, but what do you think about formatting this warning so it's easier to compare the two paths? something like
> 
> ```
> f"Path reported by clang != configured path\n\tclang path:\t{path}\n\tconfigured path:\t{config_crt_libdir_real}"
> ```
I'll try it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101335