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

Header-only mode still includes cassert, causing assert macro to be defined. #2148

Closed
toojays opened this issue Feb 24, 2021 · 1 comment
Closed

Comments

@toojays
Copy link
Contributor

toojays commented Feb 24, 2021

Consider the following example:

#define FMT_HEADER_ONLY
#include <fmt/format.h>
//#undef assert

struct Test
{
    void assert (bool pass, const char *test_name)
    {
        fmt::print("{}: {}", test_name, pass ? "PASS" : "FAIL");
    }
};

int main()
{
    Test test;
    test.assert(true, "mumble");
    return 0;
}

It fails compilation (godbolt) with:

<source>:7:50: error: macro "assert" passed 2 arguments, but takes just 1
    7 |     void assert (bool pass, const char *test_name)
      |                                                  ^
In file included from /opt/compiler-explorer/gcc-10.2.0/include/c++/10.2.0/cassert:44,
                 from /opt/compiler-explorer/libs/fmt/trunk/include/fmt/format-inl.h:12,
                 from /opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:4230,
                 from <source>:2:
/usr/include/assert.h:92: note: macro "assert" defined here
   92 | #  define assert(expr)       \
      | 
<source>:16:31: error: macro "assert" passed 2 arguments, but takes just 1
   16 |     test.assert(true, "mumble");
      |                               ^
In file included from /opt/compiler-explorer/gcc-10.2.0/include/c++/10.2.0/cassert:44,
                 from /opt/compiler-explorer/libs/fmt/trunk/include/fmt/format-inl.h:12,
                 from /opt/compiler-explorer/libs/fmt/trunk/include/fmt/format.h:4230,
                 from <source>:2:
/usr/include/assert.h:92: note: macro "assert" defined here
   92 | #  define assert(expr)       \
      | 
<source>:7:10: error: variable or field 'assert' declared void
    7 |     void assert (bool pass, const char *test_name)
      |          ^~~~~~
<source>:10:5: error: expected ';' at end of member declaration
   10 |     }
      |     ^
      |      ;
<source>: In function 'int main()':
<source>:16:10: error: 'struct Test' has no member named 'assert'
   16 |     test.assert(true, "mumble");
      |          ^~~~~~

As a workaround we can #undef assert immediately after including format.h.

It looks like some work was done to remove the dependency on cassert in 2f9acd1, but this did not extend to the header only mode?

Is it just a matter of replacing the remaining assert() calls with FMT_ASSERT()?

@vitaut
Copy link
Contributor

vitaut commented Feb 24, 2021

Removing the assert dependency from the implementation is a non-goal but I'm open to a PR that replaces it with FMT_ASSERT.

@vitaut vitaut closed this as completed Feb 24, 2021
toojays added a commit to toojays/fmt that referenced this issue 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.
vitaut pushed a commit that referenced this issue Mar 4, 2021
* Don't include <cassert>. (#2148)

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

* FMT_GCC_VERSION is not defined when we include test-assert.h, use __GCC__ instead.

* Don't explicitly suppress GCC's -Wterminate in tests' FMT_ASSERT.

Throwing from a separate function is enough to silence the warning, no need to
explicitly suppress it.

* Remove messages from assertions added in 2f699d2.

* Correct formatting around throw_assertion_failure().
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

No branches or pull requests

2 participants