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

[Jason Henline via Parallel_libs-commits]

jhen added inline comments.

================
Comment at: streamexecutor/tools/streamexecutor-config/streamexecutor-config.in:155
@@ +154,3 @@
+    all_flags.append(llvm_flags)
+    se_flag = "-I'@CMAKE_INSTALL_PREFIX@/include'"
+    if not has_token(token=se_flag, string=llvm_flags):
----------------
jlebar wrote:
> jhen wrote:
> > jlebar wrote:
> > > How does `@CMAKE_INSTALL_PREFIX@` work?  That is, who's responsible for replacing it with an actual path?  From the example in the docstring, it looks like nobody is?
> > This is a cmake built-in variable that is replaced at build time when this file is configured.
> > 
> > The code in this patch that sets up this configuration is the cmake configure_file command in line 2 in streamexecutor/tools/streamexecutor-config/CMakeLists.txt.
> > This is a cmake built-in variable that is replaced at build time when this file is configured.
> 
> ...wow.  Okay.
> 
> What happens if the install prefix contains the `'` character?  Do we just not care?
Oh, thanks for reminding me. I added the ESCAPE_QUOTES option to the cmake configure_file command. That should make sure any quotes in the variable are properly escaped.


https://reviews.llvm.org/D24302