0

The following code tries to define a self-similar datastructure JSON using the std containers std::vector and std::unordered_map but does not compile. It all seems to come down to std::unordered_map's hashtable deleting its destructor because it is inaccessible/private in the base class. I can't really put my fingers on why this would happen. Following my previous question any cyclic dependency regarding allocators should have been resolved and the cyclic dependencies of vectors and unordered_map's value types should be supported as of 2022.

Demo

#include <variant>
#include <unordered_map>
#include <vector>
#include <string>
#include <string_view>
#include <concepts>
#include <memory> // allocator
#include <memory_resource>
#include <cstddef> // std::size_t

template <typename StringType = std::string, typename Allocator = std::allocator<void>>
class JSON;

template <typename StringType, typename Allocator>
class JSON: public std::variant<std::monostate,
    std::unordered_map<StringType, JSON<StringType, Allocator>, std::hash<JSON<StringType, Allocator>>, std::equal_to<StringType>,
        typename std::allocator_traits<Allocator>::template rebind_alloc<std::pair<const StringType, JSON<StringType, Allocator>>>
    >,
    std::vector<JSON<StringType, Allocator>,
        typename std::allocator_traits<Allocator>::template rebind_alloc<JSON<StringType, Allocator>>
    >,
    bool,
    double,
    StringType>
{
public:
    using JSON::variant::variant;
};

int main() {
    JSON json;
}

clang trunk says:

/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.1/../../../../include/c++/13.0.1/bits/functional_hash.h:102:19: note: destructor of 'hash<JSON<>>' is implicitly deleted because base class '__hash_enum<JSON<basic_string<char, char_traits<char>, allocator<char>>, allocator<void>>>' has an inaccessible destructor
    struct hash : __hash_enum<_Tp>

gcc trunk says:

/opt/compiler-explorer/gcc-trunk-20230415/include/c++/13.0.1/bits/functional_hash.h:84:7: note: declared private here
   84 |       ~__hash_enum();
      |

Why would it suddenly be declared private?

Benjamin Buch
  • 4,752
  • 7
  • 28
  • 51
glades
  • 3,778
  • 1
  • 12
  • 34
  • 2
    You are mapping a string to json, the hash should be of the string. It should always be for the key type. – StoryTeller - Unslander Monica Apr 15 '23 at 09:56
  • And the error comes from the fact that you try to use std::hash for a type that has no hash implementation. – gerum Apr 15 '23 at 09:57
  • @StoryTeller-UnslanderMonica That makes sense. I didn't even see that in this template mess haha. Thank you – glades Apr 15 '23 at 10:01
  • Now you need a hash class in your template argument list. Good exercise as a toy, but why don't you just settle for `boost::json`?! Do you plan to beat it? – Red.Wave Apr 15 '23 at 17:25
  • @Red.Wave No but I need a feature that I didn't find yet. I need to parse half of the json now and the rest later. This will be a coroutine based json parser. – glades Apr 15 '23 at 18:26
  • So the ugliest part is waiting. I wouldn't use coroutines, unless I were threatened to take a bullet into my head; but that's just my personal opinion. – Red.Wave Apr 16 '23 at 17:59
  • @Red.Wave There are in my opinion only very marginal use cases where you really need coroutines and the often presented generator is not one of them. However, for this exact purpose it's actually a very natural approach. And for other problems coroutines are actually the only portable solution, e.g. to solve the cactus stack problem in [continuation passing work stealing task pools](https://github.com/ConorWilliams/libfork) – glades Apr 16 '23 at 19:53
  • The concept is fine. Final implementation in C++ sucks. I don't like the way they pckaged it up. Too complex, very dangerous and unreadable. Looking at a forward declaration, you can not tell difference with a function. Coroutine lambdas are even worse: you can commit a use-after-free without noticing. Finally, you need a lot of boilerplate for something that should be a core language feature. It is just terrible. – Red.Wave Apr 17 '23 at 06:40
  • @Red.Wave Agreed. Maybe I don't understand them well enough, but I thought it was very unintuitive as well. I think the generall effort was to make them very customizable which they are. But I think most customization points are actually nonsensical and dangerous. – glades Apr 17 '23 at 12:26
  • I think they rushed to standardize it. No keyword implementations with no library constraints were possible, if they could just wait. Now coroutines have become a toy for elite library programmers. You cannot teach them to intermediate programmers. Needing a complex boilerplate has become an essential part of it; not like a function or a class or a concept or any other simple abstraction tool present in the language. – Red.Wave Apr 17 '23 at 17:49
  • @Red.Wave Indeed that are some of my thoughts as well. However consider that coroutines have an inherent complexity due to their stateful nature that can only be abstracted away, but I think they wanted to expose it instead. It's not a simple concept in any language. – glades Apr 17 '23 at 20:34
  • The tools and terms to simplify that exist already. The final package goes against the C++ trend in the last 2 decades. If it is not easy to teach, it will be error-prone to use. Current state of coroutines takes us back to the age of raw pointers. Now we need great amount of lib support to be able to use them. I still consider using coroutines bad practice, even though the creators might love them. Look at the level of effort needed to get people to use STL; same amount is needed to just get them to use coroutines; eventually they will end-up as an elite-only toolset. Why so much fuzz then? – Red.Wave Apr 18 '23 at 12:12

1 Answers1

2

To find such errors it is best to simplify the code as long as the error still occurs. In this reduced version the problem then becomes recognizable:

#include <string>
#include <unordered_map>
#include <variant>

template <
    typename StringType = std::string,
    typename Allocator = StringType::allocator_type>
struct JSON: std::variant<std::monostate,
    std::unordered_map<
        StringType,
        JSON<StringType, Allocator>,
        std::hash<
            JSON<StringType, Allocator> // this is not hashable
        >
    >>
{
    using JSON::variant::variant;
};


int main() {
    JSON json;
}

I think what you wanted to do is std::hash<StringType> instead of std::hash<JSON<StringType, Allocator>> since the hash map needs a unique key and not a unique value.

Benjamin Buch
  • 4,752
  • 7
  • 28
  • 51