[Parallel_libs-commits] [PATCH] D24302: Add streamexecutor-config

[Justin Lebar via Parallel_libs-commits]

jlebar added inline comments.

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:62
@@ +61,3 @@
+  except ValueError:
+    return
+
----------------
Can we scope the try/except around tokens.index?

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:87
@@ +86,3 @@
+          --cppflags --cxxflags --ldflags --libs --system-libs)
+""")
+
----------------
Consider `description=__doc__` and then just updating this file's docstring.

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:129
@@ +128,3 @@
+      raise
+    print('llvm-config not found in PATH, please add it to the PATH and retry')
+    return
----------------
Nit, write to stderr (`print('foo', file=sys.stderr)`)?  (Actually not quite a nit because of how this program is going to be used -- ideally we wouldn't print anything other than flags to stdout.)

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:130
@@ +129,3 @@
+    print('llvm-config not found in PATH, please add it to the PATH and retry')
+    return
+
----------------
Exit with an error code?  Same elsewhere.

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:161
@@ +160,3 @@
+
+  print(' '.join(all_flags))
+
----------------
Should we do something to tokenize the flags?  (So that e.g. `-Idir with spaces` shows up correctly?)


https://reviews.llvm.org/D24302