Re: [Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Jim Ingham via lldb-commits]

Thanks for laying this out.  I don't think any of us have a brief for getopt, 
it was the tool at hand.  Getting some fuzzy match suggestions would be nice.

Your comment about getopts reminds me, a more fundamental problem with lldb's 
command interpreter is that the CommandObject is stateful.  So you can't 
actually call a command and then in the process of handling that command call 
back into the same command without destroying the options settings from the 
older invocation.  We actually do this  (e.g. anything that hits a breakpoint 
can call arbitrary lldb commands).  We just luck out because most commands grab 
their arguments, decide what to do with them, then do the work.  So the second 
invocation overwrites options that aren't going to get reused.  But working by 
luck is never good...

It would be much better to separate the Command definition part of the 
CommandObject from the Invocation part, so that one command definition could 
server multiple invocations.  If somebody has ambition to dig into lldb's 
command interpreter, I'd propose this task over replacing something that works 
with something else that works more nicely.

Jim


> On Feb 12, 2018, at 2:31 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> There are several components in the "command line parsing"
> 
> 1. argparse: parsing a single string into the individual words (processing 
> quotes, backslashes, etc.)
> 2. resolving the "command" part of the command line (the "frame variable" 
> part of "frame variable --raw-output FOO")
> 3. resolving the arguments of a command (the "--raw-output FOO" part)
> 
> These parts compose quite nicely, and can be individually replaced. I'll 
> briefly speak about each one :
> 
> (1) is very specific to our interactive use case (for command line utilities, 
> the shell/os does the parsing for us). There also isn't anything nearly 
> similar in llvm (afaik).
> (2) is also quite specific to how lldb commands are structured, and there 
> also isn't a suitable llvm replacement. This is the level at which the 
> "command aliases" are resolved.
> (3) is a fairly generic piece of functionality -- we even implement it by 
> delegating to getopt (in a scarily un-thread-safe way). For this there are 
> *two* competing implementations in llvm: `llvm::cl` and `llvm::opt`. Of these 
> the first one is not suitable for our use case right now as it relies on 
> global variables. Making it work probably would be a bit or work. However, 
> using the second one should be fairly easy, and it should be generic-enough 
> (it's the thing that handles crazy gcc command lines).
> 
> The reason I'm spelling it out this way is because I think you were mostly 
> talking about (3), but some of the difficulties Jim pointed out (command 
> aliases) just aren't relevant there.  I don't think llvm::opt handles 
> "shortest match" options yet, but that should be fairly easy to add.  On the 
> flip side, the library has recently gained argument suggestion capabilities 
> (`error: unknown argument '-gllbb', did you mean '-glldb'`), so we would get 
> that for free. Making sure tab completion works there would require a bit of 
> thought, but I think this should not interfere with this library too much 
> (after all, we use getopt now, and it does not support tab completion either).
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D43099
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

There are several components in the "command line parsing"

1. argparse: parsing a single string into the individual words (processing 
quotes, backslashes, etc.)
2. resolving the "command" part of the command line (the "frame variable" part 
of "frame variable --raw-output FOO")
3. resolving the arguments of a command (the "--raw-output FOO" part)

These parts compose quite nicely, and can be individually replaced. I'll 
briefly speak about each one :

(1) is very specific to our interactive use case (for command line utilities, 
the shell/os does the parsing for us). There also isn't anything nearly similar 
in llvm (afaik).
(2) is also quite specific to how lldb commands are structured, and there also 
isn't a suitable llvm replacement. This is the level at which the "command 
aliases" are resolved.
(3) is a fairly generic piece of functionality -- we even implement it by 
delegating to getopt (in a scarily un-thread-safe way). For this there are 
*two* competing implementations in llvm: `llvm::cl` and `llvm::opt`. Of these 
the first one is not suitable for our use case right now as it relies on global 
variables. Making it work probably would be a bit or work. However, using the 
second one should be fairly easy, and it should be generic-enough (it's the 
thing that handles crazy gcc command lines).

The reason I'm spelling it out this way is because I think you were mostly 
talking about (3), but some of the difficulties Jim pointed out (command 
aliases) just aren't relevant there.  I don't think llvm::opt handles "shortest 
match" options yet, but that should be fairly easy to add.  On the flip side, 
the library has recently gained argument suggestion capabilities (`error: 
unknown argument '-gllbb', did you mean '-glldb'`), so we would get that for 
free. Making sure tab completion works there would require a bit of thought, 
but I think this should not interfere with this library too much (after all, we 
use getopt now, and it does not support tab completion either).


Repository:
  rL LLVM

https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Phabricator via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL324775: Make LLDBs clang module cache path 
customizable (authored by adrian, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43099?vs=133636=133684#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43099

Files:
  lldb/trunk/include/lldb/Target/Target.h
  lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
  
lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py
  lldb/trunk/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  lldb/trunk/source/Target/Target.cpp

Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
@@ -23,6 +23,7 @@
   LINK_LIBS
 clangAST
 clangCodeGen
+clangDriver
 clangEdit
 clangFrontend
 clangLex
Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -13,6 +13,7 @@
 
 // Other libraries and framework includes
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
@@ -590,14 +591,12 @@
   // Add additional search paths with { "-I", path } or { "-F", path } here.
 
   {
-llvm::SmallString<128> DefaultModuleCache;
-const bool erased_on_reboot = false;
-llvm::sys::path::system_temp_directory(erased_on_reboot,
-   DefaultModuleCache);
-llvm::sys::path::append(DefaultModuleCache, "org.llvm.clang");
-llvm::sys::path::append(DefaultModuleCache, "ModuleCache");
+llvm::SmallString<128> Path;
+target.GetClangModulesCachePath().GetPath(Path);
+if (Path.empty())
+  clang::driver::Driver::getDefaultModuleCachePath(Path);
 std::string module_cache_argument("-fmodules-cache-path=");
-module_cache_argument.append(DefaultModuleCache.str().str());
+module_cache_argument.append(Path.str());
 compiler_invocation_arguments.push_back(module_cache_argument);
   }
 
Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -3509,6 +3509,9 @@
  OptionValue::eTypeString, nullptr, nullptr,
  "A list of trap handler function names, e.g. a common Unix user process "
  "one is _sigtramp."},
+{"clang-modules-cache-path",
+ OptionValue::eTypeFileSpec, false, 0, nullptr, nullptr,
+ "The path to the clang modules cache directory (-fmodules-cache-path)."},
 {"display-runtime-support-values", OptionValue::eTypeBoolean, false, false,
  nullptr, nullptr, "If true, LLDB will show variables that are meant to "
"support the operation of a language's runtime "
@@ -3558,6 +3561,7 @@
   ePropertyMemoryModuleLoadLevel,
   ePropertyDisplayExpressionsInCrashlogs,
   ePropertyTrapHandlerNames,
+  ePropertyClangModulesCachePath,
   ePropertyDisplayRuntimeSupportValues,
   ePropertyNonStopModeEnabled,
   ePropertyExperimental
@@ -3937,6 +3941,15 @@
   return option_value->GetCurrentValue();
 }
 
+FileSpec ::GetClangModulesCachePath() {
+  const uint32_t idx = ePropertyClangModulesCachePath;
+  OptionValueFileSpec *option_value =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
+   idx);
+  assert(option_value);
+  return option_value->GetCurrentValue();
+}
+
 FileSpecList ::GetClangModuleSearchPaths() {
   const uint32_t idx = ePropertyClangModuleSearchPaths;
   OptionValueFileSpecList *option_value =
Index: lldb/trunk/include/lldb/Target/Target.h
===
--- lldb/trunk/include/lldb/Target/Target.h
+++ lldb/trunk/include/lldb/Target/Target.h
@@ -126,6 +126,8 @@
 
   FileSpecList ();
 
+  FileSpec ();
+
   FileSpecList ();
 
   bool GetEnableAutoImportClangModules() const;
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -1914,6 +1914,14 @@
 # decorators.
 Base.setUp(self)
 
+# Set the clang modules cache path.
+if self.child:
+assert(self.getDebugInfo() == 'default')
+  

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

To me, the determinant here is whether the llvm project sees many more 
interactive tools in its future.  If lldb is the only one, then moving all this 
functionality to llvm seems like effort better spent elsewhere.  But if there 
are other interactive tools in the offing, it would be great to have all such 
tools have a common behavior, and so there's good motivation for doing this 
work.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

It's probably possible to make it work, but as Jim said, there's no drop in 
replacement currently.  There's bits and pieces of stuff that, with a dedicated 
effort, could be improved to the point of being sufficient, though.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I don't think there's any battle.  I wouldn't mind moving the command parsing 
code into llvm if there's a suitable replacement.

But so far as I can see llvm's command line parsing was primarily intended for 
parsing args for shell tools.  So it lacks features required for a replacement 
to lldb's command line parser.

The lldb interactive command interpreter absolutely needs completion callbacks. 
 Nobody would be happy with lldb if it didn't complete filenames and symbols 
and the commands themselves.  We also do shortest match rather than exact 
matches which folks find very handy.  And you have to be able to provide our 
alias system which is also very heavily used.  If somebody wants to add all 
these features to the llvm command line parser, then we could use it in lldb.  
I'm not sure that that would be a great idea, it might be seen to overly 
complicate the shell tool only parser.

Anyway, I don't see a drop-in replacement in llvm.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Davide Italiano via Phabricator via lldb-commits]

davide added inline comments.



Comment at: source/Target/Target.cpp:3949
+   idx);
+  assert(option_value);
+  return option_value->GetCurrentValue();

aprantl wrote:
> davide wrote:
> > add an assertion message if you don't mind?
> This code is mechanically duplicated from the other functions in this file, 
> and I was actually planning on making another pass that auto-generates all of 
> the properties and the accessor functions from a .inc file using an 
> llvm-style HANDLE_foo() macro.
Sure, even better :) It would be awesome if lldb didn't do its own command line 
handling but would use something like `cl::opt`, but I guess that's a battle 
for another day.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Target/Target.cpp:3949
+   idx);
+  assert(option_value);
+  return option_value->GetCurrentValue();

davide wrote:
> add an assertion message if you don't mind?
This code is mechanically duplicated from the other functions in this file, and 
I was actually planning on making another pass that auto-generates all of the 
properties and the accessor functions from a .inc file using an llvm-style 
HANDLE_foo() macro.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

This looks fine to me.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Davide Italiano via Phabricator via lldb-commits]

davide accepted this revision.
davide added a comment.

LGTM modulo minor.




Comment at: source/Plugins/ExpressionParser/Clang/CMakeLists.txt:26
 clangCodeGen
+clangDriver
 clangEdit

aprantl wrote:
> I checked and this does not affects LLDB's binary size in any measurable way.
This doesn't surprise me, as the driver is very small.



Comment at: source/Target/Target.cpp:3949
+   idx);
+  assert(option_value);
+  return option_value->GetCurrentValue();

add an assertion message if you don't mind?


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: source/Plugins/ExpressionParser/Clang/CMakeLists.txt:26
 clangCodeGen
+clangDriver
 clangEdit

I checked and this does not affects LLDB's binary size in any measurable way.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: 
source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:595-601
+if (Path.empty()) {
+  // This code is copied from the Clang driver.
+  const bool erased_on_reboot = false;
+  llvm::sys::path::system_temp_directory(erased_on_reboot, Path);
+  llvm::sys::path::append(Path, "org.llvm.clang");
+  llvm::sys::path::append(Path, "ModuleCache");
+}

jingham wrote:
> Is there a reason not to have GetClangModulesCachePath do this?  This is 
> roughly the "default value" of the value.  Is there a good reason to make 
> clients compute that?
> 
> I presume you are computing this here because clang doesn't offer an API to 
> do that?
> Is there a reason not to have GetClangModulesCachePath do this?
Yes. It is supposed to return the value of the *property*. Also, I assume that 
Target doesn't necessarily link against Clang, so calling into a Clang API 
there seems to be a layering violation.


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Adrian Prantl via Phabricator via lldb-commits]

aprantl updated this revision to Diff 133636.
aprantl added a comment.
Herald added subscribers: hintonda, mgorny.

Address most review feedback.


https://reviews.llvm.org/D43099

Files:
  include/lldb/Target/Target.h
  packages/Python/lldbsuite/test/lldbtest.py
  source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -3509,6 +3509,9 @@
  OptionValue::eTypeString, nullptr, nullptr,
  "A list of trap handler function names, e.g. a common Unix user process "
  "one is _sigtramp."},
+{"clang-modules-cache-path",
+ OptionValue::eTypeFileSpec, false, 0, nullptr, nullptr,
+ "The path to the clang modules cache directory (-fmodules-cache-path)."},
 {"display-runtime-support-values", OptionValue::eTypeBoolean, false, false,
  nullptr, nullptr, "If true, LLDB will show variables that are meant to "
"support the operation of a language's runtime "
@@ -3558,6 +3561,7 @@
   ePropertyMemoryModuleLoadLevel,
   ePropertyDisplayExpressionsInCrashlogs,
   ePropertyTrapHandlerNames,
+  ePropertyClangModulesCachePath,
   ePropertyDisplayRuntimeSupportValues,
   ePropertyNonStopModeEnabled,
   ePropertyExperimental
@@ -3937,6 +3941,15 @@
   return option_value->GetCurrentValue();
 }
 
+FileSpec ::GetClangModulesCachePath() {
+  const uint32_t idx = ePropertyClangModulesCachePath;
+  OptionValueFileSpec *option_value =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
+   idx);
+  assert(option_value);
+  return option_value->GetCurrentValue();
+}
+
 FileSpecList ::GetClangModuleSearchPaths() {
   const uint32_t idx = ePropertyClangModuleSearchPaths;
   OptionValueFileSpecList *option_value =
Index: source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -13,6 +13,7 @@
 
 // Other libraries and framework includes
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Driver/Driver.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
@@ -590,14 +591,12 @@
   // Add additional search paths with { "-I", path } or { "-F", path } here.
 
   {
-llvm::SmallString<128> DefaultModuleCache;
-const bool erased_on_reboot = false;
-llvm::sys::path::system_temp_directory(erased_on_reboot,
-   DefaultModuleCache);
-llvm::sys::path::append(DefaultModuleCache, "org.llvm.clang");
-llvm::sys::path::append(DefaultModuleCache, "ModuleCache");
+llvm::SmallString<128> Path;
+target.GetClangModulesCachePath().GetPath(Path);
+if (Path.empty())
+  clang::driver::Driver::getDefaultModuleCachePath(Path);
 std::string module_cache_argument("-fmodules-cache-path=");
-module_cache_argument.append(DefaultModuleCache.str().str());
+module_cache_argument.append(Path.str());
 compiler_invocation_arguments.push_back(module_cache_argument);
   }
 
Index: source/Plugins/ExpressionParser/Clang/CMakeLists.txt
===
--- source/Plugins/ExpressionParser/Clang/CMakeLists.txt
+++ source/Plugins/ExpressionParser/Clang/CMakeLists.txt
@@ -23,6 +23,7 @@
   LINK_LIBS
 clangAST
 clangCodeGen
+clangDriver
 clangEdit
 clangFrontend
 clangLex
Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -859,6 +859,10 @@
 self.darwinWithFramework = False
 self.makeBuildDir()
 
+# set the clang modules cache path.
+mod_cache = os.path.join(self.getBuildDir(), "module-cache")
+self.runCmd("settings set target.clang-modules-cache-path " + mod_cache)
+
 def setAsync(self, value):
 """ Sets async mode to True/False and ensures it is reset after the testcase completes."""
 old_async = self.dbg.GetAsync()
Index: include/lldb/Target/Target.h
===
--- include/lldb/Target/Target.h
+++ include/lldb/Target/Target.h
@@ -126,6 +126,8 @@
 
   FileSpecList ();
 
+  FileSpec ();
+
   FileSpecList ();
 
   bool GetEnableAutoImportClangModules() const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Jim Ingham via Phabricator via lldb-commits]

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

Think about whether it would be better to have GetClangModulesCachePath 
calculate the fallback modules path rather than having the client do it?




Comment at: 
source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:595-601
+if (Path.empty()) {
+  // This code is copied from the Clang driver.
+  const bool erased_on_reboot = false;
+  llvm::sys::path::system_temp_directory(erased_on_reboot, Path);
+  llvm::sys::path::append(Path, "org.llvm.clang");
+  llvm::sys::path::append(Path, "ModuleCache");
+}

Is there a reason not to have GetClangModulesCachePath do this?  This is 
roughly the "default value" of the value.  Is there a good reason to make 
clients compute that?

I presume you are computing this here because clang doesn't offer an API to do 
that?


https://reviews.llvm.org/D43099



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added a reviewer: jingham.

This patch makes LLDB's clang module cache path customizable via `settings set 
target.clang-modules-cache-path ` and uses it in the LLDB testsuite to 
reuse the same location inside the build directory for LLDB and clang.


https://reviews.llvm.org/D43099

Files:
  include/lldb/Target/Target.h
  packages/Python/lldbsuite/test/lldbtest.py
  source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
  source/Target/Target.cpp


Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -3509,6 +3509,9 @@
  OptionValue::eTypeString, nullptr, nullptr,
  "A list of trap handler function names, e.g. a common Unix user process "
  "one is _sigtramp."},
+{"clang-modules-cache-path",
+ OptionValue::eTypeFileSpec, false, 0, nullptr, nullptr,
+ "The path to the clang modules cache directory (-fmodules-cache-path)."},
 {"display-runtime-support-values", OptionValue::eTypeBoolean, false, false,
  nullptr, nullptr, "If true, LLDB will show variables that are meant to "
"support the operation of a language's runtime "
@@ -3558,6 +3561,7 @@
   ePropertyMemoryModuleLoadLevel,
   ePropertyDisplayExpressionsInCrashlogs,
   ePropertyTrapHandlerNames,
+  ePropertyClangModulesCachePath,
   ePropertyDisplayRuntimeSupportValues,
   ePropertyNonStopModeEnabled,
   ePropertyExperimental
@@ -3937,6 +3941,15 @@
   return option_value->GetCurrentValue();
 }
 
+FileSpec ::GetClangModulesCachePath() {
+  const uint32_t idx = ePropertyClangModulesCachePath;
+  OptionValueFileSpec *option_value =
+  m_collection_sp->GetPropertyAtIndexAsOptionValueFileSpec(nullptr, false,
+   idx);
+  assert(option_value);
+  return option_value->GetCurrentValue();
+}
+
 FileSpecList ::GetClangModuleSearchPaths() {
   const uint32_t idx = ePropertyClangModuleSearchPaths;
   OptionValueFileSpecList *option_value =
Index: source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -590,14 +590,17 @@
   // Add additional search paths with { "-I", path } or { "-F", path } here.
 
   {
-llvm::SmallString<128> DefaultModuleCache;
-const bool erased_on_reboot = false;
-llvm::sys::path::system_temp_directory(erased_on_reboot,
-   DefaultModuleCache);
-llvm::sys::path::append(DefaultModuleCache, "org.llvm.clang");
-llvm::sys::path::append(DefaultModuleCache, "ModuleCache");
+llvm::SmallString<128> Path;
+target.GetClangModulesCachePath().GetPath(Path);
+if (Path.empty()) {
+  // This code is copied from the Clang driver.
+  const bool erased_on_reboot = false;
+  llvm::sys::path::system_temp_directory(erased_on_reboot, Path);
+  llvm::sys::path::append(Path, "org.llvm.clang");
+  llvm::sys::path::append(Path, "ModuleCache");
+}
 std::string module_cache_argument("-fmodules-cache-path=");
-module_cache_argument.append(DefaultModuleCache.str().str());
+module_cache_argument.append(Path.str());
 compiler_invocation_arguments.push_back(module_cache_argument);
   }
 
Index: packages/Python/lldbsuite/test/lldbtest.py
===
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -859,6 +859,10 @@
 self.darwinWithFramework = False
 self.makeBuildDir()
 
+# set the clang modules cache path.
+mod_cache = os.path.join(self.getBuildDir(), "module-cache")
+self.runCmd("settings set target.clang-modules-cache-path " + 
mod_cache)
+
 def setAsync(self, value):
 """ Sets async mode to True/False and ensures it is reset after the 
testcase completes."""
 old_async = self.dbg.GetAsync()
Index: include/lldb/Target/Target.h
===
--- include/lldb/Target/Target.h
+++ include/lldb/Target/Target.h
@@ -126,6 +126,8 @@
 
   FileSpecList ();
 
+  FileSpec ();
+
   FileSpecList ();
 
   bool GetEnableAutoImportClangModules() const;


Index: source/Target/Target.cpp
===
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -3509,6 +3509,9 @@
  OptionValue::eTypeString, nullptr, nullptr,
  "A list of trap handler function names, e.g. a common Unix user process "
  "one is _sigtramp."},
+{"clang-modules-cache-path",
+ OptionValue::eTypeFileSpec, false, 0, nullptr, nullptr,
+ "The path to the clang modules cache directory (-fmodules-cache-path)."},