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

Check dynamic precision at compile time #1614

Closed
SanderBouwhuis opened this issue Apr 1, 2020 · 6 comments
Closed

Check dynamic precision at compile time #1614

SanderBouwhuis opened this issue Apr 1, 2020 · 6 comments

Comments

@SanderBouwhuis
Copy link

I'm probably an idiot, but isn't fmt supposed to check parameters at compile time for safety?

I have this code (which compiles without errors):

wstr += fmt::format(L"{:<{}} {}",
  bUppercaseHeaders ? Safe_ToUpper(pwcValue, _countof(pwcValue), m_pCols[i16Col].m_pwcName) : m_pCols[i16Col].m_pwcName,
  bPrintGrid ? wcVertChar : L'\t');

Of course, this crashed at run-time because I was missing the length parameter.
This fixes it:

wstr += fmt::format(L"{:<{}} {}",
  bUppercaseHeaders ? Safe_ToUpper(pwcValue, _countof(pwcValue), m_pCols[i16Col].m_pwcName) : m_pCols[i16Col].m_pwcName,
  pu32ColLen[i16Col],
  bPrintGrid ? wcVertChar : L'\t');

Why didn't I get a compile-time error?
Do I need to change a preprocessor switch to enable compile-time checking of the parameters?

@vitaut
Copy link
Contributor

vitaut commented Apr 1, 2020

You need to wrap the format string in FMT_STRING as shown in the docs: https://fmt.dev/latest/api.html#compile-time-format-string-checks

@vitaut vitaut closed this as completed Apr 1, 2020
@SanderBouwhuis
Copy link
Author

SanderBouwhuis commented Apr 1, 2020

It still doesn't work for me. This compiles, even though it's CLEARLY missing a parameter.
I know it's allowed to supply MORE parameters than needed, but surely not LESS?!?

wstrTest = fmt::format(FMT_STRING(L"{:.{}f}"), dValue);

This is what it should be without bugs:

wstrTest = fmt::format(FMT_STRING(L"{:.{}f}"), dValue, u8TotDecimals);

What else am I doing wrong?

@vitaut
Copy link
Contributor

vitaut commented Apr 1, 2020

Looks like the check in dynamic precision is missing.

@vitaut vitaut reopened this Apr 1, 2020
@SanderBouwhuis
Copy link
Author

SanderBouwhuis commented Apr 2, 2020

Also, I found some other strange behaviour:

double  dValue = 1.23456;
size_t  szLen = 0;
uint8   u8TotDecimals = 3;
wchar_t pwcA[256], pwcB[256], pwcC[256], pwcD[256];

pwcA[szLen = fmt::format_to_n(pwcA, _countof(pwcA)-1, FMT_STRING( "Test = {:.{}}"),  dValue, u8TotDecimals).size] = L'\0';
pwcB[szLen = fmt::format_to_n(pwcB, _countof(pwcB)-1, FMT_STRING( "Test = {:.{}f}"), dValue, u8TotDecimals).size] = L'\0';
pwcC[szLen = fmt::format_to_n(pwcC, _countof(pwcC)-1, FMT_STRING(L"Test = {:.{}}"),  dValue, u8TotDecimals).size] = L'\0';
pwcD[szLen = fmt::format_to_n(pwcD, _countof(pwcD)-1, FMT_STRING(L"Test = {:.{}f}"), dValue, u8TotDecimals).size] = L'\0';

Result:

pwcA = L"1.23"    (utf-8 format,   no  type specifier)
pwcB = L"1.235"   (utf-8 format,   'f' type specifier)
pwcC = L"1.23"    (wchar_t format, no  type specifier)
pwcD = L"1.235"   (wchar_t format, 'f' type specifier)
  1. Are they supposed to be different?
  2. Is supplying an utf-8 format string faster than supplying a wchar_t format string? Is so, is that also true considering the result is ending up in a wchar_t string?

@vitaut
Copy link
Contributor

vitaut commented Apr 2, 2020

Are they supposed to be different?

If you are asking whether the default and f are different then yes. Please see the docs: https://fmt.dev/latest/syntax.html

Is supplying an utf-8 format string faster than supplying a wchar_t format string? Is so, is that also true considering the result is ending up in a wchar_t string?

Normally yes.

@vitaut vitaut changed the title Check parameters at compile time(?) Check dynamic precision at compile time Apr 4, 2020
@vitaut
Copy link
Contributor

vitaut commented Apr 22, 2020

Invalid argument ids in dynamic width/precision are now reported at compile time. For example

#include <fmt/format.h>

int main() {
  fmt::print(FMT_STRING("{0:{1}}"), 42);
}

gives a compilation error because argument 1 doesn't exist:

In file included from test.cc:1:
include/fmt/format.h:2726:27: error: constexpr variable 'invalid_format' must be
initialized by a constant expression
  FMT_CONSTEXPR_DECL bool invalid_format =
                          ^
include/fmt/core.h:1727:3: note: in instantiation of function template specialization
'fmt::v6::internal::check_format_string<int, FMT_COMPILE_STRING, 0>' requested here
  check_format_string<Args...>(format_str);
  ^
...
include/fmt/core.h:569:26: note: in call to
'&checker(s, {}).context_->on_error(&"argument not found"[0])'
    if (id >= num_args_) on_error("argument not found");
                         ^

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