allow some printf calls in @safe code

[WalterBright]

With the move to @safe by default, this will make using @System printf easier to use.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#22145"

[thewilsonator]

surely this should also check for malformatted arguments too. Also %s is not the only dangerous format specifier.

[WalterBright]

Malformed arguments are picked up later by checkPrintfFormat(). I cannot think of another dangerous format specifier. Which ones do you see?

[rikkimax]

Nic has asked me to check some specs for unsafe options.

For FreeBSD & Debian:

%n writes to the integer pointer, as long as its typed correctly that should be fine.

%s and %S are indeed the big ones I think.

For Windows:

As above: %n %s %S

But also %Z which I bet this doesn't consider.

[WalterBright]

%n is taken care of by checkPrintfFormat().

%S and %Z are not in the C11 Standard and will error out int checkPrintfFormat().

[thewilsonator]

in which case please add them to the test case. Also does this apply to printf with pragma(printf) not applied.

[thewilsonator]

meant to request changes

[WalterBright]

pragma(printf) must be applied to the printf declaration in order to trigger this feature.

See https://cplusplus.com/reference/cstdio/printf/ and %S and %Z are not part of the Standard, and are currently rejected by the compiler checks. @rikkimax I'd like to review where they are coming from so we can determine what to do about them.

Note that nobody has used them yet, else we would have heard about it.

[rikkimax]

pragma(printf) must be applied to the printf declaration in order to trigger this feature.

See https://cplusplus.com/reference/cstdio/printf/ and %S and %Z are not part of the Standard, and are currently rejected by the compiler checks. @rikkimax I'd like to review where they are coming from so we can determine what to do about them.

Note that nobody has used them yet, else we would have heard about it.

%Z is from MS printf implementation, I linked that.

%S is basically in all implementations in some form or another, usually as an alias to a variant of %s.

(Not in C99 or C11, but in SUSv2, SUSv3, and SUSv4.) Synonym for ls. Don't use.

https://manpages.debian.org/trixie/manpages-dev/printf.3.en.html

[thewilsonator]

they are GNU extensions/POSIX iirc, if they are rejected outright by the compiler then fine, but we should have aa test for that

[rikkimax]

they are GNU extensions/POSIX iirc, if they are rejected outright by the compiler then fine, but we should have aa test for that

Not an extension, comes from SUS aka Unix.

[WalterBright]

From https://manpages.debian.org/trixie/manpages-dev/printf.3.en.html :

Z A nonstandard synonym for z that predates the appearance of z. Do not use in new code.

S (Not in C99 or C11, but in SUSv2, SUSv3, and SUSv4.) Synonym for ls. Don't use.

(emphasis added)

They're already rejected by checkPrintfFormat(). They've been obsoleted long ago, and the user can substitute z and ls to correct any issue. We shouldn't get stuck supporting those.

[rikkimax]

That covers posix behavior, but not the Windows one for %Z

It takes: https://learn.microsoft.com/en-us/windows/win32/api/ntdef/ns-ntdef-string or https://learn.microsoft.com/en-us/windows/win32/api/ntdef/ns-ntdef-_unicode_string

[WalterBright]

The two cites do not mention %Z. And if %Z does the same thing as the Standard %z, there is no purpose in supporting that extension.

[rikkimax]

But this does: https://learn.microsoft.com/en-us/cpp/c-runtime-library/format-specification-syntax-printf-and-wprintf-functions?view=msvc-170

Which I linked in my first comment.

[WalterBright]

It says "VS 2013 and earlier". It was obsolete 12 years ago.

[rikkimax]

It says "VS 2013 and earlier". It was obsolete 12 years ago.

It mentions UCRT, so it isn't obsolete.

I'll verify.

[rikkimax]

It's kernel only, so I can't test it. So yeah we can ignore that.

[WalterBright]

Wasn't the change request resolved?

[thewilsonator]

There have been no further pushes to the branch since that request was made, so no. Still need to add those to the test case.

[WalterBright]

They're already taken care of because an error is issued if they are used.

[thewilsonator]

then please add those to the test case