Author: Raphael Isemann Date: 2020-12-10T13:37:40+01:00 New Revision: 208e3f5d9b6c172f65dbb9cdbc9354c81c6d8911
URL: https://github.com/llvm/llvm-project/commit/208e3f5d9b6c172f65dbb9cdbc9354c81c6d8911 DIFF: https://github.com/llvm/llvm-project/commit/208e3f5d9b6c172f65dbb9cdbc9354c81c6d8911.diff LOG: [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 Added: lldb/test/Shell/Settings/TestDefaultModuleCachePath.test Modified: lldb/include/lldb/Core/ModuleList.h lldb/source/Core/ModuleList.cpp lldb/test/Shell/helper/toolchain.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h index d90b27e474ac..1609f0f77c56 100644 --- a/lldb/include/lldb/Core/ModuleList.h +++ b/lldb/include/lldb/Core/ModuleList.h @@ -56,7 +56,7 @@ class ModuleListProperties : public Properties { ModuleListProperties(); FileSpec GetClangModulesCachePath() const; - bool SetClangModulesCachePath(llvm::StringRef path); + bool SetClangModulesCachePath(const FileSpec &path); bool GetEnableExternalLookup() const; bool SetEnableExternalLookup(bool new_value); diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp index cf276ba65931..b6e1ceb40889 100644 --- a/lldb/source/Core/ModuleList.cpp +++ b/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); } diff --git a/lldb/test/Shell/Settings/TestDefaultModuleCachePath.test b/lldb/test/Shell/Settings/TestDefaultModuleCachePath.test new file mode 100644 index 000000000000..80dc0f25ce58 --- /dev/null +++ b/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) = "" diff --git a/lldb/test/Shell/helper/toolchain.py b/lldb/test/Shell/helper/toolchain.py index ebf9e03d81a0..59b078411c1c 100644 --- a/lldb/test/Shell/helper/toolchain.py +++ b/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=[], _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
