warning with conditional operator and printf

[Quuxplusone]

Bugzilla Link	PR11313
Status	NEW
Importance	P enhancement
Reported by	Axel Gonzalez (loox@e-shell.net)
Reported on	2011-11-04 13:15:38 -0700
Last modified on	2014-11-10 18:50:06 -0800
Version	trunk
Hardware	PC FreeBSD
CC	dimitry@andric.com, kremenek@apple.com, lavr@ncbi.nlm.nih.gov, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

The is a false positive in the format checker, when using conditional operator
and parameters get promoted.

There is more info in this thread:

http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-November/018464.html

--

clang -Wall -o ntohs2 ntohs2.c
ntohs2.c:10:12: warning: conversion specifies type 'unsigned short' but the
argument has type
      'int' [-Wformat]
        printf("%hu\n", x < 0 ? x : y);
                ~~^     ~~~~~~~~~~~~~
                %d
1 warning generated.

--

#include <stdio.h>
#include <netinet/in.h>

int main()
{
    uint16_t x = htons(80);
    uint16_t y = ntohs(80);
    x = x < 0 ? x : y;
    printf("%hu\n", y);
    printf("%hu\n", x < 0 ? x : y);
    return (0);
}

[Quuxplusone]

cloned to rdar://problem/10398045

[Quuxplusone]

(In reply to comment #0)
> The is a false positive in the format checker, when using conditional operator
> and parameters get promoted.
>
> There is more info in this thread:
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-November/018464.html
>
> --
>
> clang -Wall -o ntohs2 ntohs2.c
> ntohs2.c:10:12: warning: conversion specifies type 'unsigned short' but the
> argument has type
>       'int' [-Wformat]
>         printf("%hu\n", x < 0 ? x : y);
>                 ~~^     ~~~~~~~~~~~~~
>                 %d
> 1 warning generated.
>

Looking at this again, I'm not certain if this should be "fixed".  Technically,
the values aren't shorts anymore by the time they are passed to the printf
function.  The usual arithmetic conversions apply for the ternary operator.

Why not just write:

        printf("%u\n", x < 0 ? x : y);

[Quuxplusone]

Unfortunately this is *not* a false positive.  Due to the Usual Arithmetic
Conversions, the type of the ternary expression becomes int, even if both
'branches' in the expression are shorts (unsigned or not).  Try, for example,
the following program with either gcc or clang, and you will notice it prints
sizeof(int), not sizeof(short):

  #include <stdio.h>

  int main(void)
  {
    printf("%zu\n", sizeof(1 > 2 ? (short)1 : (short)2));
    return 0;
  }

So clang is actually right to warn here.  You should cast the ternary
expression to uint16_t to get rid of this warning, or use the printf format
string for a regular int.

As to technically being ints, as Ted remarks, that is true, but for the sake of
the printf format checker, that should be ignored.  Otherwise clang would also
have to warn about:

    printf("%hd\n", (short)42);

or any other passed argument with a type less than int, which is obviously not
useful.  That the arguments are always converted to 'machine words' (because of
the argument passing mechanism, e.g. the stack, or registers) should be
considered an implementation detail.

[Quuxplusone]

I give this some extra tought too

this is what I came with:

take this prgram:

#include <stdio.h>

int main()
{
    unsigned short x = 2;
    unsigned short y = 3;
    unsigned short z = 0;

    z = (x > y ? x : y);

    printf("%hu\n", (x > y ? x : y));
    printf("%hu\n", z);
    printf("%hu\n", z = (x > y ? x : y));

    return (0);
}

% clang -o trin trin.c
trin.c:11:15: warning: conversion specifies type 'unsigned short' but the
argument has type
      'int' [-Wformat]
    printf("%hu\n", (x > y ? x : y));
            ~~^     ~~~~~~~~~~~~~~~
            %d
1 warning generated.

The 3 printf should (and do) print the same the same value.

This is not a violation of the standard (http://lists.cs.uiuc.edu/pipermail/cfe-
dev/2011-November/018465.html), since the values in the first printf are
upgraded to int.

This is however an inconsistency of itself, while the values are not upgraded
on the = , they are upgraded on the printf. While not a violation of the
standard, it can cause confusion (this is what i posted it in the first place).

If this is not goint to be fixed, this at least should be well documented, that
the result is upgraded in some cases (printf) and not in others (=)

Note that this same behaviour is in other operators like + .

Note2: This doesn't generate a warning in gcc (4.2.1)

[Quuxplusone]

On 2012-01-26 08:46, Axel Gonzalez <loox@e-shell.net> wrote:
> The 3 printf should (and do) print the same the same value.

Indeed, in the end this is all a bit academic, since the result is the
same.

However, the goal of the printf format warnings is to make sure the
actual data type of the passed arguments matches the format specifiers
used.

In the first printf, the final data type of the second argument is int,
thus it does not match %hu.  Therefore clang emits a warning.

In the second and third printfs, the data type of the second argument is
short, so this is just fine.

> This is not a violation of the standard
> (http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-November/018465.html), since
> the values in the first printf are upgraded to int.
>
> This is however an inconsistency of itself, while the values are not upgraded
> on the = , they are upgraded on the printf.

The ternary expression is still promoted to int, but then implicitly
converted back to unsigned short, when you assign it to 'z'.  Thus there
is no need for a warning in that case.

> While not a violation of the
> standard, it can cause confusion (this is what i posted it in the first
place).
>
> If this is not goint to be fixed, this at least should be well documented,
that
> the result is upgraded in some cases (printf) and not in others (=)

Well, if there is anything confusing, then it is the C standard, which
says such expressions are promoted to int. :)

As said, in the assignment case, the expression is still promoted, you
just won't notice it in any way.

> Note that this same behaviour is in other operators like + .
>
> Note2: This doesn't generate a warning in gcc (4.2.1)

The fact that gcc doesn't warn about any types smaller than int, could
be considered a deficiency (although it may be a conscious design
choice, I don't know).  For example, even the latest top-of-tree gcc
(using -Wformat=2) does *not* warn about the following:

  #include <stdio.h>

  int main(void)
  {
    int i = 42;
    printf("%hu\n", i);
    return 0;
  }

while clang gives the expected:

  warning: conversion specifies type 'unsigned short' but the argument has type 'int' [-Wformat]
      printf("%hu\n", i);
          ~~^     ~
          %d

[Quuxplusone]

(In reply to comment #5)
> On 2012-01-26 08:46, Axel Gonzalez <loox@e-shell.net> wrote:
> > The 3 printf should (and do) print the same the same value.
>
> Indeed, in the end this is all a bit academic, since the result is the
> same.
>
> However, the goal of the printf format warnings is to make sure the
> actual data type of the passed arguments matches the format specifiers
> used.

I disagree.  The point of the warning is to find problems.  A potential problem
may come from a type mistmatch, but in the cases where it is a non-issue, I
think it is completely reasonable to debate whether or not there should be a
warning here or not.  On one side of the argument we can just say "always
conform the type with the format specifier", which is essentially a zero
tolerance policy.  On the other side one can make allowances for code this is
essentially safe.  The disadvantage of that approach is that it is a slippery
slope, but could result in a better experience for users.

[Quuxplusone]

> but could result in a better experience for users

I could not agree more.  We recently began using Clang in our routine builds,
and the nonsense warnings from printf()'s are just annoying.  Not only with
ternaries, but also with constants, like this:

#define NUMBER 1
printf("%hu", NUMBER);

In particular case, Clang could have analyzed that the constant is within the
short's range and did not emit any warnings, but it does!

There's more.  In general case, all small integers get promoted to ints in the
ellipsis portion of printf()'s arguments, so from the examples above, having

printf("%hd",      x < y ? x : y);
or
printf("%hd", z = (x < y ? x : y));

does not make a difference at all:  even if there was a violation of the
short's range in the "promoted" expression, which presumably the warning is
aiming at (not necessarily ternary, but like "x + 1"), it would be always cut
off by the assignment, just the same way as the "%hd" specifier would do by
taking the proper "part" of the integer argument from stack.  So nothing was
actually saved here;  both statements produce the same output, with or without
the explicit assignment, so why worry?  The warning here just create a lot of
unnecessary noise...