Make the FindTypes(std::vector<CompilerContext>, ...) API testable in lldb-test

[aprantl]

Summary

I'm planning to modify this API shortly and thought best to first make it independently testable.

This adds a -compiler-context=<...> option to lldb-test that translates a JSON-formatted string that is a list of kind/name pairs and translates it into a std::vector<CompilerContext>, a CompilerContext being a pair of context-kind and name.

[JDevlieghere]

lldb/tools/lldb-test/lldb-test.cpp

145 ↗

(On Diff #216020)

...as a JSON array...`

This makes me wonder though, wouldn't it be better to pass a file here and have it live in inputs? Do you expect it to be different across tests or could we reuse the context across tests?

[aprantl]

lldb/tools/lldb-test/lldb-test.cpp

145 ↗

(On Diff #216020)

I don't really expect a DeclContexts to have more than 3 entries in the forseeable future, so that would arguable just make the test harder to read.

[JDevlieghere]

lldb/tools/lldb-test/lldb-test.cpp

145 ↗

(On Diff #216020)

But would those entries be the same across tests or not? If they're the same I think it'd be better to reuse them, even if it's only a few entries.

[labath]

I am glad you're finding lldb-test useful. Please see my inline comment about quoting.

lldb/lit/SymbolFile/DWARF/compilercontext.ll

1 ↗

(On Diff #216020)

We generally put test data into an Inputs subfolder. Though in this case, it looks like you could actually inline the data into the test file below. (Feel free to add .ll into the list of test suffixes.)

lldb/tools/lldb-test/lldb-test.cpp

145 ↗

(On Diff #216020)

I actually think it is nice to have the entries present in the test file directly. However, what I think will be an issue is that you need to use two layers of quotes to pass a json string over the command line. I expect this will cause problems on windows, because quoting works differently there.

What I would actually suggest is to ditch JSON, and use a custom format for passing this. Maybe something like --compiler-context Module:CModule,Module:SubModule,.... That would be both shorter and completely avoid quotes. I don't think is should make parsing this signtificantly more difficult than what you have now.

[aprantl]

Address Pavel's comments.

[aprantl]

lldb/tools/lldb-test/lldb-test.cpp

145 ↗

(On Diff #216020)

But would those entries be the same across tests or not? If they're the same I think it'd be better to reuse them, even if it's only a few entries.

No, they would be highly specific to the test. A context is a search criterium, something like "any class type called Foo nested inside a module called A".

[labath]

lgtm

lldb/tools/lldb-test/lldb-test.cpp

33 ↗

(On Diff #216174)

I guess this is not needed now.

[JDevlieghere]

lldb/tools/lldb-test/lldb-test.cpp

145 ↗

(On Diff #216020)

Okay, in that case passing them on the command line is definitely the way to go.

lldb/trunk/lit/SymbolFile/DWARF/compilercontext.ll

+; Test finding types by CompilerContext.
+; RUN: llc %s -filetype=obj -o %t.o
+; RUN: lldb-test symbols %t.o -find=type -compiler-context="Module:CModule,Module:SubModule,Structure:FromSubmodule" | FileCheck %s
+;
+;
+; CHECK: Found 1 types:
+; CHECK: struct FromSubmodule {
+; CHECK-NEXT:     unsigned int x;
+; CHECK-NEXT:     unsigned int y;
+; CHECK-NEXT:     unsigned int z;
+; CHECK-NEXT: }
+
+source_filename = "/t.c"
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.14.0"
+
+!llvm.dbg.cu = !{!2}
+!llvm.linker.options = !{}
+!llvm.module.flags = !{!18, !19}
+!llvm.ident = !{!22}
+
+; This simulates the debug info for a Clang module.
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, nameTableKind: GNU, retainedTypes: !{!11})
+!3 = !DIFile(filename: "t.c", directory: "/")
+!8 = !DIModule(scope: !9, name: "SubModule", includePath: "", isysroot: "/")
+!9 = !DIModule(scope: null, name: "CModule", includePath: "", isysroot: "/")
+!11 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "FromSubmodule", scope: !8, file: !3, line: 1, size: 96, elements: !13)
+!13 = !{!14, !16, !17}
+!14 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !11, file: !3, line: 2, baseType: !15, size: 32)
+!15 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
+!16 = !DIDerivedType(tag: DW_TAG_member, name: "y", scope: !11, file: !3, line: 2, baseType: !15, size: 32, offset: 32)
+!17 = !DIDerivedType(tag: DW_TAG_member, name: "z", scope: !11, file: !3, line: 2, baseType: !15, size: 32, offset: 64)
+!18 = !{i32 2, !"Dwarf Version", i32 4}
+!19 = !{i32 2, !"Debug Info Version", i32 3}
+!22 = !{!"clang version 10.0.0 (https://github.com/llvm/llvm-project 056f1b5cc7c2133f0cb3e30e7f24808d321096d7)"}

lldb/trunk/lit/SymbolFile/DWARF/lit.local.cfg

-config.suffixes = ['.cpp', '.s', '.test']
+config.suffixes = ['.cpp', '.s', '.test', '.ll']

lldb/trunk/tools/lldb-test/lldb-test.cpp

Regex("regex",
           cl::desc("Search using regular expressions (avaliable for variables "
                    "and functions only)."),
           cl::sub(SymbolsSubcommand));
 static cl::opt<std::string>
     Context("context",
             cl::desc("Restrict search to the context of the given variable."),
             cl::value_desc("variable"), cl::sub(SymbolsSubcommand));
 
+static cl::opt<std::string> CompilerContext(
+    "compiler-context",
+    cl::desc("Specify a compiler context as \"kind:name,...\"."),
+    cl::value_desc("context"), cl::sub(SymbolsSubcommand));
+
 static cl::list<FunctionNameType> FunctionNameFlags(
     "function-flags", cl::desc("Function search flags:"),
     cl::values(clEnumValN(eFunctionNameTypeAuto, "auto",
                           "Automatically deduce flags based on name."),
                clEnumValN(eFunctionNameTypeFull, "full", "Full function name."),
                clEnumValN(eFunctionNameTypeBase, "base", "Base name."),
                clEnumValN(eFunctionNameTypeMethod, "method", "Method name."),
                clEnumValN(eFunctionNameTypeSelector, "selector",

 
 bool evalMalloc(StringRef Line, IRMemoryMapTestState &State);
 bool evalFree(StringRef Line, IRMemoryMapTestState &State);
 int evaluateMemoryMapCommands(Debugger &Dbg);
 } // namespace irmemorymap
 
 } // namespace opts
 
+std::vector<CompilerContext> parseCompilerContext() {
+  std::vector<CompilerContext> result;
+  if (opts::symbols::CompilerContext.empty())
+    return result;
+
+  StringRef str{opts::symbols::CompilerContext};
+  SmallVector<StringRef, 8> entries_str;
+  str.split(entries_str, ',', /*maxSplit*/-1, /*keepEmpty=*/false);
+  for (auto entry_str : entries_str) {
+    StringRef key, value;
+    std::tie(key, value) = entry_str.split(':');
+    auto kind =
+        StringSwitch<CompilerContextKind>(key)
+            .Case("TranslationUnit", CompilerContextKind::TranslationUnit)
+            .Case("Module", CompilerContextKind::Module)
+            .Case("Namespace", CompilerContextKind::Namespace)
+            .Case("Class", CompilerContextKind::Class)
+            .Case("Structure", CompilerContextKind::Structure)
+            .Case("Union", CompilerContextKind::Union)
+            .Case("Function", CompilerContextKind::Function)
+            .Case("Variable", CompilerContextKind::Variable)
+            .Case("Enumeration", CompilerContextKind::Enumeration)
+            .Case("Typedef", CompilerContextKind::Typedef)
+            .Default(CompilerContextKind::Invalid);
+    if (value.empty()) {
+      WithColor::error() << "compiler context entry has no \"name\"\n";
+      exit(1);
+    }
+    result.push_back({kind, ConstString{value}});
+  }
+  outs() << "Search context: {\n";
+  for (auto entry: result)
+    entry.Dump();
+  outs() << "}\n";
+
+  return result;
+}
+
 template <typename... Args>
 static Error make_string_error(const char *Format, Args &&... args) {
   return llvm::make_error<llvm::StringError>(
       llvm::formatv(Format, std::forward<Args>(args)...).str(),
       llvm::inconvertibleErrorCode());
 }
 
 TargetSP opts::createTarget(Debugger &Dbg, const std::string &Filename) {
Error opts::symbols::findTypes(lldb_private::Module &Module) {
   Expected<CompilerDeclContext> ContextOr = getDeclContext(Symfile);
   if (!ContextOr)
     return ContextOr.takeError();
   CompilerDeclContext *ContextPtr =
       ContextOr->IsValid() ? &*ContextOr : nullptr;
 
   DenseSet<SymbolFile *> SearchedFiles;
   TypeMap Map;
+  if (!Name.empty())
     Symfile.FindTypes(ConstString(Name), ContextPtr, true, UINT32_MAX,
                       SearchedFiles, Map);
+  else
+    Symfile.FindTypes(parseCompilerContext(), true, Map);
 
   outs() << formatv("Found {0} types:\n", Map.GetSize());
   StreamString Stream;
   Map.Dump(&Stream, false);
   outs() << Stream.GetData() << "\n";
   return Error::success();
 }
 
if (Regex || !File.empty() || Line != 0)
       return make_string_error("Cannot search for namespaces using regular "
                                "expressions, file names or line numbers.");
     return findNamespaces;
 
   case FindType::Type:
     if (Regex || !File.empty() || Line != 0)
       return make_string_error("Cannot search for types using regular "
                                "expressions, file names or line numbers.");
+    if (!Name.empty() && !CompilerContext.empty())
+      return make_string_error("Name is ignored if compiler context present.");
+
     return findTypes;
 
   case FindType::Variable:
     if (Line != 0)
       return make_string_error("Cannot search for variables "
                                "using line numbers.");
     return findVariables;
   }