This is an archive of the discontinued LLVM Phabricator instance.

[lldb][DataFormatter] Add std::ranges::ref_view formatter
ClosedPublic

Authored by Michael137 on Nov 23 2022, 3:11 AM.

Details

Summary

This patch adds a formatter for std::ranges::ref_view<T>.
It simply holds a T*, so all this formatter does is dereference
this pointer and format it as T would be.

Testing

  • Added API tests

Diff Detail

Event Timeline

Michael137 created this revision.Nov 23 2022, 3:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2022, 3:11 AM
Michael137 requested review of this revision.Nov 23 2022, 3:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2022, 3:11 AM
  • Use typedef in test
  • Clean up test
aprantl accepted this revision.Nov 29 2022, 1:50 PM

Small nitpicks, otherwise good!

lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
42

Personally I prefer

///Pointer to the dereferenced __range_ member.
lldb::ValueObjectSP m_range_sp = nullptr;

for longer comments.

56

I wonder if it would be more readable to move those one-line function definitions into the declaration?

This revision is now accepted and ready to land.Nov 29 2022, 1:50 PM
  • Move short definitions inline
  • Fix doxygen comment
Michael137 marked an inline comment as done.Nov 30 2022, 6:21 AM
Michael137 added inline comments.
lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
42

Done

56

Looks better, done

Michael137 marked an inline comment as done.Nov 30 2022, 6:22 AM
aprantl added inline comments.Nov 30 2022, 9:35 AM
lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
36

To be very nitpicking: LLVM style wants full sentences in comments, so let's add a . at the end. :-)

labath added a subscriber: labath.Dec 1 2022, 7:24 AM

You may want to check that this kind of automatic dereferencing does not send lldb into a tailspin if the printed data structure is recursive. I know we had problems like that with smart pointer pretty printers.

I'd try some code like:

#include <ranges>
#include <vector>

struct A {
  std::ranges::ref_view<std::vector<A>> a;
};

int main() {
    std::vector<A> v;
    v.push_back(A{v});
    v[0].a = v;
    // print v ?
}

You may want to check that this kind of automatic dereferencing does not send lldb into a tailspin if the printed data structure is recursive. I know we had problems like that with smart pointer pretty printers.

I'd try some code like:

#include <ranges>
#include <vector>

struct A {
  std::ranges::ref_view<std::vector<A>> a;
};

int main() {
    std::vector<A> v;
    v.push_back(A{v});
    v[0].a = v;
    // print v ?
}

@labath good point

E.g., the following would cause such unbounded recursion (didn't find a good way of triggering it using the actual ref_view since it's not default constructible, but it's probably doable):

#include <vector>

namespace std {
inline namespace __1 {
namespace ranges {
template<typename T>
struct ref_view {
    T* __range_;
};
}
}
}

struct Foo {
    std::ranges::ref_view<std::vector<Foo>> a;
};

int main() {
    Foo f;
    std::vector<Foo> v;
    v.push_back(f);

    std::ranges::ref_view<std::vector<Foo>> r{.__range_ = &v};
    f.a = r;

    return 0;
}