Consider unsigned long for non-u/U decimal literals (C90/C++03)

[hubert.reinterpretcast]

Summary

This modifies Clang to reflect that under pre-C99 ISO C, decimal
constants may have type unsigned long even if they do not contain u
or U in their suffix (C90 subclause 6.1.3.2 paragraph 5). The same is
done for C++ without C++11 which--because of undefined behaviour--allows
for behaviour compatible with ISO C90 in the case of an unsuffixed
decimal literal and is otherwise identical to C90 in its treatment of
integer literals (C++03 subclause 2.13.1 [lex.icon] paragraph 2).

Messages are added to the c99-compat and c++11-compat groups to warn
on such literals, since they behave differently under the newer
standards.

Fixes PR 16678.

[hubert.reinterpretcast]

Ping.

[rsmith]

Sorry for the delay.

Can you combine the test files together into a single file with more RUN: lines (perhaps with -Ds for the things that change between the old and new modes, and -x to specify C versus C++ input)? Note that expected-* comments within #if'd out regions are ignored. That'll make it much more obvious what you're changing and what is expected to change between modes. (There are also some costs associated with the number of separate test files that can be reduced by doing this.)

include/clang/Basic/DiagnosticCommonKinds.td

119

In C++98, this should probably be an ExtWarn rather than a Warning, because this case has undefined behavior. Splitting this into two warnings would also allow you to put the warning into -Wc99-compat or -Wc++11-compat as appropriate, and to indicate that the C++98 case has undefined behavior.

120–121

It would be clearer and more useful to phrase this as:

"integer literal is too large to be represented in type 'long', interpreting as 'unsigned long'; this literal will have type '%select{long long|unsigned long long}0' in C99 onwards"

(with s/C99/C++11/ for the C++98 warning.)

You can easily compute the signedness of the long long in SemaExpr by checking if LongLongSize > LongSize.

[hubert.reinterpretcast]

include/clang/Basic/DiagnosticCommonKinds.td

119

In C++98, the undefined behaviour is only on the case of a decimal integer literal with none of u/U/l/L. If the l/L is present, the choice of unsigned long is required to be considered after long.

I agree that an ExtWarn is appropriate for the undefined behaviour case, but I do not think that the -Wc99-compat/-Wc++11-compat split still follows.

120–121

The use of unsigned long long in C99/C++11 modes would also be an extension though. Saying "C99 onwards" implies the Standard, not the mode; so perhaps "in C99-and-later modes"?

[rsmith]

include/clang/Basic/DiagnosticCommonKinds.td

119

OK, so:

• For the cases where the code has undefined behavior, the warning should be an ExtWarn
• For the cases where the code has defined behavior but changes meaning in a later standard, the warning should be in -Wc*-compat
• For the cases where the code becomes invalid in a later standard, the warning should say so
• We should split this warning up into enough separate diagnostics that we can cover all of the above cases with correct, clear, helpful diagnostics

I think the different cases are:

1. C89, no suffix or L, LONG_MAX == LLONG_MAX (c99-compat), "will be ill-formed in C99 onwards"
2. C89, no suffix or L, LONG_MAX < LLONG_MAX (c99-compat), "will have type 'long long' in C99 onwards"
3. C++98, no suffix, LONG_MAX == LLONG_MAX (c++11-compat, ExtWarn), "will be ill-formed in C++11 onwards"
4. C++98, L suffix, LONG_MAX == LLONG_MAX (c++11-compat), "will be ill-formed in C++11 onwards"
5. C++98, no suffix, LONG_MAX < LLONG_MAX (c++11-compat, ExtWarn), "will have type 'long long' in C++11 onwards"
6. C++98, L suffix, LONG_MAX < LLONG_MAX (c++11-compat), "will have type 'long long' in C++11 onwards"

I think you can cover the above with three different diagnostics, for 1+2, 3+5, and 4+6, with a %select in each one selecting on whether 'long long' is large enough for the literal.

Each of these should be in either -Wc99-compat or -Wc++11-compat because the meaning of the program would be different under -std=c99 or -std=c++11, respectively.

[hubert.reinterpretcast]

Split+regroup the new messages; merge testcases

[rsmith]

LGTM, thanks!

include/clang/Basic/DiagnosticCommonKinds.td

def warn_cxx98_compat_longlong : Warning<
   "'long long' is incompatible with C++98">,
   InGroup<CXX98CompatPedantic>, DefaultIgnore;
 def err_integer_literal_too_large : Error<
   "integer literal is too large to be represented in any %select{signed |}0"
   "integer type">;
 def ext_integer_literal_too_large_for_signed : ExtWarn<
   "integer literal is too large to be represented in a signed integer type, "
   "interpreting as unsigned">,
-  InGroup<DiagGroup<"implicitly-unsigned-literal">>;
+  InGroup<ImplicitlyUnsignedLiteral>;
+def warn_old_implicitly_unsigned_long : Warning<
+  "integer literal is too large to be represented in type 'long', "
+  "interpreting as 'unsigned long' per C89; this literal will "
+  "%select{have type 'long long'|be ill-formed}0 in C99 onwards">,
+  InGroup<C99Compat>;
+def warn_old_implicitly_unsigned_long_cxx : Warning<
+  "integer literal is too large to be represented in type 'long', "
+  "interpreting as 'unsigned long' per C++98; this literal will "
+  "%select{have type 'long long'|be ill-formed}0 in C++11 onwards">,
+  InGroup<CXX11Compat>;
+def ext_old_implicitly_unsigned_long_cxx : ExtWarn<
+  "integer literal is too large to be represented in type 'long' and is "
+  "subject to undefined behavior under C++98, interpreting as 'unsigned long'; "
+  "this literal will %select{have type 'long long'|be ill-formed}0 "
+  "in C++11 onwards">,
+  InGroup<CXX11Compat>;
 
 // SEH
 def err_seh_expected_handler : Error<
   "expected '__except' or '__finally' block">;
 def err_seh___except_block : Error<
   "%0 only allowed in __except block or filter expression">;
 def err_seh___except_filter : Error<
   "%0 only allowed in __except filter expression">;

include/clang/Basic/DiagnosticGroups.td

 def MissingDeclarations: DiagGroup<"missing-declarations">;
 def : DiagGroup<"missing-format-attribute">;
 def : DiagGroup<"missing-include-dirs">;
 def MissingNoreturn : DiagGroup<"missing-noreturn">;
 def MultiChar : DiagGroup<"multichar">;
 def : DiagGroup<"nested-externs">;
 def CXX11LongLong : DiagGroup<"c++11-long-long">;
 def LongLong : DiagGroup<"long-long", [CXX11LongLong]>;
+def ImplicitlyUnsignedLiteral : DiagGroup<"implicitly-unsigned-literal">;
 def MethodSignatures : DiagGroup<"method-signatures">;
 def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">;
 def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">;
 def MismatchedTags : DiagGroup<"mismatched-tags">;
 def MissingFieldInitializers : DiagGroup<"missing-field-initializers">;
 def ModuleBuild : DiagGroup<"module-build">;
 def ModuleConflict : DiagGroup<"module-conflict">;
 def NewlineEOF : DiagGroup<"newline-eof">;

lib/Sema/SemaExpr.cpp

if (Literal.GetIntegerValue(ResultVal)) {
 
         // Does it fit in a unsigned long?
         if (ResultVal.isIntN(LongSize)) {
           // Does it fit in a signed long?
           if (!Literal.isUnsigned && ResultVal[LongSize-1] == 0)
             Ty = Context.LongTy;
           else if (AllowUnsigned)
             Ty = Context.UnsignedLongTy;
+          // Check according to the rules of C90 6.1.3.2p5. C++03 [lex.icon]p2
+          // is compatible.
+          else if (!getLangOpts().C99 && !getLangOpts().CPlusPlus11) {
+            const unsigned LongLongSize =
+                Context.getTargetInfo().getLongLongWidth();
+            Diag(Tok.getLocation(),
+                 getLangOpts().CPlusPlus
+                     ? Literal.isLong
+                           ? diag::warn_old_implicitly_unsigned_long_cxx
+                           : /*C++98 UB*/ diag::
+                                 ext_old_implicitly_unsigned_long_cxx
+                     : diag::warn_old_implicitly_unsigned_long)
+                << (LongLongSize > LongSize ? /*will have type 'long long'*/ 0
+                                            : /*will be ill-formed*/ 1);
+            Ty = Context.UnsignedLongTy;
+          }
           Width = LongSize;
         }
       }
 
       // Check long long if needed.
       if (Ty.isNull()) {
         unsigned LongLongSize = Context.getTargetInfo().getLongLongWidth();
 

test/Sema/PR16678.c

+// RUN: %clang_cc1 -DX32TYPE=ULONG -triple powerpc-unknown-linux-gnu -std=c89 -x c %s -verify
+// RUN: %clang_cc1 -DX32TYPE=ULONG -triple powerpc-unknown-linux-gnu -std=iso9899:199409 -x c %s -verify
+// RUN: %clang_cc1 -DX32TYPE=ULONG -triple powerpc-unknown-linux-gnu -std=c++98 -x c++ %s -verify
+// RUN: %clang_cc1 -DX32TYPE=LLONG -triple powerpc-unknown-linux-gnu -std=c99 -x c %s -verify
+// RUN: %clang_cc1 -DX32TYPE=LLONG -triple powerpc-unknown-linux-gnu -std=c11 -x c %s -verify
+// RUN: %clang_cc1 -DX32TYPE=LLONG -triple powerpc-unknown-linux-gnu -std=c++11 -x c++ %s -verify
+// RUN: %clang_cc1 -DX32TYPE=LLONG -triple powerpc-unknown-linux-gnu -std=c++1y -x c++ %s -verify
+// RUN: %clang_cc1 -DX32TYPE=LLONG -triple powerpc-unknown-linux-gnu -std=c++1z -x c++ %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULONG -triple powerpc64-unknown-linux-gnu -std=c89 -x c %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULONG -triple powerpc64-unknown-linux-gnu -std=iso9899:199409 -x c %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULONG -triple powerpc64-unknown-linux-gnu -std=c++98 -x c++ %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULLONG -triple powerpc64-unknown-linux-gnu -std=c99 -x c %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULLONG -triple powerpc64-unknown-linux-gnu -std=c11 -x c %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULLONG -triple powerpc64-unknown-linux-gnu -std=c++11 -x c++ %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULLONG -triple powerpc64-unknown-linux-gnu -std=c++1y -x c++ %s -verify
+// RUN: %clang_cc1 -DX64TYPE=ULLONG -triple powerpc64-unknown-linux-gnu -std=c++1z -x c++ %s -verify
+
+#ifdef X64TYPE
+#define X32TYPE long
+#endif
+
+#define IS_ULONG_ULONG 1
+#define IS_ULONG2(X) IS_ULONG_##X
+#define IS_ULONG(X) IS_ULONG2(X)
+
+#if !defined(X64TYPE) && !IS_ULONG(X32TYPE)
+// expected-no-diagnostics
+#endif
+
+typedef unsigned long ULONG;
+typedef long long LLONG;
+typedef unsigned long long ULLONG;
+
+
+/******************************************************************************
+ * Test 2^31 as a decimal literal with no suffix and with the "l" and "L" cases.
+ ******************************************************************************/
+extern X32TYPE x32;
+extern __typeof__(2147483648) x32;
+extern __typeof__(2147483648l) x32;
+extern __typeof__(2147483648L) x32;
+
+#if IS_ULONG(X32TYPE)
+#if !__cplusplus
+
+/******************************************************************************
+ * Under pre-C99 ISO C, unsigned long is attempted for decimal integer literals
+ * that do not have a suffix containing "u" or "U" if the literal does not fit
+ * within the range of int or long. See 6.1.3.2 paragraph 5.
+ ******************************************************************************/
+// expected-warning@39 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C89; this literal will have type 'long long' in C99 onwards}}
+// expected-warning@40 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C89; this literal will have type 'long long' in C99 onwards}}
+// expected-warning@41 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C89; this literal will have type 'long long' in C99 onwards}}
+#else
+
+/******************************************************************************
+ * Under pre-C++11 ISO C++, the same holds if the literal contains an "l" or "L"
+ * in its suffix; otherwise, the behavior is undefined. See 2.13.1 [lex.icon]
+ * paragraph 2.
+ ******************************************************************************/
+// expected-warning@39 {{integer literal is too large to be represented in type 'long' and is subject to undefined behavior under C++98, interpreting as 'unsigned long'; this literal will have type 'long long' in C++11 onwards}}
+// expected-warning@40 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C++98; this literal will have type 'long long' in C++11 onwards}}
+// expected-warning@41 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C++98; this literal will have type 'long long' in C++11 onwards}}
+#endif
+#endif
+
+
+#ifdef X64TYPE
+
+/******************************************************************************
+ * Test 2^63 as a decimal literal with no suffix and with the "l" and "L" cases.
+ ******************************************************************************/
+extern X64TYPE x64;
+extern __typeof__(9223372036854775808) x64;
+extern __typeof__(9223372036854775808l) x64;
+extern __typeof__(9223372036854775808L) x64;
+
+#if IS_ULONG(X64TYPE)
+
+#if !__cplusplus
+
+/******************************************************************************
+ * Under pre-C99 ISO C, unsigned long is attempted for decimal integer literals
+ * that do not have a suffix containing "u" or "U" if the literal does not fit
+ * within the range of int or long. See 6.1.3.2 paragraph 5.
+ ******************************************************************************/
+// expected-warning@74 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C89; this literal will be ill-formed in C99 onwards}}
+// expected-warning@75 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C89; this literal will be ill-formed in C99 onwards}}
+// expected-warning@76 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C89; this literal will be ill-formed in C99 onwards}}
+#else
+
+/******************************************************************************
+ * Under pre-C++11 ISO C++, the same holds if the literal contains an "l" or "L"
+ * in its suffix; otherwise, the behavior is undefined. See 2.13.1 [lex.icon]
+ * paragraph 2.
+ ******************************************************************************/
+// expected-warning@74 {{integer literal is too large to be represented in type 'long' and is subject to undefined behavior under C++98, interpreting as 'unsigned long'; this literal will be ill-formed in C++11 onwards}}
+// expected-warning@75 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C++98; this literal will be ill-formed in C++11 onwards}}
+// expected-warning@76 {{integer literal is too large to be represented in type 'long', interpreting as 'unsigned long' per C++98; this literal will be ill-formed in C++11 onwards}}
+#endif
+#else
+
+/******************************************************************************
+ * The status quo in C99/C++11-and-later modes for the literals in question is
+ * to interpret them as unsigned as an extension.
+ ******************************************************************************/
+// expected-warning@74 {{integer literal is too large to be represented in a signed integer type, interpreting as unsigned}}
+// expected-warning@75 {{integer literal is too large to be represented in a signed integer type, interpreting as unsigned}}
+// expected-warning@76 {{integer literal is too large to be represented in a signed integer type, interpreting as unsigned}}
+#endif
+#endif
+
+
+/******************************************************************************
+ * Test preprocessor arithmetic with 2^31 as a decimal literal with no suffix
+ * and with the "l" and "L" cases.
+ ******************************************************************************/
+#if !IS_ULONG(X32TYPE)
+
+/******************************************************************************
+ * If the literal is signed without need for the modified range of the signed
+ * integer types within the controlling constant expression for conditional
+ * inclusion, then it will also be signed with said modified range.
+ ******************************************************************************/
+#define EXPR(X) ((X - X) - 1 < 0)
+#else
+
+/******************************************************************************
+ * Strictly speaking, in pre-C99/C++11 ISO C/C++, the preprocessor arithmetic is
+ * evaluated with the range of long/unsigned long; however, both Clang and GCC
+ * evaluate using 64-bits even when long/unsigned long are 32-bits outside of
+ * preprocessing.
+ *
+ * If the range used becomes 32-bits, then this test will enforce the treatment
+ * as unsigned of the literals in question.
+ *
+ * Note:
+ * Under pre-C99/C++11 ISO C/C++, whether the interpretation of the literal is
+ * affected by the modified range of the signed and unsigned integer types
+ * within the controlling constant expression for conditional inclusion is
+ * unclear.
+ ******************************************************************************/
+#define PP_LONG_MAX ((0ul - 1ul) >> 1)
+#define EXPR(X)                                                                \
+  (PP_LONG_MAX >= 0x80000000 || (X - X) - 1 > 0) // either 2^31 fits into a
+                                                 // preprocessor "long" or the
+                                                 // literals in question are
+                                                 // unsigned
+#endif
+
+#if !(EXPR(2147483648) && EXPR(2147483648l) && EXPR(2147483648L))
+#error Unexpected signedness or conversion behavior
+#endif