[lldb] Filter duplicates in Target::GetScratchTypeSystems

[teemperor]

Summary

Target::GetScratchTypeSystems returns the list of scratch TypeSystems. The
current implementation is iterating over all LanguageType values and retrieves
the respective TypeSystem for each LanguageType.

All C/C++/Obj-C LanguageTypes are however mapped to the same
ScratchTypeSystemClang instance, so the current implementation adds this
single TypeSystem instance several times to the list of TypeSystems (once
for every LanguageType that we support).

The only observable effect of this is that SBTarget.FindTypes for builtin
types currently queries the ScratchTypeSystemClang several times (and also
adds the same result several times).

[labath]

lldb/source/Target/Target.cpp

2238

This might be a good use case for a "sorted vector" https://llvm.org/docs/ProgrammersManual.html#a-sorted-vector

[teemperor]

• Use std::vector -> sort -> unique

[teemperor]

Eh wait, that's not correct. We can't sort this as there is no ordering for TypeSystems and if we sort by pointer value then this is all non-deterministic. I'll revert back to the original revision.

[teemperor]

• Back to SetVector

[bulbazord]

Makes sense to me!

[labath]

In D111931#3070198, @teemperor wrote:

Eh wait, that's not correct. We can't sort this as there is no ordering for TypeSystems and if we sort by pointer value then this is all non-deterministic. I'll revert back to the original revision.

Ah, sorry for leading you into a dead end.

[teemperor]

In D111931#3072069, @labath wrote:

In D111931#3070198, @teemperor wrote:

Eh wait, that's not correct. We can't sort this as there is no ordering for TypeSystems and if we sort by pointer value then this is all non-deterministic. I'll revert back to the original revision.

Ah, sorry for leading you into a dead end.

No worries, I just noticed it after I updated the review. Just wanted to elaborate why I'm reverting back to the original implementation.

lldb/source/Target/Target.cpp

 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/Timer.h"
 
 #include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SetVector.h"
 
 #include <memory>
 #include <mutex>
 
 using namespace lldb;
 using namespace lldb_private;
 
 constexpr std::chrono::milliseconds EvaluateExpressionOptions::default_timeout;
Target::GetScratchTypeSystemForLanguage(lldb::LanguageType language,
   return m_scratch_type_system_map.GetTypeSystemForLanguage(language, this,
                                                             create_on_demand);
 }
 
 std::vector<TypeSystem *> Target::GetScratchTypeSystems(bool create_on_demand) {
   if (!m_valid)
     return {};
 
-  std::vector<TypeSystem *> scratch_type_systems;
+  // Some TypeSystem instances are associated with several LanguageTypes so
+  // they will show up several times in the loop below. The SetVector filters
+  // out all duplicates as they serve no use for the caller.
+  llvm::SetVector<TypeSystem *> scratch_type_systems;
 
   LanguageSet languages_for_expressions =
       Language::GetLanguagesSupportingTypeSystemsForExpressions();
 
   for (auto bit : languages_for_expressions.bitvector.set_bits()) {
     auto language = (LanguageType)bit;
     auto type_system_or_err =
         GetScratchTypeSystemForLanguage(language, create_on_demand);
     if (!type_system_or_err)
       LLDB_LOG_ERROR(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_TARGET),
                      type_system_or_err.takeError(),
                      "Language '{}' has expression support but no scratch type "
                      "system available",
                      Language::GetNameForLanguageType(language));
     else
-      scratch_type_systems.emplace_back(&type_system_or_err.get());
+      scratch_type_systems.insert(&type_system_or_err.get());
   }
 
-  return scratch_type_systems;
+  return scratch_type_systems.takeVector();
 }
 
 PersistentExpressionState *
 Target::GetPersistentExpressionStateForLanguage(lldb::LanguageType language) {
   auto type_system_or_err = GetScratchTypeSystemForLanguage(language, true);
 
   if (auto err = type_system_or_err.takeError()) {
     LLDB_LOG_ERROR(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_TARGET),

lldb/test/API/lang/c/builtin-types/TestCBuiltinTypes.py

+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @no_debug_info_test
+    def test_FindTypes_on_scratch_AST(self):
+        """
+        Tests FindTypes invoked with only LLDB's scratch AST present.
+        """
+        target = self.dbg.GetDummyTarget()
+        # There should be only one instance of 'unsigned long' in our single
+        # scratch AST. Note: FindTypes/SBType hahave no filter by language, so
+        # pick something that is unlikely to also be found in the scratch
+        # TypeSystem of other language plugins.
+        self.assertEqual(len(target.FindTypes("unsigned long")), 1)