stella.stamenova added inline comments.
================
Comment at: lit/CMakeLists.txt:10-13
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER
${LLDB_TEST_C_COMPILER})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER
${LLDB_TEST_CXX_COMPILER})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR
${LLVM_LIBRARY_OUTPUT_INTDIR})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR
${LLVM_RUNTIME_OUTPUT_INTDIR})
----------------
stella.stamenova wrote:
> zturner wrote:
> > This only works if you're using a just-built clang, which might not be the
> > case. In fact, it's usually not the case, because it's common to want to
> > run the test suite against a debug build of lldb but using a release build
> > of clang (otherwise you'll be there all day waiting for it to finish).
> >
> > I feel like if the user specifies an absolute path to the test compiler on
> > the command line, that should be what it uses -- always. If we want to use
> > a just built toolchain, maybe we need something else, like
> > `-DLLDB_TEST_BUILT_CLANG=ON`, which would trigger this logic.
> >
> > As I don't use this configuration though, I'm interested in hearing your
> > thoughts.
> It actually does work in the case when a user specifies a compiler on the
> command line as well as when the just-built clang is used and the default
> today is to use the just-built clang.
>
> As far as I can tell, you can specify the compiler with LLDB_TEST_C_COMPILER
> (or LLDB_TEST_CXX_COMPILER) when calling CMake or by passing a value to the
> tests directly with -C. I assume that you are concerned about the first case
> - when passing the property to CMake?
>
> In that case LLDB_TEST_C_COMPILER is set to /path/to/compiler and these lines
> won't actually affect it - unless for some reason the path contained
> ${CMAKE_CFG_INTDIR}. If ${CMAKE_CFG_INTDIR} is a period, which is the likely
> scenario, it will just be replaced by another period since the build mode
> would be the same.
>
> The only case when it might not work is if ${CMAKE_CFG_INTDIR} is in the path
> but not a period - but that is unlikely since the scenario would mean that
> ${CMAKE_CFG_INTDIR} is, say, $Configuration and the path that the user
> specified contains $Configuration AND it is different than the one they're
> using for LLDB.
>
> On the other hand, without this change it is not possible to use the
> just-built compiler when using Visual Studio, for example, because the path
> to the just built compiler is not being set correctly and this particular
> substitution needs to happen here.
>
> The way to make sure that both scenarios work always is to guard against the
> substitution when the user explicitly specifies a path or to enforce the
> sibstitution when they're using the just-built clang so a property like
> LLDB_TEST_BUILD_CLANG would work. If you think that the error scenario is
> likely enough, then we should add the property.
>
Actually, I misspoke. There is another way to do it without an extra property
that the user has to pass.
Right now, in the main CMakeLists.txt for LLDB, we create a property
LLDB_DEFAULT_TEST_C_COMPILER which overwrites LLDB_TEST_C_COMPILER if
LLDB_TEST_C_COMPILER is not set by the user. We could at that point first check
whether LLDB_TEST_C_COMPILER was explicitly set and internally create a
property to tell us whether to overwrite later. I think that may actually be
better since it maintains the current behavior and there's no need for a new
explicit property.
Thoughts?
https://reviews.llvm.org/D43096
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
