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

<format>: dynamic width or precision of non-integral type should be rejected #2785

Closed
mordante opened this issue Jun 12, 2022 · 5 comments · Fixed by #4155
Closed

<format>: dynamic width or precision of non-integral type should be rejected #2785

mordante opened this issue Jun 12, 2022 · 5 comments · Fixed by #4155
Labels
bug Something isn't working fixed Something works now, yay! format C++20/23 format

Comments

@mordante
Copy link
Contributor

mordante commented Jun 12, 2022

Describe the bug
Per [format.string.std]/7

… If the corresponding formatting argument is not of integral type 
… an exception of type format_­error is thrown.

When using a non-integral type as arg-id in std::format the code violates [format.fmt.string]/3 and is therefore ill-formed. The STL incorrectly accepts the format string, however, and throws an exception at run-time https://godbolt.org/z/b868c3M8z.

#include <format>
#include <iostream>

int main() {
    try {
        std::cout << std::format("{:*^{}}\n", 'a', 1.1);
    } catch(...) {
        std::cout << "die\n";
    }
    try {
        std::cout << std::format("{:.{}}\n", "abc", 1.1);
    } catch(...) {
        std::cout << "die\n";
    }
}
@CaseyCarter CaseyCarter added bug Something isn't working format C++20/23 format labels Jun 12, 2022
@cpplearner
Copy link
Contributor

cpplearner commented Jun 13, 2022

Currently _Basic_format_string uses _Format_checker which calls _Compile_time_parse_format_specs which simply calls formatter::parse, which does not know the type of other formatting arguments.

It might be possible to e.g. make _Format_checker handle known types specially, but should it do something that can't be mimicked by user-defined formatters?

(Aside: MSVC STL requires /utf-8 for compile-time format string checking. With the default execution character set, the checking is disabled.)

@CaseyCarter CaseyCarter changed the title <format>: using the width and precision option with a non-integral type as arg-id is well-formed <format>: dynamic width or precision of non-integral type should be rejeceted Jun 13, 2022
@CaseyCarter CaseyCarter changed the title <format>: dynamic width or precision of non-integral type should be rejeceted <format>: dynamic width or precision of non-integral type should be rejected Jun 13, 2022
@frederick-vs-ja
Copy link
Contributor

The sentence is changed in C++26 by WG21-P2757R3, which seemingly claims that a very similar case (std::format("{:>{}}", "hello", "10")) is well-formed in C++23 (or C++20 with DRs).

If the The option is valid only if the corresponding formatting argument is not of standard signed or unsigned integer type, or . If its value is negative, an exception of type format_error is thrown.

@CaseyCarter
Copy link
Contributor

CaseyCarter commented Nov 9, 2023

The sentence is changed in C++26 by WG21-P2757R3, which seemingly claims that a very similar case (std::format("{:>{}}", "hello", "10")) is well-formed in C++23 (or C++20 with DRs).

The paper is confused. That case (and the case in this thread) are clearly ill-formed per [format.fmt.string]/3. The issue the paper actually addresses is that [format.fmt.string]/3 isn't implementable for non-Standard format arguments with the current machinery in the Standard.

@frederick-vs-ja
Copy link
Contributor

The sentence is changed in C++26 by WG21-P2757R3, which seemingly claims that a very similar case (std::format("{:>{}}", "hello", "10")) is well-formed in C++23 (or C++20 with DRs).

The paper is confused. That case (and the case in this thread) are clearly ill-formed per [format.fmt.string]/3. The issue the paper actually addresses is that [format.fmt.string]/3 isn't implementable for non-Standard format arguments with the current machinery in the Standard.

Thanks. It seems that we should treat the wording change in that paragraph editorial...

@mordante
Copy link
Contributor Author

@StephanTLavavej To answer your question in #4152. I believe it's not incorrect since C++23, but since C++20.
http://eel.is/c++draft/format#string.std-10

If { arg-idopt } is used in a width or precision option, the value of the corresponding formatting argument
is used as the value of the option.
The option is valid only if the corresponding formatting argument is of standard signed or unsigned integer
type. If its value is negative, an exception of type format_error is thrown.

Since the value is not a "standard signed or unsigned integer type". It's not valid.

The wording in C++20 was slightly different
https://timsong-cpp.github.io/cppwp/n4861/format#string.std-7, but close enough. It changed by https://cplusplus.github.io/LWG/issue3720

@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay! format C++20/23 format
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants