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

fmt::format/print(FMT_STRING("noarg string")) is broken in gcc with c++11 set. #2039

Closed
yeswalrus opened this issue Dec 1, 2020 · 11 comments

Comments

@yeswalrus
Copy link
Contributor

yeswalrus commented Dec 1, 2020

Found while working on #2038. Adding

EXPECT_EQ("42", fmt::format(FMT_STRING("42")));

to format-test.cc creates completely inscrutable error messages. The underlying issue appears to be a problem with make_args_checked when called with zero arguments in the parameter pack. This should either be worked around, or a platform dependent static assert added to make this error message more tractable and the limitation documented.
Arguably this is an incorrect use of the API anyways, but a better error message could likely save someone pain. It just shows up as a index out of range error in format.h, and since it only happens in GCC, there's basically no info about which source line actually caused it.

@vitaut
Copy link
Contributor

vitaut commented Dec 1, 2020

FMT_STRING requires C++14 so errors are expected. A PR to clarify the requirement is welcome.

@yeswalrus
Copy link
Contributor Author

yeswalrus commented Dec 1, 2020

That sort of surprises me - There's no such requirement documented anywhere I could find, including in the tests, and it seems to work just fine in gcc 4.8 with -std=c++11 as long as there are no formatting errors, and more than one argument is passed. What do we want to do here, just add docs? It seems to me it'd be better to add some kind of enforcement, but I'll put together a doc change first.

@vitaut
Copy link
Contributor

vitaut commented Dec 3, 2020

Ah, you are right. FMT_STRING does work with C++11 where it is a noop. The error was fixed in #2042.

@vitaut vitaut closed this as completed Dec 3, 2020
@yeswalrus
Copy link
Contributor Author

yeswalrus commented Dec 3, 2020

Repro case is in the title, and remains unresolved: https://godbolt.org/z/azTEns. It appears to be triggered on all versions of GCC. This issue is similar to but different from the one referred to and fixed in #2042 - Edit: my bad. The issue is that in GCC with -c++11, FMT_STRING just doesn't cause compile-time code generation, not that it fails to compile normally. It will fail to compile when placed in format-test.cc due to the additional checks added by FMT_PEDANTIC however. I'm currently trying to repro that with godbolt

@vitaut
Copy link
Contributor

vitaut commented Dec 3, 2020

The issue is that in GCC with -c++11, FMT_STRING just doesn't cause compile-time code generation, not that it fails to compile

That part is expected:

FMT_STRING does work with C++11 where it is a noop.

@alexezeder
Copy link
Contributor

FMT_STRING does work with C++11 where it is a noop. The error was fixed in #2042.

@vitaut But this PR is only about FMT_COMPILE as far as I understand

@vitaut
Copy link
Contributor

vitaut commented Dec 3, 2020

But this PR is only about FMT_COMPILE as far as I understand

Yeah, that PR turned out to be unrelated.

@yeswalrus
Copy link
Contributor Author

Update - I cannot seem to replicate this with godbolt. Locally it was failing to compile with -Warray-bounds but wiping cache seems to have resolved it. Sorry for the fuss

@vitaut
Copy link
Contributor

vitaut commented Dec 3, 2020

No worries, glad it's resolved now.

@yeswalrus
Copy link
Contributor Author

Could we re-open this? I'm now seeing it again both locally and on the build servers for #2049

@vitaut
Copy link
Contributor

vitaut commented Dec 15, 2020

Sure, but please provide a repro.

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

3 participants