[Lldb-commits] [PATCH] D66546: Extend FindTypes w/ CompilerContext to allow filtering by language

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Looks good overall. Just a question of it we want to return "const LanguageSet &" to avoid copies. Also switch static functions that currently return "LanguageSet" over to use static variables and llvm::once to init them and then return "const LanguageSet &".



================
Comment at: lldb/include/lldb/Target/Language.h:269-271
+  static LanguageSet GetLanguagesSupportingTypeSystems();
+  static LanguageSet GetLanguagesSupportingTypeSystemsForExpressions();
+  static LanguageSet GetLanguagesSupportingREPLs();
----------------
return a "const LanguageSet &" to avoid copies?


================
Comment at: lldb/source/Core/Debugger.cpp:1626
   if (language == eLanguageTypeUnknown) {
-    std::set<LanguageType> repl_languages;
+    LanguageSet repl_languages = Language::GetLanguagesSupportingREPLs();
 
----------------
"const LanguageSet &" to avoid copies?


================
Comment at: lldb/source/Core/PluginManager.cpp:2172
+  for (unsigned i = 0; i < instances.size(); ++i)
+    all.bitvector |= instances[i].supported_languages_for_types.bitvector;
+  return all;
----------------
Add a LanguageSet method to do this? We are playing with internals here. Maybe:
```
all.Merge(instances[i]);
```


================
Comment at: lldb/source/Core/PluginManager.cpp:2265-2266
+LanguageSet PluginManager::GetREPLAllTypeSystemSupportedLanguages() {
   std::lock_guard<std::recursive_mutex> guard(GetREPLMutex());
+  LanguageSet all;
   REPLInstances &instances = GetREPLInstances();
----------------
This function is static. Should we return a "const LanguageSet &" here? Also use std::once/llvm::once?:

```
static LanguageSet g_langs;
std::once once(...) {
  ... do work to populate
});
return g_langs;
```



================
Comment at: lldb/source/Interpreter/OptionValueLanguage.cpp:43
     ConstString lang_name(value.trim());
-    std::set<lldb::LanguageType> languages_for_types;
-    std::set<lldb::LanguageType> languages_for_expressions;
-    Language::GetLanguagesSupportingTypeSystems(languages_for_types,
-                                                languages_for_expressions);
-
+    LanguageSet languages_for_types = Language::GetLanguagesSupportingTypeSystems();
     LanguageType new_type =
----------------
"const LanguageSet &"?


================
Comment at: lldb/source/Symbol/ClangASTContext.cpp:737
 
-void ClangASTContext::EnumerateSupportedLanguages(
-    std::set<lldb::LanguageType> &languages_for_types,
-    std::set<lldb::LanguageType> &languages_for_expressions) {
-  static std::vector<lldb::LanguageType> s_supported_languages_for_types(
-      {lldb::eLanguageTypeC89, lldb::eLanguageTypeC, lldb::eLanguageTypeC11,
-       lldb::eLanguageTypeC_plus_plus, lldb::eLanguageTypeC99,
-       lldb::eLanguageTypeObjC, lldb::eLanguageTypeObjC_plus_plus,
-       lldb::eLanguageTypeC_plus_plus_03, lldb::eLanguageTypeC_plus_plus_11,
-       lldb::eLanguageTypeC11, lldb::eLanguageTypeC_plus_plus_14});
-
-  static std::vector<lldb::LanguageType> s_supported_languages_for_expressions(
-      {lldb::eLanguageTypeC_plus_plus, lldb::eLanguageTypeObjC_plus_plus,
-       lldb::eLanguageTypeC_plus_plus_03, lldb::eLanguageTypeC_plus_plus_11,
-       lldb::eLanguageTypeC_plus_plus_14});
-
-  languages_for_types.insert(s_supported_languages_for_types.begin(),
-                             s_supported_languages_for_types.end());
-  languages_for_expressions.insert(
-      s_supported_languages_for_expressions.begin(),
-      s_supported_languages_for_expressions.end());
+LanguageSet ClangASTContext::GetSupportedLanguagesForTypes() {
+  LanguageSet languages;
----------------
return "const LanguageSet &"? Then make "static LanguageSet g_languages;" and use llvm::once to control one time init?


================
Comment at: lldb/source/Symbol/ClangASTContext.cpp:754
+LanguageSet ClangASTContext::GetSupportedLanguagesForExpressions() {
+  LanguageSet languages;
+  languages.Insert(lldb::eLanguageTypeC_plus_plus);
----------------
return "const LanguageSet &"? Then make "static LanguageSet g_languages;" and use llvm::once to control one time init?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66546/new/

https://reviews.llvm.org/D66546