davide created this revision. davide added a reviewer: rsmith. davide added a subscriber: cfe-commits.
While I think there's no need to not mirror gcc warning, probably the message can be reworded in a slightly different way. I tried, but suggestions are more than welcome. Fixes PR24026 % ./clang -Wshift-negative-value emit.c emit.c:3:14: warning: shifting a negative signed value is undefined [-Wshift-negative-value] int a = -1 << 3; ~~ ^ 1 warning generated. Repository: rL LLVM http://reviews.llvm.org/D10938 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/CXX/expr/expr.const/p2-0x.cpp test/Sema/shift.c Index: test/Sema/shift.c =================================================================== --- test/Sema/shift.c +++ test/Sema/shift.c @@ -39,16 +39,16 @@ i = 1 << (WORD_BIT - 2); i = 2 << (WORD_BIT - 1); // expected-warning {{bits to represent, but 'int' only has}} i = 1 << (WORD_BIT - 1); // expected-warning {{sets the sign bit of the shift expression}} - i = -1 << (WORD_BIT - 1); + i = -1 << (WORD_BIT - 1); // expected-warning {{shifting a negative signed value is undefined}} i = 0 << (WORD_BIT - 1); i = (char)1 << (WORD_BIT - 2); unsigned u; u = 1U << (WORD_BIT - 1); u = 5U << (WORD_BIT - 1); long long int lli; - lli = INT_MIN << 2; // expected-warning {{bits to represent, but 'int' only has}} + lli = INT_MIN << 2; // expected-warning {{shifting a negative signed value is undefined}} lli = 1LL << (sizeof(long long) * CHAR_BIT - 2); } Index: test/CXX/expr/expr.const/p2-0x.cpp =================================================================== --- test/CXX/expr/expr.const/p2-0x.cpp +++ test/CXX/expr/expr.const/p2-0x.cpp @@ -157,7 +157,7 @@ constexpr int shl_unsigned_negative = unsigned(-3) << 1; // ok constexpr int shl_unsigned_into_sign = 1u << 31; // ok constexpr int shl_unsigned_overflow = 1024u << 31; // ok - constexpr int shl_signed_negative = (-3) << 1; // expected-error {{constant expression}} expected-note {{left shift of negative value -3}} + constexpr int shl_signed_negative = (-3) << 1; // expected-warning {{shifting a negative signed value is undefined}} // expected-error {{constant expression}} expected-note {{left shift of negative value -3}} constexpr int shl_signed_ok = 1 << 30; // ok constexpr int shl_signed_into_sign = 1 << 31; // ok (DR1457) constexpr int shl_signed_into_sign_2 = 0x7fffffff << 1; // ok (DR1457) Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -7937,9 +7937,19 @@ // representable in the result type, so never warn for those. llvm::APSInt Left; if (LHS.get()->isValueDependent() || - !LHS.get()->isIntegerConstantExpr(Left, S.Context) || + !LHS.get()->EvaluateAsInt(Left, S.Context) || LHSType->hasUnsignedIntegerRepresentation()) return; + + // If LHS does not have a signed type and non-negative value + // then, the behavior is undefined. Warn about it. + if (Left.isNegative()) { + S.DiagRuntimeBehavior(Loc, LHS.get(), + S.PDiag(diag::warn_shift_lhs_negative) + << LHS.get()->getSourceRange()); + return; + } + llvm::APInt ResultBits = static_cast<llvm::APInt&>(Right) + Left.getMinSignedBits(); if (LeftBits.uge(ResultBits)) Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -4740,6 +4740,8 @@ InGroup<DivZero>; def warn_remainder_by_zero : Warning<"remainder by zero is undefined">, InGroup<DivZero>; +def warn_shift_lhs_negative : Warning<"shifting a negative signed value is undefined">, + InGroup<DiagGroup<"shift-negative-value">>; def warn_shift_negative : Warning<"shift count is negative">, InGroup<DiagGroup<"shift-count-negative">>; def warn_shift_gt_typewidth : Warning<"shift count >= width of type">,
Index: test/Sema/shift.c =================================================================== --- test/Sema/shift.c +++ test/Sema/shift.c @@ -39,16 +39,16 @@ i = 1 << (WORD_BIT - 2); i = 2 << (WORD_BIT - 1); // expected-warning {{bits to represent, but 'int' only has}} i = 1 << (WORD_BIT - 1); // expected-warning {{sets the sign bit of the shift expression}} - i = -1 << (WORD_BIT - 1); + i = -1 << (WORD_BIT - 1); // expected-warning {{shifting a negative signed value is undefined}} i = 0 << (WORD_BIT - 1); i = (char)1 << (WORD_BIT - 2); unsigned u; u = 1U << (WORD_BIT - 1); u = 5U << (WORD_BIT - 1); long long int lli; - lli = INT_MIN << 2; // expected-warning {{bits to represent, but 'int' only has}} + lli = INT_MIN << 2; // expected-warning {{shifting a negative signed value is undefined}} lli = 1LL << (sizeof(long long) * CHAR_BIT - 2); } Index: test/CXX/expr/expr.const/p2-0x.cpp =================================================================== --- test/CXX/expr/expr.const/p2-0x.cpp +++ test/CXX/expr/expr.const/p2-0x.cpp @@ -157,7 +157,7 @@ constexpr int shl_unsigned_negative = unsigned(-3) << 1; // ok constexpr int shl_unsigned_into_sign = 1u << 31; // ok constexpr int shl_unsigned_overflow = 1024u << 31; // ok - constexpr int shl_signed_negative = (-3) << 1; // expected-error {{constant expression}} expected-note {{left shift of negative value -3}} + constexpr int shl_signed_negative = (-3) << 1; // expected-warning {{shifting a negative signed value is undefined}} // expected-error {{constant expression}} expected-note {{left shift of negative value -3}} constexpr int shl_signed_ok = 1 << 30; // ok constexpr int shl_signed_into_sign = 1 << 31; // ok (DR1457) constexpr int shl_signed_into_sign_2 = 0x7fffffff << 1; // ok (DR1457) Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -7937,9 +7937,19 @@ // representable in the result type, so never warn for those. llvm::APSInt Left; if (LHS.get()->isValueDependent() || - !LHS.get()->isIntegerConstantExpr(Left, S.Context) || + !LHS.get()->EvaluateAsInt(Left, S.Context) || LHSType->hasUnsignedIntegerRepresentation()) return; + + // If LHS does not have a signed type and non-negative value + // then, the behavior is undefined. Warn about it. + if (Left.isNegative()) { + S.DiagRuntimeBehavior(Loc, LHS.get(), + S.PDiag(diag::warn_shift_lhs_negative) + << LHS.get()->getSourceRange()); + return; + } + llvm::APInt ResultBits = static_cast<llvm::APInt&>(Right) + Left.getMinSignedBits(); if (LeftBits.uge(ResultBits)) Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -4740,6 +4740,8 @@ InGroup<DivZero>; def warn_remainder_by_zero : Warning<"remainder by zero is undefined">, InGroup<DivZero>; +def warn_shift_lhs_negative : Warning<"shifting a negative signed value is undefined">, + InGroup<DiagGroup<"shift-negative-value">>; def warn_shift_negative : Warning<"shift count is negative">, InGroup<DiagGroup<"shift-count-negative">>; def warn_shift_gt_typewidth : Warning<"shift count >= width of type">,
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits