JDevlieghere added inline comments.
================ Comment at: lldb/tools/lldb-test/lldb-test.cpp:145 + "compiler-context", + cl::desc("Specify a compiler context as \"[(type,name),...]\"."), + cl::value_desc("context"), cl::sub(SymbolsSubcommand)); ---------------- aprantl wrote: > labath wrote: > > JDevlieghere wrote: > > > aprantl wrote: > > > > JDevlieghere wrote: > > > > > > ...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? > > > > 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. > > > 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. > > 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. > > 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". Okay, in that case passing them on the command line is definitely the way to go. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66453/new/ https://reviews.llvm.org/D66453 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits