Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add formatter for std::exception #3062

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

zach2good
Copy link
Contributor

@zach2good zach2good commented Aug 27, 2022

Adds a single simple formatter for std::exception and accompanying test.

Reference:
#2977
#3012

Closes #2977

@zach2good
Copy link
Contributor Author

Will come back and fix those failing CI jobs in a couple of days 👍

@zach2good zach2good force-pushed the std_exception_formatter branch from 1cc09bc to 7da2a28 Compare August 31, 2022 11:33
include/fmt/std.h Outdated Show resolved Hide resolved
@zach2good zach2good force-pushed the std_exception_formatter branch 3 times, most recently from b5c02fc to 2821d2c Compare August 31, 2022 12:40
test/std-test.cc Outdated Show resolved Hide resolved
Co-authored-by: fekir <[email protected]>
Co-authored-by: Alexey Ochapov <[email protected]>
Co-authored-by: Vladislav Shchapov <[email protected]>
@zach2good zach2good force-pushed the std_exception_formatter branch from 2821d2c to 76adb05 Compare August 31, 2022 13:00
@Baardi
Copy link
Contributor

Baardi commented Sep 2, 2022

Sorry for a stupid, uneducated question.
But does it make sense to include the exception type as well, when formatting?

E.g. a std::runtime_error("My exception") would then be formatted as "std::runtime_error: My exception", as it typically is formatted in C#, by using typeid(xxx).name()?
Or is that what you previously discussed, and decided against it due to portability issues?

@zach2good
Copy link
Contributor Author

Not necessarily previously discussed (@vitaut's suggestions in #3012 (comment)), but you're right in that it isn't super portable to do that:

Using:

return fmt::formatter<string_view>::format(fmt::format("{}: {}", typeid(T).name(), ex.what()), ctx);

clang (name mangling):

/mnt/c/dev/fmt/test/std-test.cc:88: Failure
Expected equality of these values:
  fmt::format("{}", ex)
    Which is: "St9exception: Test Exception"
  str
    Which is: "Test Exception"
/mnt/c/dev/fmt/test/std-test.cc:89: Failure
Expected equality of these values:
  fmt::format("{:?}", ex)
    Which is: "\"St9exception: Test Exception\""
  escstr
    Which is: "\"Test Exception\""
/mnt/c/dev/fmt/test/std-test.cc:95: Failure
Expected equality of these values:
  fmt::format("{}", ex)
    Which is: "St13runtime_error: Test Exception"
  str
    Which is: "Test Exception"
/mnt/c/dev/fmt/test/std-test.cc:96: Failure
Expected equality of these values:
  fmt::format("{:?}", ex)
    Which is: "\"St13runtime_error: Test Exception\""
  escstr
    Which is: "\"Test Exception\""

MSVC (addition of "class"):

C:\dev\fmt\test\std-test.cc(88): error: Expected equality of these values:
  fmt::format("{}", ex)
    Which is: "class std::exception: Test Exception"
  str
    Which is: "Test Exception"
C:\dev\fmt\test\std-test.cc(89): error: Expected equality of these values:
  fmt::format("{:?}", ex)
    Which is: "\"class std::exception: Test Exception\""
  escstr
    Which is: "\"Test Exception\""
C:\dev\fmt\test\std-test.cc(95): error: Expected equality of these values:
  fmt::format("{}", ex)
    Which is: "class std::runtime_error: Test Exception"
  str
    Which is: "Test Exception"
C:\dev\fmt\test\std-test.cc(96): error: Expected equality of these values:
  fmt::format("{:?}", ex)
    Which is: "\"class std::runtime_error: Test Exception\""
  escstr
    Which is: "\"Test Exception\""

@Baardi
Copy link
Contributor

Baardi commented Sep 2, 2022

For gcc you can demangle the name, and for msvc you can strip the class I guess. Not sure about clang.

Searching online, it generally seems like a doable thing, at least for the 3 main compilers, although it would introduce a lot of complexity that might not not be wanted in fmtlib.

@vitaut vitaut merged commit 4191477 into fmtlib:master Sep 2, 2022
@vitaut
Copy link
Contributor

vitaut commented Sep 2, 2022

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support std::exception
5 participants