[PATCH] D17734: [libcxx] PR26777 Fix tests when built with CXX="ccache clang++"

[Eric Fiselier via cfe-commits]

EricWF requested changes to this revision.
EricWF added a comment.
This revision now requires changes to proceed.

I appreciate the patch, handling 'ccache' is a good thing to have.  I'll look at this again once the inline comment is addressed.


================
Comment at: CMakeLists.txt:225
@@ +224,3 @@
+# treated as seperate parameters.
+if(CMAKE_CXX_COMPILER_ARG1)
+  string(STRIP ${CMAKE_CXX_COMPILER_ARG1} LIBCXX_COMPILER )
----------------
What if somebody uses CXX='clang++ --foo'. It seems incorrect to handle ccache in this way. Instead I would rather we
1) Check if 'ARG1' is present.
2) If so attempt to find the string 'ccache' within 'CMAKE_CXX_COMPILER'
3) Only if we manage to match it do we use  'CMAKE_CXX_COMPILER_ARG1' as 'LIBCXX_COMPILER'. 


http://reviews.llvm.org/D17734