This is an archive of the discontinued LLVM Phabricator instance.

JSON library (just a prototype at this point).
AbandonedPublic

Authored by sammccall on Oct 23 2017, 6:36 AM.

Details

Reviewers
None
Summary

Main features:

  • DOM, fully parsed/validated ahead of time
  • All DOM elements arena-allocated and owned by the document
  • tagged-pointer representation of Values, integers/nulls/booleans inline
  • expressive syntax for JSON literals with Twine-like semantics

Event Timeline

sammccall created this revision.Oct 23 2017, 6:36 AM
sammccall updated this revision to Diff 119855.Oct 23 2017, 8:25 AM

Clean up types, add more standard APIs to Object/Array, clang-format.

I've simply skimmed through the code, but overall design looks great.

It's a bit of a pity that Arrays may allocate outside the arena. What are the reasons we prefer SmallVector to std::vector if we're allocating in the arena anyway?

include/llvm/Support/JSONDetail.h
83

s/ValeWrapper/ValueWrapper

It's a bit of a pity that Arrays may allocate outside the arena.

Yeah :-(

It's not trivial to retrofit SmallVector (we'd need to add realloc to Allocator, and SmallVector's complex inheritance hierarchy, templates, and empty base optimization might need some untangling).
I talked to Chandler about this, and he said this is the part where people write their own specialized vector.

BTW, If it wasn't for Array's heap allocations, I think we might be able to make Value's destructor empty and just abandon the objects, which would speed up/simplify a bit. The arenas just have PODs, vectors, and StringMaps...

What are the reasons we prefer SmallVector to std::vector if we're allocating in the arena anyway?

Huh, you're right: a sized std::vector initialized on an arena has the same layout as a perfectly-sized SmallVector.

With our current broken arena-allocation, SmallVector's SSO means we end up on the arena some of the time. (And avoiding the small allocations is the important part, because there won't be many big ones). But if we can adapt BumpPtrAllocator& to be an STL allocator, then std::vector would be the way to go. Time to see if someone already did that...

klimek added inline comments.Oct 23 2017, 10:20 AM
unittests/Support/JSONTest.cpp
26

Can we use variadic templates for array(1, 2, 3)?

Switch to std::vector, with BumpPtrAllocator adapted to the STL allocator.
All allocations are now on the arena.

sammccall added inline comments.Oct 23 2017, 11:00 AM
unittests/Support/JSONTest.cpp
26

That would be nice... I can't make it work properly when the elements are init-lists though.

My understanding is that with variadic templates you have to accept any (heterogeneous) parameters, and then rely on SFINAE to reject the ones you don't want.

This works for "ordinary" values: I can pass a string, with a SFINAE-check that string is convertible to Literal, and then convert it in the body.

But it doesn't work if the parameter is an init-list, like array(1, {"foo", 42}), because it can't deduce a type for that parameter. Unfortunately these are the cases we care most about as they're the ambiguous ones that array() is supposed to solve.

This is what I had half-working:

template <typename R, typename...> struct all { typedef R type; };
template <typename... T>
typename all<Literal,
  typename std::enable_if<
    std::is_convertible<T, const Literal&>::value
  >::type...
>::type
array(T... V) {
  return Literal(std::initializer_list<Literal>{V...}, 1);
}
sammccall updated this revision to Diff 119936.Oct 23 2017, 2:12 PM

Fix SmallInt testing, and allow it to build on 32-bit platforms.

sammccall updated this revision to Diff 119961.Oct 23 2017, 4:08 PM

Add kind() API, ordering, hashing.

sammccall updated this revision to Diff 120436.Oct 26 2017, 9:23 AM

Value semantics for literals
Bugfix in parser

sammccall updated this revision to Diff 120565.Oct 27 2017, 3:03 AM

Add a few simple tests, fix unicode escapes (error recovery).

sammccall abandoned this revision.Nov 17 2017, 12:37 AM