clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
One quick either documentation update for the error string, or switch to
ConstString... Probably best to the the error string docs as if we use fancy
C++ union stuff it might not
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/include/lldb/API/SBTrace.h:31
+
+ /// Start tracing a live process using a provided configuration.
///
wallace added inline comments.
Comment at: lldb/include/lldb/Target/Trace.h:31-32
+///
+/// This class assumes that errors will be extremely rare compared to the
number
+/// of correct instructions and storing them as \a ConstString should be fine.
+class TraceInstruction {
wallace added inline comments.
Comment at: lldb/include/lldb/Target/Trace.h:31-32
+///
+/// This class assumes that errors will be extremely rare compared to the
number
+/// of correct instructions and storing them as \a ConstString should be fine.
+class TraceInstruction {
wallace updated this revision to Diff 350495.
wallace added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103588/new/
https://reviews.llvm.org/D103588
Files:
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/lldb-e
wallace updated this revision to Diff 350493.
wallace marked an inline comment as done.
wallace added a comment.
address all comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103500/new/
https://reviews.llvm.org/D103500
Files:
lldb/bindings
wallace marked 18 inline comments as done.
wallace added inline comments.
Comment at: lldb/include/lldb/API/SBStructuredData.h:103
friend class SBBreakpointName;
+ friend class SBTrace;
clayborg wrote:
> Is this needed?
yes, to access the actual StructuredD
wallace added a comment.
https://reviews.llvm.org/P8265 is a sample DAP output with these messages
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101128/new/
https://reviews.llvm.org/D101128
___
lldb-comm
wallace added inline comments.
Comment at: lldb/include/lldb/Target/Trace.h:31-32
+///
+/// This class assumes that errors will be extremely rare compared to the
number
+/// of correct instructions and storing them as \a ConstString should be fine.
+class TraceInstruction {
aprantl added a comment.
I'm not sure I fully understand the suggestion:
> I think we should just remove the functionality form the timer class again. I
> only added it there because of the macro.
... and replace its uses with what?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103575
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So it would be nice to try and not encode errors into the TraceInstruction
class and deal with any errors at decode time.
Comment at: lldb/include/lldb/Target/
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/include/lldb/API/SBProcess.h:97
+ /// Deprecated
lldb::SBThread GetSelectedThread() const;
Remove? This is not depre
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
Some small rename I forgot to point out, but LGTM modulo that rename. Thanks
for cleaning this up!
Comment at: lldb/include/lldb/Symbol/Symtab.h:186
private:
+ Uniqu
wallace updated this revision to Diff 350419.
wallace added a comment.
Addressed the comments except for the "static function that we can call and
remove the passing of the instance variable for the report callback all over to
the event classes". That's a little bit hard to achieve because the c
bulbazord updated this revision to Diff 350405.
bulbazord added a comment.
Remove unused line of code that I forgot to remove previously
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103652/new/
https://reviews.llvm.org/D103652
Files:
lldb/inclu
bulbazord updated this revision to Diff 350404.
bulbazord added a comment.
Address feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103652/new/
https://reviews.llvm.org/D103652
Files:
lldb/include/lldb/Symbol/Symtab.h
lldb/source/Symbol/
Author: Raphael Isemann
Date: 2021-06-07T18:45:04+02:00
New Revision: 2c2feebcd1274425c853e3cff7cec6ba033c3ccd
URL:
https://github.com/llvm/llvm-project/commit/2c2feebcd1274425c853e3cff7cec6ba033c3ccd
DIFF:
https://github.com/llvm/llvm-project/commit/2c2feebcd1274425c853e3cff7cec6ba033c3ccd.dif
DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103744/new/
https://reviews.llvm.org/D103744
_
18 matches
Mail list logo
