[lldb] Fix that symbols.clang-modules-cache-path is never initialized

LLDB is supposed to ask the Clang Driver what the default module cache path is
and then use that value as the default for the
`symbols.clang-modules-cache-path` setting. However, we use the property type
`String` to change `symbols.clang-modules-cache-path` even though the type of
that setting is `FileSpec`, so the setter will simply do nothing and return
`false`. We also don't check the return value of the setter, so this whole code
ends up not doing anything at all.

This changes the setter to use the correct property type and adds an assert that
we actually successfully set the default path. Also adds a test that checks that
the default value for this setting is never unset/empty path as this would
effectively disable the import-std-module feature from working by default.

Reviewed By: JDevlieghere, shafik

Differential Revision: https://reviews.llvm.org/D92772

lldb/include/lldb/Core/ModuleList.h

@@ -56,7 +56,7 @@ public:
   ModuleListProperties();
 
   FileSpec GetClangModulesCachePath() const;
-  bool SetClangModulesCachePath(llvm::StringRef path);
+  bool SetClangModulesCachePath(const FileSpec &path);
   bool GetEnableExternalLookup() const;
   bool SetEnableExternalLookup(bool new_value);
 

lldb/source/Core/ModuleList.cpp

@@ -82,8 +82,9 @@ ModuleListProperties::ModuleListProperties() {
                                            [this] { UpdateSymlinkMappings(); });
 
   llvm::SmallString<128> path;
-  clang::driver::Driver::getDefaultModuleCachePath(path);
-  SetClangModulesCachePath(path);
+  if (clang::driver::Driver::getDefaultModuleCachePath(path)) {
+    lldbassert(SetClangModulesCachePath(FileSpec(path)));
+  }
 }
 
 bool ModuleListProperties::GetEnableExternalLookup() const {
@@ -104,8 +105,8 @@ FileSpec ModuleListProperties::GetClangModulesCachePath() const {
       ->GetCurrentValue();
 }
 
-bool ModuleListProperties::SetClangModulesCachePath(llvm::StringRef path) {
-  return m_collection_sp->SetPropertyAtIndexAsString(
+bool ModuleListProperties::SetClangModulesCachePath(const FileSpec &path) {
+  return m_collection_sp->SetPropertyAtIndexAsFileSpec(
       nullptr, ePropertyClangModulesCachePath, path);
 }
 

lldb/test/Shell/Settings/TestDefaultModuleCachePath.test

@@ -0,0 +1,9 @@
+# RUN: %lldb-noinit -x -s %s | FileCheck %s
+settings show symbols.clang-modules-cache-path
+q
+# This test checks that we get *any* clang modules cache path by default. The
+# actual path depends on the operating system.
+# CHECK: symbols.clang-modules-cache-path (file) = "
+# Clang treats an empty path in the same way as 'no path', so explicitly check
+# that we never have an empty path by default.
+# CHECK-NOT: symbols.clang-modules-cache-path (file) = ""

lldb/test/Shell/helper/toolchain.py

@@ -54,6 +54,10 @@ def use_lldb_substitutions(config):
                   command=FindTool('lldb'),
                   extra_args=['-S', lldb_init],
                   unresolved='fatal'),
+        ToolSubst('%lldb-noinit',
+                  command=FindTool('lldb'),
+                  extra_args=['--no-lldbinit'],
+                  unresolved='fatal'),
         ToolSubst('%lldb-server',
                   command=FindTool("lldb-server"),
                   extra_args=[],