clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
================
Comment at: lldb/include/lldb/Core/PluginManager.h:342
+ static const char *GetTraceSchema(ConstString name);
+
----------------
This is fine if we have a case where we want the scheme for a given plug-in. We
should probably also add a call that takes a "size_t index" as an argument that
is the index of the plug-ins. When the index is too big, returns NULL. This
will allow us to iterate over all of the installed plug-ins and print out the
schemas for each one?
================
Comment at: lldb/include/lldb/Target/Trace.h:94
+ static llvm::Expected<llvm::StringRef>
+ FindPluginSchema(llvm::StringRef plugin_name);
----------------
Do we also want a non static version of this to be able to get the schema for
the current Trace plugin?
================
Comment at: lldb/include/lldb/Target/TraceSessionFileParser.h:107
+
+using namespace lldb_private;
+
----------------
labath wrote:
> delete. "using namespace" in a header file is a big no-no.
indeed! I will trickly through to any .cpp file that includes it
================
Comment at: lldb/source/Commands/CommandObjectTrace.cpp:242
"Show the schema of the given trace plugin.",
"trace schema <plug-in>"),
m_options() {}
----------------
Make <plug-in> optional so we can list all trace plug-in schemas?
================
Comment at: lldb/source/Plugins/Trace/intel-pt/ThreadIntelPT.h:14
+
+class ThreadIntelPT : public lldb_private::Thread {
+public:
----------------
labath wrote:
> btw, (most of) new plugins tend to place themselves in a sub-namespace of
> lldb_private. That avoids prefixing everything with `lldb_private::`
I second this, add a plug-in namespace!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88841/new/
https://reviews.llvm.org/D88841
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
