Author: nico Date: Fri Mar 2 16:01:22 2012 New Revision: 151943 URL: http://llvm.org/viewvc/llvm-project?rev=151943&view=rev Log: Add -Wstring-plus-int, which warns on "str" + int and int + "str".
It doesn't warn if the integer is known at compile time and within the bounds of the string. Discussion: http://comments.gmane.org/gmane.comp.compilers.clang.scm/47203 Added: cfe/trunk/test/SemaCXX/string-plus-int.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Sema/builtins.c cfe/trunk/test/SemaCXX/array-bounds-ptr-arith.cpp cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp cfe/trunk/test/SemaCXX/null_in_arithmetic_ops.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Mar 2 16:01:22 2012 @@ -161,6 +161,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">; @@ -326,6 +327,7 @@ ReturnType, SelfAssignment, SizeofArrayArgument, + StringPlusInt, Trigraphs, Uninitialized, UnknownPragmas, Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Mar 2 16:01:22 2012 @@ -3430,6 +3430,12 @@ "explicitly assigning a variable of type %0 to itself">, InGroup<SelfAssignment>, DefaultIgnore; +def warn_string_plus_int : Warning< + "adding %0 to a string does not append to the string">, + InGroup<StringPlusInt>; +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<SizeofArrayArgument>; Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Fri Mar 2 16:01:22 2012 @@ -6097,7 +6097,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, Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Mar 2 16:01:22 2012 @@ -5948,6 +5948,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<StringLiteral>(LHSExpr->IgnoreImpCasts()); + Expr* IndexExpr = RHSExpr; + if (!StrExpr) { + StrExpr = dyn_cast<StringLiteral>(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) { @@ -5959,7 +5999,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() || @@ -5973,6 +6014,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()) { @@ -7669,7 +7714,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); @@ -7712,7 +7757,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; Modified: cfe/trunk/test/Sema/builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/test/Sema/builtins.c (original) +++ cfe/trunk/test/Sema/builtins.c Fri Mar 2 16:01:22 2012 @@ -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) { Modified: cfe/trunk/test/SemaCXX/array-bounds-ptr-arith.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/array-bounds-ptr-arith.cpp?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/array-bounds-ptr-arith.cpp (original) +++ cfe/trunk/test/SemaCXX/array-bounds-ptr-arith.cpp Fri Mar 2 16:01:22 2012 @@ -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) { Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original) +++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Fri Mar 2 16:01:22 2012 @@ -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 { Modified: cfe/trunk/test/SemaCXX/null_in_arithmetic_ops.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/null_in_arithmetic_ops.cpp?rev=151943&r1=151942&r2=151943&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/null_in_arithmetic_ops.cpp (original) +++ cfe/trunk/test/SemaCXX/null_in_arithmetic_ops.cpp Fri Mar 2 16:01:22 2012 @@ -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 <stddef.h> void f() { Added: cfe/trunk/test/SemaCXX/string-plus-int.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/string-plus-int.cpp?rev=151943&view=auto ============================================================================== --- cfe/trunk/test/SemaCXX/string-plus-int.cpp (added) +++ cfe/trunk/test/SemaCXX/string-plus-int.cpp Fri Mar 2 16:01:22 2012 @@ -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); +} + _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
