This revision was automatically updated to reflect the committed changes.
Closed by commit rL340543: Add libc++ data formatter for std::function
(authored by adrian, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
shafik updated this revision to Diff 161997.
shafik added a comment.
Updating comment
https://reviews.llvm.org/D50864
Files:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
This looks fine. There was one place where you referred back to case 1 by name
not ordinal, but otherwise this is quite clear. Don't need another round of
review, just fix that nit and
shafik added a comment.
@jingham @vsk I believe I have addressed most of your comments
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:58
+
+ if (process != nullptr) {
+Status status;
jingham wrote:
> vsk wrote:
> > Please use an early exit here,
shafik updated this revision to Diff 161837.
shafik marked 12 inline comments as done.
shafik added a comment.
Fixes based on comments
- Switched to early exits
- Added a lot more comments to explain all the cases being dealt with and
noting which cases different sections are dealing with
jingham added a comment.
Sounds like std::function objects need to run static initializers before they
are useful. So trying to format them w/o a process won't do any good.
Probably better to just return false directly if you don't have a process in
that case.
jingham added a comment.
For the most part this is fine. There are two bits to work on:
1. I think you could do all of this work on a static std::function object from
the data section even if you don't have a process. It might be worth seeing
whether you can do that. It looks like you can
vsk added inline comments.
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:46
+ // Member __f_ has type __base*, the contents of which will either directly
+ // hold a pointer to the callable object or vtable entry which will hold the
+ // type information need to
shafik created this revision.
shafik added reviewers: jingham, davide.
Herald added a reviewer: EricWF.
Herald added a subscriber: christof.
Adding formatter summary for std::function.
- Added LibcxxFunctionSummaryProvider
- Removed LibcxxFunctionFrontEnd
- Modified data formatter tests
