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, and everywhere else in the function where
> > it's possible to do so. If the process has really disappeared, there's no
> > need to print anything. In general there's no need to present an incomplete
> > result from this formatter.
> Data formatters can get called in the absence of a process. For simple C
> structs, for instance, they might be used to look at an initialized static
> object in a DATA section. For instance, I could do something like:
>
> #include <functional>
> auto g_func = [](int x, int y) {return x + y; };
> int
> main(int argc, char ** argv)
> {
> int ret = g_func(argc, 100);
> return ret;
> }
>
> and I should be able to see g_func using "target var" before running the
> program. I think the only place you care about the process here is whether
> you should resolve a load address or not. Target->ReadMemory will read from
> a running process if that target has one (but preferring data from the
> underlying files if that's faster.) So I think this formatter should be able
> to do its job with or without a process.
>
> But I agree with Vedant that in cases where you can't get useful information
> (for instance if you can't get the vtable address), then you should return
> false immediately and not try to present a summary. That way instead of
> showing an incomplete summary, lldb will show the raw printing of the type,
> which is more useful than a place-holder summary string.
>
> You already do that for the case where you can't read the process memory.
> Using early return when you discover an error will make it easier to ensure
> you are doing that right every time you get an error.
AFAIT we can't derive useful information for the static case.
`std::function` does not have constexpr constructors so it can't be initialized
during constant initialization. I confirmed in lldb that all the fields are
uninitialized therefore we have no pointers to examine.
================
Comment at: source/Plugins/Language/CPlusPlus/LibCxx.cpp:136-140
+ // Given:
+ //
+ // main::$_1::__invoke(...)
+ //
+ // We want to slice off __invoke(...) and append operator()()
----------------
jingham wrote:
> In the first bit of code, you put the explaining comment before the code that
> handles the condition. This time it's in the middle of the code. Maybe move
> this comment before "if (symbol_name.contains(...)"
I updated the commenting a big, hopefully it clears things up.
https://reviews.llvm.org/D50864
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
