aprantl marked an inline comment as done.
aprantl 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));
----------------
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".


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

Reply via email to