microsoft
`<exception>`: constructor does not copy what_arg if `_HAS_EXCEPTIONS=0`
Describe the bug
When _HAS_EXCEPTIONS is defined to 0, the constructors of the exception classes do not copy the what_arg, but only store the pointer: https://github.com/microsoft/STL/blob/472161105d596192194d4715ccad307c6c163b4a/stl/inc/exception#L90
This leads to errors if the pointee is a temporary string.
If _HAS_EXCEPTIONS is set to 1, a copy is made just as the standard would require it.
Command-line test case
The following code outputs [json.exception.parse_error] foo if _HAS_EXCEPTIONS is set to 1, and some garbage if _HAS_EXCEPTIONS is set to 0:
#include <exception> // exception
#include <stdexcept> // runtime_error
#include <string> // string
#include <iostream> // cout
namespace nlohmann
{
namespace detail2
{
class exception : public std::exception
{
public:
const char* what() const noexcept override
{
return m.what();
}
protected:
exception(const char* what_arg) : m(what_arg) {}
private:
std::runtime_error m;
};
class parse_error : public exception
{
public:
static parse_error create(const std::string& what_arg)
{
std::string w = "[json.exception.parse_error] " + what_arg;
return parse_error(w.c_str());
}
private:
parse_error(const char* what_arg) : exception(what_arg) {}
};
} // namespace detail2
} // namespace nlohmann
int main()
{
auto error = nlohmann::detail2::parse_error::create("foo");
std::cout << error.what() << std::endl;
}
The example comes from https://github.com/nlohmann/json where exceptions are created by a SAX parser, but it is up to the client whether they are thrown or not. Therefore, using exception classes while disabling throwing exceptions is a usecase.
Expected behavior
The exception makes a copy of the what_arg argument, regardless of the value of _HAS_EXCEPTIONS.
STL version
I could reproduce the issue with MSVC 19.16.27045.0 and MSVC 19.29.30040.0.
Additional context
The issue was originally found here: https://github.com/nlohmann/json/discussions/2824
_HAS_EXCEPTIONS=0 is quasi-supported (it's not officially documented, and has various design/usability issues like this one, although we've tried to avoid breaking how it works). I've marked this as vNext because I believe that dynamically allocating/deallocating memory in the _HAS_EXCEPTIONS=0 implementation of exception would be ABI-breaking. Also marked as decision needed - as @mnatsuhara mentioned, we need to decide whether to change this long-standing behavior (e.g. attempting to malloc memory, and storing a string literal as a fallback if we're unable to; that would also require storing whether the string is dynamically allocated, which is super-definitely ABI breaking).
Finally, @CaseyCarter reminded me that this constructor itself is non-Standard (this applies to the normal _HAS_EXCEPTIONS=1 implementation too) - that's tracked by #882.
Thanks for the quick response!
About the non-standard issue: I think it's not about the constructor of std::exception, but rather about the behavior of the derived classes like runtime_error(const char* what_arg) - there I would still expect a copy of the argument to be made.
About the non-standard issue: I think it's not about the constructor of
std::exception, but rather about the behavior of the derived classes likeruntime_error(const char* what_arg)- there I would still expect a copy of the argument to be made.
Ah, I see - thanks for the clarification. It hadn't occurred to me the the standard constructors in the derived classes are implemented in terms of this non-standard constructor in exception.
We talked about this at the weekly maintainer meeting and we believe that in the _HAS_EXCEPTIONS=0 case we should do what we do for _HAS_EXCEPTIONS=1: try to copy the string, and if we can't allocate memory, then just store nothing. (Separately we should get rid of that non-Standard base class constructor although doing so will be an epic undertaking.)