[Lldb-commits] [PATCH] D132642: [lldb] Fix dotest argument order

[Jonas Devlieghere via Phabricator via lldb-commits]

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