mib added a comment.
Took a look at the SBAPI changes and left few comments but overall, that part
looked good to me. It would be nice if you could delete the depreciated
method/class.
================
Comment at: lldb/bindings/interface/SBTrace.i:32-51
+ /// deprecated
void StopTrace(SBError &error,
lldb::tid_t thread_id);
+ /// deprecated
void GetTraceConfig(SBTraceOptions &options,
SBError &error);
----------------
JDevlieghere wrote:
> Do you have clients that already rely on these functions? Writing deprecated
> above them means nothing. When I was doing the reproducers, I included a
> warning in the header saying that this was under development and the API
> wasn't final, which allowed me to iterate on it. Can we just remove these
> methods and to the same here?
+1
================
Comment at: lldb/bindings/interface/SBTraceOptions.i:12
%feature("docstring",
-"Represents the possible options when doing processor tracing.
-
-See :py:class:`SBProcess.StartTrace`."
+"deprecated"
) SBTraceOptions;
----------------
Same comment as @JDevlieghere here
================
Comment at: lldb/include/lldb/API/SBTrace.h:100
-protected:
- typedef std::shared_ptr<TraceImpl> TraceImplSP;
-
- friend class SBProcess;
+ /// Deprecated
+ /// \{
----------------
Same here.
================
Comment at: lldb/source/API/SBTrace.cpp:92
LLDB_RECORD_METHOD_NO_ARGS(bool, SBTrace, IsValid);
- return this->operator bool();
+ return LLDB_RECORD_RESULT(this->operator bool());
}
----------------
No need to use `LLDB_RECORD_RESULT` on primitive types such as `bool`.
================
Comment at: lldb/source/API/SBTrace.cpp:97
LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBTrace, operator bool);
+ return LLDB_RECORD_RESULT((bool)m_opaque_sp);
+}
----------------
Same here.
================
Comment at: lldb/source/API/SBTrace.cpp:103
+ size_t offset, lldb::tid_t thread_id) {
+ LLDB_RECORD_DUMMY(size_t, SBTrace, GetTraceData,
+ (lldb::SBError &, void *, size_t, size_t, lldb::tid_t),
----------------
Either use `LLDB_RECORD_DUMMY` on all the deprecated method or don't use it at
all :) If you could get rid of the deprecated methods that would be even better.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103500/new/
https://reviews.llvm.org/D103500
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
