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

No validation on number of arguments? #492

Closed
bryceschober opened this issue Apr 3, 2017 · 9 comments
Closed

No validation on number of arguments? #492

bryceschober opened this issue Apr 3, 2017 · 9 comments

Comments

@bryceschober
Copy link

@bryceschober bryceschober commented Apr 3, 2017

There doesn't seem to be any validation on the number of arguments passed matching the number in the format, at least in my usage of the writer api. Since fmtlib was advertised as a "safe" alternative to printf(), I guess I assumed that it would implement an equivalent of the compile-time checking of -Wformat, which does warn about argument count mismatches. Am I doing something wrong, or just expecting too much of fmtlib?

@vitaut
Copy link
Contributor

@vitaut vitaut commented Apr 5, 2017

Unfortunately compile-time checks are not supported although there are some tricks in #62 for printf formatting.

As for checking the arguments, the library follows Python's str.format conventions and reports an error when trying to access non-existent argument (or argument of a wrong type), but it's OK to pass more arguments than needed.

@vitaut vitaut closed this Apr 5, 2017
@foonathan
Copy link
Collaborator

@foonathan foonathan commented Apr 5, 2017

Have you considered doing something with UDLs?

Like: "Hello, {}!"_format which creates a format_str<1> object containing the format string itself and the number of arguments. Then simply another format() overload accepting this objects that can do static_assert() with sizeof...(Args). With C++14 constexpr it's pretty straightforward, with 11 also easy, just a little verbose and ugly.

@dean0x7d
Copy link
Contributor

@dean0x7d dean0x7d commented Apr 6, 2017

@foonathan Unfortunately, UDLs don't help here because there is no way to get type-system constants out of a constexpr function. The following seems like it should work, but it does not:

#include <type_traits>

constexpr auto operator""_size(char const*, std::size_t size) {
    return std::integral_constant<std::size_t, size>{};
    //                                         ^~~~
    // error: non-type template argument is not a constant expression
}

int main() {
    static_assert("hello"_size.value == 5, "");
}

The issue is that constexpr functions must be callable with either compiletime or runtime arguments, so no argument-dependent types can be generated. Up to and including C++17, the only way to get around this is by using macros... but it's as ugly as it sounds.

This could be solved if future standards allow overloading on the constexprness of the arguments or perhaps with static reflection (I haven't kept up with the reflection proposals, so I'm just guessing on the latter).

@foonathan
Copy link
Collaborator

@foonathan foonathan commented Apr 6, 2017

There's a UDL version that takes the chars at non-type template argument... but not for string literals, damn it, you're right.

@vitaut
Copy link
Contributor

@vitaut vitaut commented Nov 6, 2017

Compile-time format string checks are now available: http://zverovich.net/2017/11/05/compile-time-format-strings.html

@fulara
Copy link

@fulara fulara commented Nov 19, 2018

@vitaut but that does not solve mentioned issue right?
there is still no validation whether the number of {} matches the number of specified arguments, if they are without indices?
Should I create a issue/featurerequest for that? or is this behavior something you dont want at all?

@vitaut
Copy link
Contributor

@vitaut vitaut commented Nov 19, 2018

There is a validation in a sense that you cannot refer to nonexisting argument. Passing extra ones is OK as explained earlier.

@fulara
Copy link

@fulara fulara commented Nov 19, 2018

Yes, and I dont see good reason why.
even printf provides warning:

<source>:8:22: warning: too many arguments for format [-Wformat-extra-args]
printf("%d", 1, 2);

Just today I had an issue where I passed extra one..

Can you please clarify whether this is concious decision or a limitation of the language? Because if you were able to achieve checking of {1} then I would hope that checking of argument could would be possible as well? :)

@vitaut
Copy link
Contributor

@vitaut vitaut commented Nov 21, 2018

Can you please clarify whether this is concious decision or a limitation of the language?

This is a conscious decision. You can think of it as having unused arguments in a function - there are valid use cases for that. If we had a way of reporting warnings, it could be an opt-in warning as in printf case, but it shouldn't be a hard error.

leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any error (either runtime or compile-time) [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any error (either runtime or compile-time) [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
leoetlino added a commit to leoetlino/dolphin that referenced this issue Nov 19, 2020
Unfortunately, {fmt} allows passing too many arguments to a format call
without raising any runtime or compile-time error [1].

As this is a common source of bugs since we started migrating to {fmt},
this commit adds some custom logic to validate the number of
replacement fields in format strings in addition to {fmt}'s own checks.

[1] fmtlib/fmt#492
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants