JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: lldb/test/API/CMakeLists.txt:166
+set(LLDB_TEST_COMMON_ARGS ${LLDB_TEST_COMMON_ARGS_VAR} CACHE INTERNAL STRING)
+
----------------
fdeazeve wrote:
> JDevlieghere wrote:
> > Instead of having two variables, why not move this to line 40 and make
> > `LLDB_TEST_COMMON_ARGS` a cached variable and operate directly on that?
> I actually address this in the commit message:
>
> > The variable LLDB_TEST_COMMON_ARGS needs to be a CACHE property, but it
> is modified throughout the CMake file with set or list or string
> commands, which don't work with properties. As such, a temporary
> variable LLDB_TEST_COMMON_ARGS_VAR is created.
>
> Does this make sense?
Gotcha, I didn't know about that limitation. I'm 99% sure you can still use the
same variable and then cache it:
```
set(LLDB_TEST_COMMON_ARGS "")
list(APPEND LLDB_TEST_COMMON_ARGS "FOO")
list(APPEND LLDB_TEST_COMMON_ARGS "BAR")
set(LLDB_TEST_COMMON_ARGS "${LLDB_TEST_COMMON_ARGS}" CACHE INTERNAL STRING)
```
But maybe that's needless opaque.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132642/new/
https://reviews.llvm.org/D132642
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
