wallace added inline comments.
================
Comment at: lldb/bindings/interface/SBTraceCursor.i:56
+
+ explicit operator bool() const;
+};
----------------
+1
================
Comment at: lldb/include/lldb/API/SBTraceCursor.h:21
+public:
+ /// Default constructor for an invalid \a SBTraceCursor object.
+ SBTraceCursor();
----------------
+1
================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:201
lldb::TraceEvent DecodedThread::GetEventByIndex(int item_index) const {
+ // This currently returns an undefined value when the item isn't an event.
return m_item_data[item_index].event;
----------------
jj10306 wrote:
> what's the best return value if this is called and the item isn't an event?
> ideally we could return something similar to LLDB_INVALID_ADDRESS, but since
> this is an enum we would need to add a new variant that represents the
> "invalid" case. Perhaps we could bring back the `eTraceEventNone` variant
> that was recently removed?
> wdyt?
I was thinking about that and I think a better contract is to expect users not
to use a getter that is unrelated to the current trace type. If we don't add
strict expectations, then consumers might create buggy code without realizing.
================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:234-236
+ return GetItemKindByIndex(item_index) == lldb::eTraceItemKindError
+ ? m_item_data[item_index].error
+ : nullptr;
----------------
like this. It might be better just to fail or return garbage and that will let
the user know that they are doing something they shouldn't.
Besides that, in c++ it's easy to look for nullptr when getting char *, but in
python, this might be converted eventually to an actual empty str, and that
might hide some possible bugs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130930/new/
https://reviews.llvm.org/D130930
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
