Page MenuHomePhabricator
Differential D51211 Diff 178605 test/Sema/format-strings-bitfield-promotion.c

Changeset View

Standalone View

View Options

test/Sema/format-strings-bitfield-promotion.c

  • This file was added.
1​// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s
2​// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -fsyntax-only -verify %s
aaron.ballmanUnsubmitted
Not Done
Reply

Running your test through GCC looks like the behavior matches here for C; can you also add a C++ test that demonstrates the behavior does not change?

https://godbolt.org/z/zRYDMG

aaron.ballman: Running your test through GCC looks like the behavior matches here for C; can you also add a…
aaron.ballmanUnsubmitted
Not Done
Reply

Strangely, the above godbolt link dropped the output windows, here's a different link that shows the behavioral differences between C and C++ mode in GCC: https://godbolt.org/z/R3zRHe

aaron.ballman: Strangely, the above godbolt link dropped the output windows, here's a different link that…
ebevhanAuthorUnsubmitted
Not Done
Reply

Hmm, I'll have a look at this.

ebevhan: Hmm, I'll have a look at this.
ebevhanAuthorUnsubmitted
Done
Reply

That gcc godbolt is a bit odd. The type of the bitfield expression in the C++ example is long and not int, but in Clang, it's clearly being converted. If I change the example a bit, we get this warning:

<source>:11:12: warning: format '%d' expects argument of type 'int', but argument 2 has type 'long int' [-Wformat=]
   11 |   printf("%d", bf.a); // expected-warning {{format specifies type 'long' but the argument has type 'int'}}
      |           ~^   ~~~~
      |            |      |
      |            int    long int

But in Clang, we get a cast to int:

| `-ImplicitCastExpr 0xd190748 <col:17, col:20> 'int' <IntegralCast>
|   `-ImplicitCastExpr 0xd190730 <col:17, col:20> 'long' <LValueToRValue>
|     `-MemberExpr 0xd190618 <col:17, col:20> 'long' lvalue bitfield .a 0xd18f790
|       `-DeclRefExpr 0xd1905f8 <col:17> 'struct bitfields':'bitfields' lvalue Var 0xd18fa18 'bf' 'struct bitfields':'bitfields'

So gcc and Clang are doing things differently here.

The code in isPromotableBitField says:

// FIXME: C does not permit promotion of a 'long : 3' bitfield to int.
//        We perform that promotion here to match GCC and C++.

but clearly gcc isn't doing this in the C++ case. The comments also mention some things about gcc bugs that Clang does not follow, but that's in reference to a C DR.

ebevhan: That gcc godbolt is a bit odd. The type of the bitfield expression in the C++ example is `long`…
aaron.ballmanUnsubmitted
Not Done
Reply

C++ disallows the rank conversion from int to long as well. [conv.prom]p1 does not apply because long int has a higher rank than int, but [conv.prom]p5 allows the promotion if the range of values is identical between the two types.

C makes this UB in several ways -- you can't have a bit-field whose type is something other than int, unsigned int, or _Bool (6.7.2.1p5) or promoting from types other than those (6.3.1.1p2), but otherwise matches the C++ behavior in terms of promotion (including the rank conversion).

You may have to dig further into what Clang is doing, but I would guess that the diagnostics should be triggered in both C and C++ similarly.

Ultimately, I'd like to see tests for cases where sizeof(int) == sizeof(long), sizeof(int) != sizeof(long), and variants for C and C++ of each.

aaron.ballman: C++ disallows the rank conversion from int to long as well. [conv.prom]p1 does not apply…
ebevhanAuthorUnsubmitted
Not Done
Reply

I'm not sure the warning should trigger in C++; the behavior is correct there. The expression in those cases should be of type long, not int. The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int, but the rules in C say that the value _is_ promoted.

The strange promotion behavior only occurs in C because of the issue with bitfields larger than int. It's not really permitted according to the standard, but it's supported anyway to match C++. Though, it ends up not matching C++ due to these promotion differences.

I'll add tests for the different int and long sizes, though the only case where it would make a difference would be if int was larger than 32 bits, which it isn't on any target.

ebevhan: I'm not sure the warning should trigger in C++; the behavior is correct there. The expression…
aaron.ballmanUnsubmitted
Not Done
Reply

The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int, but the rules in C say that the value _is_ promoted.

Ahhh, that explains the differences (my eyes glossed over that in the standards text), thank you!

aaron.ballman: > The bitfield promotions in C++ say that values _can_ be promoted if the value fits in int…
3​
4​int printf(const char *restrict, ...);
5​
6​struct bitfields {
7​ long a : 2;
8​ unsigned long b : 2;
9​ long c : 32; // assumes that int is 32 bits
10​ unsigned long d : 32; // assumes that int is 32 bits
11​} bf;
12​
13​void bitfield_promotion() {
14​ printf("%ld", bf.a); // expected-warning {{format specifies type 'long' but the argument has type 'int'}}
15​ printf("%lu", bf.b); // expected-warning {{format specifies type 'unsigned long' but the argument has type 'int'}}
16​ printf("%ld", bf.c); // expected-warning {{format specifies type 'long' but the argument has type 'int'}}
17​ printf("%lu", bf.d); // expected-warning {{format specifies type 'unsigned long' but the argument has type 'unsigned int'}}
18​}