Incorrect -Wformat type warnings with non-variadic functions

[sapphyrus]

This piece of code produces an incorrect warning when compiled with -Wformat:

template <typename... Args>
__attribute__((__format__(__printf__, 1, 2)))
void my_format(char const *format, Args... args);

void foo()
{
	float x = 0.f;

	my_format("%f", x);
}

<source>:9:18: warning: format specifies type 'double' but the argument has type 'float' [-Wformat]
        my_format("%f", x);
                   ~~   ^
                   %f

Meanwhile, declaring my_format as a variadic function results in no warning, as expected:

__attribute__((__format__(__printf__, 1, 2)))
void my_format(char const *format, ...);

void foo()
{
	float x = 0.f;

	my_format("%f", x);
}

It doesn't seem to be related to my_format being templated, declaring it as void my_format(char const *format, float f); results in the same incorrect warning.

[inclyc]

Looks like this occurs if my_format is not variadic. Might related to default argument promotions

int my_format(char const *format, float f) __attribute__ ((format (printf, 1, 2)));

https://godbolt.org/z/8TPqoMv5s

---

upd: Note that we expect my_format to be variadic, our codes may not concerned about non-variadic in this case. If you declare my_format variadic, float x will be promoted into double.

[inclyc]

I think it might be a little misunderstanding on the format attribute, we expect the function to be variadic, because that's how printf works. If you write it as a template or directly declared as float, this is inconsistent with the behavior that printf will implicitly convert float to double.

Our error messages and suggested fixes still look very odd. (suggested you change %f to %f)

[sapphyrus]

As far as I understood, ever since https://reviews.llvm.org/D112579, __attribute__((format)) is allowed on non-variadic functions?

[inclyc]

As far as I understood, ever since https://reviews.llvm.org/D112579, __attribute__((format)) is allowed on non-variadic functions?

Yes, this is now allowed, what I was trying to say in previous comments was that default argument promotion happens with "stdio" printf (which is variadic), which is different from templates or declared as "float" directly. These tests are interesting and I guess e3bd67e should have addressed this (but apparently not).

[erichkeane]

The error itself IS correct here (in that when it is a C variadic, all floats are promoted to doubles, but when it is a variadic template/template param, it stays a float). The suggested fix is obviously bonkers, and probably should only be there if the argument IS a double (or would be promoted to one).

[llvmbot]

@llvm/issue-subscribers-good-first-issue

[Aditya-pixel]

Hi, I wanted to work on this issue. I suggest the fix could be adding a check in the compatible types for type Double, if the type is Float then it should return Match. This case is not considered in e3bd67e, in the file FormatString.cpp Can you let me know if this fix is correct. If not, can you guide me with the changes.

[inclyc]

Hi, I wanted to work on this issue.

Thanks! Welcome new contributor :)

I suggest the fix could be adding a check in the compatible types for type Double, if the type is Float then it should return Match. This case is not considered in e3bd67e, in the file FormatString.cpp Can you let me know if this fix is correct. If not, can you guide me with the changes.

I'm not sure if this is correct, because as for variadic template/template param, actually there is no default argument promotion guaranteed. So, say "compatible" may not be correct. I wonder if we should give a warning saying "Note! No default argument promotion!". Your changes planned is another good choice because typically these variadic tempates call normal printf eventually.

WDYT? @erichkeane @AaronBallman

[AaronBallman]

I think the diagnostic is correct but the suggested fix is a bug. I think we might want to add a helper function to ASTContext to decide whether a type is promotable or not (we already have ASTContext::isPromotableIntegerType() but we don't have anything for half/float types and we probably should) and then use that to determine whether to issue the fix or not.

I don't think we want to warn users that there is no default argument promotion happening, but I'm still thinking about that. My intuition is that this pattern isn't super common to begin with and so we don't really know what users expect in that case, thus we should leave the diagnostic alone for the time being until we get more field experience with it.

[JoyNyuguto]

@sapphyrus @SamTebbs33 Joy Nyuguto Outreachy
The warning occurs because the format specifier "%f" is intended for double-precision floating-point values but the argument passed to my_format is a single-precision floating-point value of type float.

To resolve the warning in the templated version of my_format, you can explicitly cast the float argument to double:
my_format("%f", static_cast(x));
``

Alternatively, you can use a different format specifier that is intended for single-precision floating-point values, such as "%g" or "%.f"

my_format("%g", x); my_format("%.f", x);
In both cases, the warning should no longer be triggered because the format specifier matches the type of the argument.