[PATCH] C++11: Reject string literal to non-const char * conversion

[Ismail Pazarbasi]

  We decided to limit this patch to overload resolution, addressing only PR16314.

  When comparing candidate functions, this conversion is considered worse than other conversions to prevent the candidate function to be the best. If this conversion happens in the best viable function, we will accept it with a warning (current behavior).

Hi rsmith,

http://llvm-reviews.chandlerc.com/D1965

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D1965?vs=5010&id=5328#toc

Files:
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/overload-0x.cpp
  unittests/ASTMatchers/ASTMatchersTest.cpp

Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -1172,6 +1172,18 @@
   return ICS;
 }
 
+/// Checks whether conversion sequence is string literal to (non-const) char *
+/// in C++11 mode. -fwritable-strings still enables the conversion.
+static inline bool
+isIllFormedStringLiteralConversion(Sema &S,
+                                   const ImplicitConversionSequence &ICS) {
+  return (ICS.isStandard() && ICS.Standard.First == ICK_Array_To_Pointer &&
+          ICS.Standard.Second == ICK_Identity &&
+          ICS.Standard.Third == ICK_Qualification &&
+          ICS.Standard.DeprecatedStringLiteralToCharPtr &&
+          !S.getLangOpts().WritableStrings && S.getLangOpts().CPlusPlus11);
+}
+
 /// TryImplicitConversion - Attempt to perform an implicit conversion
 /// from the given expression (Expr) to the given type (ToType). This
 /// function returns an implicit conversion sequence that can be used
@@ -1513,7 +1525,7 @@
     FromType = S.Context.getArrayDecayedType(FromType);
 
     if (S.IsStringLiteralToNonConstPointerConversion(From, ToType)) {
-      // This conversion is deprecated. (C++ D.4).
+      // This conversion is deprecated in C++03 (D.4)
       SCS.DeprecatedStringLiteralToCharPtr = true;
 
       // For the purpose of ranking in overload resolution
@@ -3259,7 +3271,7 @@
 
   return ImplicitConversionSequence::Indistinguishable;
 }
-  
+
 /// CompareImplicitConversionSequences - Compare two implicit
 /// conversion sequences to determine whether one is better than the
 /// other or if they are indistinguishable (C++ 13.3.3.2).
@@ -3283,9 +3295,14 @@
   //   treated as a user-defined sequence that is indistinguishable
   //   from any other user-defined conversion sequence.
   if (ICS1.getKindRank() < ICS2.getKindRank())
-    return ImplicitConversionSequence::Better;
+    return isIllFormedStringLiteralConversion(S, ICS1)
+               ? ImplicitConversionSequence::Worse
+               : ImplicitConversionSequence::Better;
+
   if (ICS2.getKindRank() < ICS1.getKindRank())
-    return ImplicitConversionSequence::Worse;
+    return isIllFormedStringLiteralConversion(S, ICS2)
+               ? ImplicitConversionSequence::Better
+               : ImplicitConversionSequence::Worse;
 
   // The following checks require both conversion sequences to be of
   // the same kind.
Index: test/SemaCXX/overload-0x.cpp
===================================================================
--- test/SemaCXX/overload-0x.cpp
+++ test/SemaCXX/overload-0x.cpp
@@ -9,3 +9,22 @@
     a = "help"; // expected-error {{no viable overloaded '='}}
   }
 }
+
+namespace PR16314 {
+  void f(char*);
+  int &f(...);
+  void x()
+  {
+    int &r = f("foo");
+  }
+}
+
+namespace warn_if_best {
+  void f(char *);
+  void f(double);
+  void x()
+  {
+    f("foo");  // expected-warning {{conversion from string literal to 'char *' is deprecated}}
+  }
+}
+
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -2583,13 +2583,13 @@
 }
 
 TEST(FunctionalCast, MatchesSimpleCase) {
-  std::string foo_class = "class Foo { public: Foo(char*); };";
+  std::string foo_class = "class Foo { public: Foo(const char*); };";
   EXPECT_TRUE(matches(foo_class + "void r() { Foo f = Foo(\"hello world\"); }",
                       functionalCastExpr()));
 }
 
 TEST(FunctionalCast, DoesNotMatchOtherCasts) {
-  std::string FooClass = "class Foo { public: Foo(char*); };";
+  std::string FooClass = "class Foo { public: Foo(const char*); };";
   EXPECT_TRUE(
       notMatches(FooClass + "void r() { Foo f = (Foo) \"hello world\"; }",
                  functionalCastExpr()));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1965.2.patch
Type: text/x-patch
Size: 3867 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131102/5082edef/attachment.bin>