mapbox/variant

warning: 'bad_variant_access'

Closed this issue · 5 comments

With clang++ -Weverything I see:

deps/mapbox/variant/variant.hpp:58:7: warning: 'bad_variant_access' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Wweak-vtables]
class bad_variant_access : public std::runtime_error
      ^

/cc @artemp - worth fixing?

joto commented

I think this is not fixable if I understand the error correctly. We want to derive our exception class from std::runtime_error which is a virtual class. But this being a header-only library there is no way to define the methods of our class anywhere but in the header. There is no .cpp file to define them in.

Yes, as long as we want to derive from std::runtime_error or std::exception we'll face this issue.

Having a look at implementation - with our current usage there's no benefit or requirement to derive from std::runtime_error and it'll be better to derive from std::exception (same as boost::variant)

class bad_variant_access : public std::exception
{
public:
    virtual const char* what() const noexcept
    {
        return "mapbox::util::bad_variant_access: FAIL";
    }
}; // class bad_variant_access

At least the above is slightly more efficient ^

/cc @joto @springmeyer

joto commented

See #48. I specifically used std::runtime_error to be backwards compatible with current user code that might check for it, but I believe this should be std::logic_error.

As for your proposed change: I don't think making exceptions more efficient is really important, but otherwise that change looks okay to me if we are sure we always want a fixed string as the message here.

Okay thanks for the details and discussion. Going to close as I agree there is nothing needing fixed.

I m also facing same issue.

anyone have solution or workaround?