Index: test/Sema/builtins.c =================================================================== --- test/Sema/builtins.c (revision 151602) +++ test/Sema/builtins.c (working copy) @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -triple=i686-apple-darwin9 +// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wno-string-plus-int -triple=i686-apple-darwin9 // This test needs to set the target because it uses __builtin_ia32_vec_ext_v4si int test1(float a, int b) { Index: test/SemaCXX/constant-expression-cxx11.cpp =================================================================== --- test/SemaCXX/constant-expression-cxx11.cpp (revision 151602) +++ test/SemaCXX/constant-expression-cxx11.cpp (working copy) @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple i686-linux -fsyntax-only -verify -std=c++11 -pedantic %s -Wno-comment +// RUN: %clang_cc1 -triple i686-linux -Wno-string-plus-int -fsyntax-only -verify -std=c++11 -pedantic %s -Wno-comment namespace StaticAssertFoldTest { Index: test/SemaCXX/null_in_arithmetic_ops.cpp =================================================================== --- test/SemaCXX/null_in_arithmetic_ops.cpp (revision 151602) +++ test/SemaCXX/null_in_arithmetic_ops.cpp (working copy) @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -fblocks -Wnull-arithmetic -verify %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -fblocks -Wnull-arithmetic -verify -Wno-string-plus-int %s #include void f() { Index: test/SemaCXX/array-bounds-ptr-arith.cpp =================================================================== --- test/SemaCXX/array-bounds-ptr-arith.cpp (revision 151602) +++ test/SemaCXX/array-bounds-ptr-arith.cpp (working copy) @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -verify -Warray-bounds-pointer-arithmetic %s +// RUN: %clang_cc1 -verify -Wno-string-plus-int -Warray-bounds-pointer-arithmetic %s void swallow (const char *x) { (void)x; } void test_pointer_arithmetic(int n) { Index: test/SemaCXX/string-plus-int.cpp =================================================================== --- test/SemaCXX/string-plus-int.cpp (revision 0) +++ test/SemaCXX/string-plus-int.cpp (revision 0) @@ -0,0 +1,62 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-array-bounds %s -fpascal-strings + +void consume(const char* c) {} +void consume(const unsigned char* c) {} +void consume(const wchar_t* c) {} +void consumeChar(char c) {} + +enum MyEnum { + kMySmallEnum = 1, + kMyEnum = 5 +}; + +enum OperatorOverloadEnum { + kMyOperatorOverloadedEnum = 5 +}; + +const char* operator+(const char* c, OperatorOverloadEnum e) { + return "yo"; +} + +const char* operator+(OperatorOverloadEnum e, const char* c) { + return "yo"; +} + +void f(int index) { + // Should warn. + consume("foo" + 5); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + consume("foo" + index); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + consume("foo" + kMyEnum); // expected-warning {{adding 'MyEnum' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + + consume(5 + "foo"); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + consume(index + "foo"); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + consume(kMyEnum + "foo"); // expected-warning {{adding 'MyEnum' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + + // FIXME: suggest replacing with "foo"[5] + consumeChar(*("foo" + 5)); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + consumeChar(*(5 + "foo")); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + + consume(L"foo" + 5); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}} + + // Should not warn. + consume(&("foo"[3])); + consume(&("foo"[index])); + consume(&("foo"[kMyEnum])); + consume("foo" + kMySmallEnum); + consume(kMySmallEnum + "foo"); + + consume(L"foo" + 2); + + consume("foo" + 3); // Points at the \0 + consume("foo" + 4); // Points 1 past the \0, which is legal too. + consume("\pfoo" + 4); // Pascal strings don't have a trailing \0, but they + // have a leading length byte, so this is fine too. + + consume("foo" + kMyOperatorOverloadedEnum); + consume(kMyOperatorOverloadedEnum + "foo"); + + #define A "foo" + #define B "bar" + consume(A B + sizeof(A) - 1); +} + Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td (revision 151602) +++ include/clang/Basic/DiagnosticGroups.td (working copy) @@ -160,6 +160,7 @@ def : DiagGroup<"switch-default">; def : DiagGroup<"synth">; def SizeofArrayArgument : DiagGroup<"sizeof-array-argument">; +def StringPlusInt : DiagGroup<"string-plus-int">; def StrncatSize : DiagGroup<"strncat-size">; def TautologicalCompare : DiagGroup<"tautological-compare">; def HeaderHygiene : DiagGroup<"header-hygiene">; @@ -323,6 +324,7 @@ ReturnType, SelfAssignment, SizeofArrayArgument, + StringPlusInt, Trigraphs, Uninitialized, UnknownPragmas, Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td (revision 151602) +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy) @@ -3426,6 +3426,12 @@ "explicitly assigning a variable of type %0 to itself">, InGroup, DefaultIgnore; +def warn_string_plus_int : Warning< + "adding %0 to a string does not append to the string">, + InGroup; +def note_string_plus_int_silence : Note< + "use array indexing to silence this warning">; + def warn_sizeof_array_param : Warning< "sizeof on array function parameter will return size of %0 instead of %1">, InGroup; Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h (revision 151602) +++ include/clang/Sema/Sema.h (working copy) @@ -6074,7 +6074,7 @@ ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, bool IsCompAssign = false); QualType CheckAdditionOperands( // C99 6.5.6 - ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, + ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, unsigned Opc, QualType* CompLHSTy = 0); QualType CheckSubtractionOperands( // C99 6.5.6 ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp (revision 151602) +++ lib/Sema/SemaExpr.cpp (working copy) @@ -5943,6 +5943,46 @@ return false; } +/// diagnoseStringPlusInt - Emit a warning when adding an integer to a string +/// literal. +static void diagnoseStringPlusInt(Sema &Self, SourceLocation OpLoc, + Expr *LHSExpr, Expr *RHSExpr) { + StringLiteral* StrExpr = dyn_cast(LHSExpr->IgnoreImpCasts()); + Expr* IndexExpr = RHSExpr; + if (!StrExpr) { + StrExpr = dyn_cast(RHSExpr->IgnoreImpCasts()); + IndexExpr = LHSExpr; + } + + bool IsStringPlusInt = StrExpr && + IndexExpr->getType()->isIntegralOrUnscopedEnumerationType(); + if (!IsStringPlusInt) + return; + + llvm::APSInt index; + if (IndexExpr->EvaluateAsInt(index, Self.getASTContext())) { + unsigned StrLenWithNull = StrExpr->getLength() + 1; + if (index.isNonNegative() && + index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull), + index.isUnsigned())) + return; + } + + SourceRange DiagRange(LHSExpr->getLocStart(), RHSExpr->getLocEnd()); + Self.Diag(OpLoc, diag::warn_string_plus_int) + << DiagRange << IndexExpr->IgnoreImpCasts()->getType(); + + // Only print a fixit for "str" + int, not for int + "str". + if (IndexExpr == RHSExpr) { + SourceLocation EndLoc = Self.PP.getLocForEndOfToken(RHSExpr->getLocEnd()); + Self.Diag(OpLoc, diag::note_string_plus_int_silence) + << FixItHint::CreateInsertion(LHSExpr->getLocStart(), "&") + << FixItHint::CreateReplacement(SourceRange(OpLoc), "[") + << FixItHint::CreateInsertion(EndLoc, "]"); + } else + Self.Diag(OpLoc, diag::note_string_plus_int_silence); +} + /// \brief Emit error when two pointers are incompatible. static void diagnosePointerIncompatibility(Sema &S, SourceLocation Loc, Expr *LHSExpr, Expr *RHSExpr) { @@ -5954,7 +5994,8 @@ } QualType Sema::CheckAdditionOperands( // C99 6.5.6 - ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, QualType* CompLHSTy) { + ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, unsigned Opc, + QualType* CompLHSTy) { checkArithmeticNull(*this, LHS, RHS, Loc, /*isCompare=*/false); if (LHS.get()->getType()->isVectorType() || @@ -5968,6 +6009,10 @@ if (LHS.isInvalid() || RHS.isInvalid()) return QualType(); + // Diagnose "string literal" '+' int. + if (Opc == BO_Add) + diagnoseStringPlusInt(*this, Loc, LHS.get(), RHS.get()); + // handle the common case first (both operands are arithmetic). if (LHS.get()->getType()->isArithmeticType() && RHS.get()->getType()->isArithmeticType()) { @@ -7664,7 +7709,7 @@ ResultTy = CheckRemainderOperands(LHS, RHS, OpLoc); break; case BO_Add: - ResultTy = CheckAdditionOperands(LHS, RHS, OpLoc); + ResultTy = CheckAdditionOperands(LHS, RHS, OpLoc, Opc); break; case BO_Sub: ResultTy = CheckSubtractionOperands(LHS, RHS, OpLoc); @@ -7707,7 +7752,7 @@ ResultTy = CheckAssignmentOperands(LHS.get(), RHS, OpLoc, CompResultTy); break; case BO_AddAssign: - CompResultTy = CheckAdditionOperands(LHS, RHS, OpLoc, &CompLHSTy); + CompResultTy = CheckAdditionOperands(LHS, RHS, OpLoc, Opc, &CompLHSTy); if (!CompResultTy.isNull() && !LHS.isInvalid() && !RHS.isInvalid()) ResultTy = CheckAssignmentOperands(LHS.get(), RHS, OpLoc, CompResultTy); break;