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
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@151943 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 086a8b5..cb59cba 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -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,
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index dc88815..1c749e3 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -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>;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 7d9f835..6289b9f 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -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,
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 1bab239..ab96f74 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -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;
diff --git a/test/Sema/builtins.c b/test/Sema/builtins.c
index 17888b0..b8b0367 100644
--- a/test/Sema/builtins.c
+++ b/test/Sema/builtins.c
@@ -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) {
diff --git a/test/SemaCXX/array-bounds-ptr-arith.cpp b/test/SemaCXX/array-bounds-ptr-arith.cpp
index ce1ace6..16e2567 100644
--- a/test/SemaCXX/array-bounds-ptr-arith.cpp
+++ b/test/SemaCXX/array-bounds-ptr-arith.cpp
@@ -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) {
diff --git a/test/SemaCXX/constant-expression-cxx11.cpp b/test/SemaCXX/constant-expression-cxx11.cpp
index dcc437a..66fd064 100644
--- a/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/test/SemaCXX/constant-expression-cxx11.cpp
@@ -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 {
diff --git a/test/SemaCXX/null_in_arithmetic_ops.cpp b/test/SemaCXX/null_in_arithmetic_ops.cpp
index 24590ce..a6c0dbf 100644
--- a/test/SemaCXX/null_in_arithmetic_ops.cpp
+++ b/test/SemaCXX/null_in_arithmetic_ops.cpp
@@ -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() {
diff --git a/test/SemaCXX/string-plus-int.cpp b/test/SemaCXX/string-plus-int.cpp
new file mode 100644
index 0000000..3be3f07
--- /dev/null
+++ b/test/SemaCXX/string-plus-int.cpp
@@ -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);
+}
+