[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 180075.
ArnaudBienner added a comment.
Herald added a reviewer: serge-sans-paille.

Make -Wstring-plus-int warns even if when the result is not out of bounds


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382

Files:
  bindings/python/tests/cindex/test_diagnostics.py
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/string-plus-int.cpp


Index: test/SemaCXX/string-plus-int.cpp
===================================================================
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   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("foo" + kMySmallEnum); // 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}}
+  consume(kMySmallEnum + "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}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{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);
 }
 
 template <typename T>
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9143,16 +9143,6 @@
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
     return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-    llvm::APSInt index = Result.Val.getInt();
-    unsigned StrLenWithNull = StrExpr->getLength() + 1;
-    if (index.isNonNegative() &&
-        index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-                              index.isUnsigned()))
-      return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
       << DiagRange << IndexExpr->IgnoreImpCasts()->getType();
Index: bindings/python/tests/cindex/test_diagnostics.py
===================================================================
--- bindings/python/tests/cindex/test_diagnostics.py
+++ bindings/python/tests/cindex/test_diagnostics.py
@@ -51,7 +51,7 @@
         self.assertEqual(tu.diagnostics[0].fixits[0].value, '.f0 = ')
 
     def test_diagnostic_range(self):
-        tu = get_tu('void f() { int i = "a" + 1; }')
+        tu = get_tu('void f() { int i = "a"; }')
         self.assertEqual(len(tu.diagnostics), 1)
         self.assertEqual(tu.diagnostics[0].severity, Diagnostic.Warning)
         self.assertEqual(tu.diagnostics[0].location.line, 1)
@@ -63,7 +63,7 @@
         self.assertEqual(tu.diagnostics[0].ranges[0].start.line, 1)
         self.assertEqual(tu.diagnostics[0].ranges[0].start.column, 20)
         self.assertEqual(tu.diagnostics[0].ranges[0].end.line, 1)
-        self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 27)
+        self.assertEqual(tu.diagnostics[0].ranges[0].end.column, 23)
         with self.assertRaises(IndexError):
             tu.diagnostics[0].ranges[1].start.line
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55382.180075.patch
Type: text/x-patch
Size: 5585 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190103/6230f042/attachment-0001.bin>