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

Don't include <cassert>. (#2148) #2152

Merged
merged 5 commits into from Mar 4, 2021
Merged

Don't include <cassert>. (#2148) #2152

merged 5 commits into from Mar 4, 2021

Conversation

toojays
Copy link
Contributor

@toojays toojays commented Feb 25, 2021

This commit replaces use of the assert() macro in format-inl.h with
FMT_ASSERT(). This allows us to drop the cassert include.

I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.

Note that the change to FMT_ASSERT in test-assert.h is to avoid the following warning:

In file included from /home/jscott/src/fmt/test/format-impl-test.cc:10:
/home/jscott/src/fmt/include/fmt/format-inl.h: In destructor ‘fmt::v7::detail::bigint::~bigint()’:
/home/jscott/src/fmt/test/test-assert.h:26:52: warning: throw will always call terminate() [-Wterminate]
   26 |   if (!(condition)) throw assertion_failure(message);
      |                                                    ^
/home/jscott/src/fmt/include/fmt/format-inl.h:1300:15: note: in expansion of macro ‘FMT_ASSERT’
 1300 |   ~bigint() { FMT_ASSERT(bigits_.capacity() <= bigits_capacity, ""); }
      |               ^~~~~~~~~~
/home/jscott/src/fmt/test/test-assert.h:26:52: note: in C++11 destructors default to noexcept
   26 |   if (!(condition)) throw assertion_failure(message);
      |                                                    ^
/home/jscott/src/fmt/include/fmt/format-inl.h:1300:15: note: in expansion of macro ‘FMT_ASSERT’
 1300 |   ~bigint() { FMT_ASSERT(bigits_.capacity() <= bigits_capacity, ""); }
      |               ^~~~~~~~~~

Actually it seems that just moving the throw into a separate function (even inline) is enough to squelch the warning, with GCC 9.3 at least. In the end I went with this more explicit version.

This commit replaces use of the assert() macro in format-inl.h with
FMT_ASSERT(). This allows us to drop the cassert include.
Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

include/fmt/format-inl.h Outdated Show resolved Hide resolved
test/test-assert.h Outdated Show resolved Hide resolved
Throwing from a separate function is enough to silence the warning, no need to
explicitly suppress it.
@toojays toojays requested a review from vitaut March 1, 2021 22:50
test/test-assert.h Outdated Show resolved Hide resolved
test/test-assert.h Outdated Show resolved Hide resolved
@toojays toojays requested a review from vitaut March 3, 2021 09:22
@vitaut vitaut merged commit 772aeca into fmtlib:master Mar 4, 2021
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.

None yet

2 participants