[Parallel_libs-commits] [PATCH] D24302: Add streamexecutor-config
Justin Lebar via Parallel_libs-commits
parallel_libs-commits at lists.llvm.org
Wed Sep 7 10:34:07 PDT 2016
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
More information about the Parallel_libs-commits
mailing list