2

Here's what I'm trying:

auto fwd_args = std::forward_as_tuple(std::forward<Args>(args)...);
auto key = std::make_pair(std::type_index(typeid(T)), std::any(fwd_args));

The error is:

error C2440: '': cannot convert from 'std::tuple<const char (&)[78],_Ty &&>' to 'std::any'

Which traces back to here:

factory->get<Font>(R"(C:\Fonts\myfont.ttf)", 24)

Where the Font c'tor is:

explicit Font(const std::string& filename, float fontSize=32) {

My questions:

  1. Can I cast an arbitrary args to an std::any?
  2. If not, how can I use arbitrary args as a key in a map?

Full code below:

class SingletonFactory {
  public:
    template<typename T, typename... Args>
    const T &get(Args &&... args) {
        auto fwd_args = std::forward_as_tuple(std::forward<Args>(args)...);
        auto key = std::make_pair(std::type_index(typeid(T)), std::any(fwd_args));

        auto it = _cache.find(key);

        if(it != _cache.end()) {
            return std::any_cast<T>(it->second);
        }

        return std::any_cast<T>(_cache.emplace(std::piecewise_construct, std::forward_as_tuple(key), fwd_args).first);
    }

  private:
    std::map<std::pair<std::type_index, std::any>, std::any> _cache{};
};

Instead of std::pair<std::type_index, std::any> we can try using a struct which implements all the necessary methods...

Example on Godbolt


What I'm trying to do:

I'm trying to build a cache for some assets/resources for my game. e.g. if I want to use the same font in two different places, I don't want to have load it twice (which involves reading it from disk and converting it into a texture and uploading it to the GPU). And because the resources have handles, it's important that their destructors are called deterministically (e.g. when unloading a level I will destroy the factory).

I can do this all manually of course, but I don't want to have to keep track of where I use a 24px FontA vs a 32px FontB and manually pipe those objects around my game. I just want general cache that I can dump everything into. Then if I've used that specific asset before, great, it'll be re-used, if not, it can make a new one. If I later decide to scrap that level or asset or what have you, I just delete the get<> and it's gone, I don't have to backtrack and find every place I piped it through.

mpen
  • 272,448
  • 266
  • 850
  • 1,236
  • 8
    Attempting to defeat C++'s type safety (which is a core, fundamental, baked-in property of C++) always ends in tears. – Sam Varshavchik Dec 05 '21 at 22:43
  • 10
    According to `cppreference` on `std::any`: "The class `any` describes a type-safe container for single values of any *copy constructible* type." I don't think that tuple of references are copy constructible. – ALX23z Dec 05 '21 at 22:45
  • @SamVarshavchik It might, but hopefully I've minimized the surface area because `get<>` itself is strongly typed, so I'm hoping nothing leaks. If not, I guess I'll find out the hard way. – mpen Dec 05 '21 at 22:54
  • @ALX23z https://en.cppreference.com/w/cpp/utility/tuple/tuple isn't (2) the copy constructor? – mpen Dec 05 '21 at 22:58
  • 1
    @mpen see below on that page when it's allowed. – bipll Dec 05 '21 at 23:02
  • 2
    Separate from the specific question, using `pair` as a key to a `std::map` already can't work. `std::any` doesn't have any comparison operators defined. – Barry Dec 05 '21 at 23:15
  • 1
    Ehm... (8) is the copy constructor, not (2). And the requirement is that all tuple's elements are copy constructible. – ALX23z Dec 05 '21 at 23:39
  • @ALX23z Ah, right, well `std::string` and `float` should be copy-constructible, so that shouldn't be a problem – mpen Dec 05 '21 at 23:48
  • @mpen but as the error code stated, those aren't `string` or `float`. Those are `const char (&)[78]` and `_Ty &&` (with `Ty=float`). Neither of which are remotely copyable. – ALX23z Dec 06 '21 at 01:32
  • @ALX23z Is there some way to type `Args` using the constructor of `T` rather than the args that were passed to `get<>()`? – mpen Dec 06 '21 at 02:03
  • Or I guess I can explicitly call it like `factory->get("str",32.f)` – mpen Dec 06 '21 at 02:13
  • Why do you want this? C++ is **_not_** javascript. It's a compiled type-safe language, and for a reason: type-erasure introduces all kinds of problems. This sounds like an XY problem: you are asking us to fix your solution to a problem, instead of stating the actual problem. So: What's the actual underlying problem you are trying to solve? – JHBonarius Dec 17 '21 at 11:58
  • @JHBonarius It's only 'erased' inside the factory. The `get<>` method still returns a specific type. The problem I want to solve is caching resources. Fonts, images, meshes and other assets for my game. I don't want to have to maintain a different map for each type of asset I need to store. Just want one bucket I can throw everything into, that'll let me iterate quickly. – mpen Dec 17 '21 at 22:42
  • 1
    You combining "map" and "quickly" into one sentence made me jump. I hope you know that std::map is implemented using a red-black tree and that std::any uses dynamic allocation to store the underlying object? In any performance application we just use std::vector, with -indeed- a separate container per type. Often you even split out the types' members into separate containers, to keep cache coherency. – JHBonarius Dec 18 '21 at 10:31
  • In your example: `.less = [argsTuple] (any const&) {}` that won't work. `less` has to be a `std::function` not `decltype ([local_capture_here](auto&){})`. Also: please use `const` where it's needed, when it's needed. Everywhere is not it. – viraltaco_ Dec 18 '21 at 15:53
  • @JHBonarius I'd be happy with separate containers per type, but I couldn't figure out how to do that dynamically. It's possible if the caches are made static like in https://stackoverflow.com/q/32628963/65387 but then the resources are not destructed when the factory is. – mpen Dec 18 '21 at 20:08

3 Answers3

4

Remember that type erasure facilities like std::any and std::function only exposes the interface they promise and nothing more: std::any encapsulates copy-ability but nothing more, thus you cannot compare equality / hash a std::any object. Using an additional std::function just to store the operator< is cumbersome (that's essentially using two vtables for every type), you're better off using hand-rolled type erasure.

Also, inferred from your requirements, you have to special-case const char* or const char(&)[N] parameters, since you want them stored as std::string, and also for their comparison operators. This also solves your "storing a std::tuple with reference members in std::any" problem. (See edit note #2 for more discussions.)

The code in your godbolt link was incorrect in some places, especially that you were passing the arguments for T's constructor to construct a std::any (missing the std::in_place_type<T> at the front, that is).

The following implementation uses C++20 for convenience, but it can be made working under older standard with some modification.

Edit #1: Fixed uninitialized initial hash value, really a noob error on me.

Edit #2: Yes, the trick to special-case const char* isn't great, and it prevents c'tors that take const char* from working. You could just rewrite it as "just decay every parameter and don't take any special action for const char* or const char(&)[N]", and that will work for all the c'tors. But this also will only work if you pass in string literals, or else you might have a dangling pointer stored in your hash map. Maybe this approach is OK if you specify every location where you really want to pass the reference by std::string (e.g. by using UDL like "hello"s or explicitly constructing a std::string).
AFAIK you cannot get the parameter types of c'tors since C++ explicitly disallows taking the address of c'tors, and if you cannot form the pointer to member function you cannot do template tricks on it. Also, overload resolution might be another barrier to achieving this.

Edit #3: I didn't notice that there would be non-copyable objects to cache. In this case, std::any is of no use, since it can only store copyable objects. Using similar type erasure technique non-copyable objects could also be stored. My implementation just uses std::unique_ptr to store the erased keys and values, forcing them to be stored on the heap. This simple method even supports non-copyable and non-movable types. If SBO is needed, more sophisticated ways to store the type-erased objects must be used.

#include <iostream>
#include <unordered_map>
#include <type_traits>

// Algorithm taken from boost
template <typename T>
void hash_combine(std::size_t& seed, const T& value)
{
    static constexpr std::size_t golden_ratio = []
    {
        if constexpr (sizeof(std::size_t) == 4)
            return 0x9e3779b9u;
        else if constexpr (sizeof(std::size_t) == 8)
            return 0x9e3779b97f4a7c15ull;
    }();
    seed ^= std::hash<T>{}(value) + golden_ratio +
        std::rotl(seed, 6) + std::rotr(seed, 2);
}

class Factory
{
public:
    template <typename T, typename... Args>
    const T& get(Args&&... args)
    {
        Key key = construct_key<T, Args...>(static_cast<Args&&>(args)...);
        if (const auto iter = cache_.find(key); iter != cache_.end())
            return static_cast<ValueImpl<T>&>(*iter->second).value;
        Value value = key->construct();
        const auto [iter, emplaced] = cache_.emplace(
            std::piecewise_construct,
            // Move the key, or it would be forwarded as an lvalue reference in the tuple
            std::forward_as_tuple(std::move(key)),
            // Also the value, remember that this tuple constructs a std::any, not a T
            std::forward_as_tuple(std::move(value))
        );
        return static_cast<ValueImpl<T>&>(*iter->second).value;
    }

private:
    struct ValueModel
    {
        virtual ~ValueModel() noexcept = default;
    };

    template <typename T>
    struct ValueImpl final : ValueModel
    {
        T value;

        template <typename... Args>
        explicit ValueImpl(Args&&... args): value(static_cast<Args&&>(args)...) {}
    };
    
    using Value = std::unique_ptr<ValueModel>;

    struct KeyModel
    {
        virtual ~KeyModel() noexcept = default;
        virtual std::size_t hash() const = 0;
        virtual bool equal(const KeyModel& other) const = 0;
        virtual Value construct() const = 0;
    };

    template <typename T, typename... Args>
    class KeyImpl final : public KeyModel
    {
    public:
        template <typename... Ts>
        explicit KeyImpl(Ts&&... args): args_(static_cast<Ts&&>(args)...) {}

        // Use hash_combine to get a hash
        std::size_t hash() const override
        {
            std::size_t seed{};
            std::apply([&](auto&&... args)
            {
                (hash_combine(seed, args), ...);
            }, args_);
            return seed;
        }

        bool equal(const KeyModel& other) const override
        {
            const auto* ptr = dynamic_cast<const KeyImpl*>(&other);
            if (!ptr) return false; // object types or parameter types don't match
            return args_ == ptr->args_;
        }

        Value construct() const override
        {
            return std::apply([](const Args&... args)
            {
                return std::make_unique<ValueImpl<T>>(args...);
            }, args_);
        }

    private:
        std::tuple<Args...> args_;
    };

    using Key = std::unique_ptr<KeyModel>;
    using Hasher = decltype([](const Key& key) { return key->hash(); });
    using KeyEqual = decltype([](const Key& lhs, const Key& rhs) { return lhs->equal(*rhs); });

    std::unordered_map<Key, Value, Hasher, KeyEqual> cache_;

    template <typename T, typename... Args>
    static Key construct_key(Args&&... args)
    {
        constexpr auto decay_or_string = []<typename U>(U&& arg)
        {
            // convert to std::string if U decays to const char*
            if constexpr (std::is_same_v<std::decay_t<U>, const char*>)
                return std::string(arg);
                // Or just decay the parameter otherwise
            else
                return std::decay_t<U>(arg);
        };
        using KeyImplType = KeyImpl<T, decltype(decay_or_string(static_cast<Args&&>(args)))...>;
        return std::make_unique<KeyImplType>(decay_or_string(static_cast<Args&&>(args))...);
    }
};

struct IntRes
{
    int id;
    explicit IntRes(const int id): id(id) {}
};

struct StringRes
{
    std::string id;
    explicit StringRes(std::string id): id(std::move(id)) {}
};

int main()
{
    Factory factory;
    std::cout << factory.get<IntRes>(42).id << std::endl;
    std::cout << factory.get<StringRes>("hello").id << std::endl;
}
Chlorie
  • 829
  • 4
  • 10
  • Nicely done. Shouldn't the local `seed` be initialized in `KeyImpl::hash()`? – bogdan Dec 17 '21 at 14:40
  • Thank you for the detailed answer, but I think there's something wrong here yet. The cache is never hitting? https://godbolt.org/z/MPMo5YxzK – mpen Dec 17 '21 at 22:53
  • One other thing, does the `std::is_same_v, const char*>` trick prevent me from having objects that actually *do* take `const char*` in their constructor args? Is it possible to pull the types from the constructor for `T` instead of trying to infer them from `Args`? – mpen Dec 17 '21 at 23:13
  • @bogdan Nice catch, that is really a noob error on me... – Chlorie Dec 18 '21 at 05:44
  • @mpen (1) That's a noob error on me, I forgot to initialize the hash value :facepalm: (2) I have updated the answer on this matter. – Chlorie Dec 18 '21 at 06:13
  • @Chlorie Still missing the "not copy-constructable" requirement now; https://godbolt.org/z/qTvsGTs3j although I'm not quite sure why `construct()` is trying to call that c'tor. But without it "IntRes dtor" is called twice too which is also suspicious. – mpen Dec 18 '21 at 20:02
  • 1
    @mpen Sorry I missed the requirement that the object might also be non-copyable. In this case, `std::any` is of no use since `std::any` requires the object it stores to be copyable. I'll update the answer again, adapting the code to support non-copyable type. – Chlorie Dec 19 '21 at 12:59
  • 1
    @mpen As for why the d'tor is called twice: Notice that there's a move construction of `std::any` in the cache-miss path. `std::any` uses small buffer optimization only if the object type is small, and that the type is nothrow-move-constructible, to satisfy the `noexcept`-ness of `std::any`'s move c'tor. No special c'tors means that `IntRes` fits in SBO, thus the `IntRes` move c'tor is called, resulting in another `d'tor` output; writing the copy c'tor disables move, thus disables SBO, moving `std::any` is just copying pointers (to the heap-allocated object), so no extra `d'tor` output. – Chlorie Dec 19 '21 at 13:07
  • Looks like it works the way I need now. Thank you so much! I'm going re-read this a few times; still wrapping my head around it. The fact that it works on non-moveable types is great too; I had to add move c'tors to a bunch of my classes to get my them to work, but maybe I don't need to now! https://godbolt.org/z/5oY439GYa – mpen Dec 19 '21 at 22:38
2

The problem, as noted in the comments, is not the std::tuple template. It's specifically this part in your code:

const T &get(Args &&... args) {
    auto fwd_args = std::forward_as_tuple(std::forward<Args>(args)...);

That's obviously a tuple of references. You can't even put one reference in a std::any, let alone a tuple of them. If you can live with copies, just drop the references here. There's no rule in C++ that Template Argument Packs must be forwarded as tuples of references.

As for Q2, "If not, how can I use arbitrary args as a key in a map?" - you can't. Map keys must have a partial ordering. Even the float fontSize is already a bit problematic, if someone would pass a NaN.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • Not too worried about NaNs, all the fontSizes will be hardcoded by me. I can live with copies. Not sure why I can't have partial ordering. If the type_index matches then I ought to be able to be able to do a comparison. – mpen Dec 06 '21 at 00:27
  • @mpen: Using _only_ the `type_index` is possible, The problem is comparing two `std::any` objects; you've got no idea what types they have. Storing that information in the form of a type-erased lambda is indeed a workaround. – MSalters Dec 06 '21 at 00:34
  • You can put a tuple of references in an `std::any`, as long as they're lvalue references :-). The problem is that the second tuple element is an rvalue reference in that instantiation, and that makes the tuple not copy constructible. – bogdan Dec 16 '21 at 15:57
0

Ignoring the code you provided, and reading "What I'm trying to do".
Here is all I have:

#include <map>
#include <any>
#include <string_view>

class LevelCache {
  std::map<const std::string_view, std::any> self_;

public:
  template <class T> auto get(std::string_view key) const -> T const& {
    // If it's not there what on earth do I return? Throw. You handle it.
    return std::any_cast<T const&>(self_.at(key));
  }

  auto operator [](std::string_view key) -> std::any& {
    return self_[key]; // not const. Used for insert
  }
};

#include <iostream>
auto main() -> int {
  auto lvl = LevelCache();
  lvl["Some resource"] = 1;
  lvl["a string"] = "some string"; // Not sure if UB, or not. 

  std::cout << "lvl[\"Some resource\"] -> " << lvl.get<int>("Some resource") << '\n'
            << "lvl[\"a string\"] -> " << lvl.get<char const*>("a string");
}

I couldn't come up with anything better of the top of my head. Does that solve (part of) the problem you're facing? Because I'm having a hard time understanding the exact concern.
Compiler Explorer

viraltaco_
  • 814
  • 5
  • 14
  • No. You're missing two key requirements. One being that I don't want to pre-populate the cache. Two is that `get()` should accept args, not just a key. – mpen Dec 18 '21 at 19:50
  • Ok, correct me if I'm wrong: you want the factory to return a reference to some resource when you call `get` and it should store that resource internally? Why are you using a map at all? You don't need to store the argument you pass to the constructor, do you? Isn't it the same as storing the constructed object anyway? – viraltaco_ Dec 18 '21 at 20:06
  • Yes, I want it to return a reference if it's already been constructed. And yes, I do need to store the args. How also can I differentiate between "arial.ttf" and "verdana.ttf"? They're both `Font` objects but they're distinct resources. – mpen Dec 18 '21 at 20:20
  • Does [this code](https://godbolt.org/z/j7n4qGd6o) solve at least one problem (intentionally ignoring the other for now; namely, this always creates the object) – viraltaco_ Dec 18 '21 at 20:32
  • Yes. The caching is really the most important part though :-) – mpen Dec 18 '21 at 20:55