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

[Richard Smith]

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:4493-4496
@@ -4492,2 +4492,6 @@
   "conversion from string literal to %0 is deprecated">, InGroup<DeprecatedWritableStr>;
+def ext_deprecated_string_literal_conversion : ExtWarn<
+  "conversion from string literal to %0 is ill-formed in C++11">, InGroup<DeprecatedWritableStr>;
+def err_deprecated_string_literal_conversion : Error<
+  "conversion from string literal to %0 is ill-formed in C++11">;
 def warn_deprecated_string_literal_conversion_c : Warning<
----------------
Since the last revision of your patch we've added a new way to do this. Add `SFINAEFailure` to the `ext_` diagnostic and drop the `err_` one:

    def ext_deprecated_string_literal_conversion : ExtWarn<
      "conversion from string literal to %0 is ill-formed in C++11">,
      InGroup<DeprecatedWritableStr>, SFINAEFailure;

================
Comment at: lib/Sema/SemaOverload.cpp:1152
@@ -1151,2 +1151,3 @@
         ICS.Standard.CopyConstructor = Constructor;
+        ICS.Standard.DeprecatedStringLiteralToCharPtr = false;
         if (ToCanon != FromCanon)
----------------
This is redundant; `StandardConversionSequence::setAsIdentityConversion` already does this.

================
Comment at: lib/Sema/SemaOverload.cpp:1245
@@ -1243,2 +1244,3 @@
     ICS.Standard.setAllToTypes(ToType);
+    ICS.Standard.DeprecatedStringLiteralToCharPtr = false;
 
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:2987
@@ -2984,2 +2986,3 @@
     User.FoundConversionFunction = Best->FoundDecl;
+    User.Before.DeprecatedStringLiteralToCharPtr = false;
     User.After.setAsIdentityConversion();
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:2991
@@ -2987,2 +2990,3 @@
     User.After.setAllToTypes(ToType);
+    User.After.DeprecatedStringLiteralToCharPtr = false;
     return OR_Success;
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:3183
@@ -3178,2 +3182,3 @@
         User.Before.setAsIdentityConversion();
+        User.Before.DeprecatedStringLiteralToCharPtr = false;
       } else {
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:3198
@@ -3192,2 +3197,3 @@
       User.After.setAllToTypes(ToType);
+      User.After.DeprecatedStringLiteralToCharPtr = false;
       return OR_Success;
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:4419
@@ -4397,2 +4418,3 @@
     ICS.Standard.ObjCLifetimeConversionBinding = false;
+    ICS.Standard.DeprecatedStringLiteralToCharPtr = false;
   } else if (ICS.isUserDefined()) {
----------------
This looks wrong: if `TryImplicitConversion` performed a deprecated conversion to `char*`, we still have one in this conversion sequence. This might matter for a case like binding a `'char *const &'` to a string literal.

================
Comment at: lib/Sema/SemaOverload.cpp:4547
@@ -4523,2 +4546,3 @@
       Result.UserDefined.Before.setAllToTypes(QualType());
+      Result.UserDefined.Before.DeprecatedStringLiteralToCharPtr = false;
 
----------------
This is redundant.

================
Comment at: lib/Sema/SemaOverload.cpp:4552
@@ -4527,2 +4551,3 @@
       Result.UserDefined.After.setAllToTypes(ToType);
+      Result.UserDefined.After.DeprecatedStringLiteralToCharPtr = false;
       Result.UserDefined.ConversionFunction = 0;
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:4645
@@ -4619,2 +4644,3 @@
       Result.Standard.setAllToTypes(ToType);
+      Result.Standard.DeprecatedStringLiteralToCharPtr = false;
     }
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:4808
@@ -4781,2 +4807,3 @@
     = (Method->getRefQualifier() == RQ_None);
+  ICS.Standard.DeprecatedStringLiteralToCharPtr = false;
   return ICS;
----------------
Likewise.

================
Comment at: lib/Sema/SemaOverload.cpp:3330
@@ +3329,3 @@
+
+  if (S.getLangOpts().CPlusPlus11 && !S.getLangOpts().WritableStrings &&
+      hasDeprecatedStringLiteralToCharPtrConversion(ICS1) !=
----------------
Please add a brief comment before this explaining that the deprecated conversion is ill-formed in c++11, so we rank any conversion sequence that uses it as worse than any sequence that does not.

================
Comment at: test/SemaCXX/overload-0x.cpp:22-24
@@ +21,5 @@
+  {
+    // CXX03: call void @_ZN7PR163141fEPc
+    // CXX11: call i32* (...)* @_ZN7PR163141fEz
+    f("foo");
+#if __cplusplus < 201103L
----------------
Please use a syntax-only test for this, rather than an IR generation test. For instance:

    int &r = f("foo");
  #if __cplusplus < 201103L
    // expected-warning at -2 {{deprecated}}
    // expected-error at -3 {{cannot bind}}
  #endif


================
Comment at: test/SemaCXX/overload-0x.cpp:42
@@ +41,3 @@
+#else
+    // expected-warning at -4 {{warning: conversion from string literal to 'char *' is deprecated}}
+#endif
----------------
This won't work: "warning: " is not part of the string that -verify checks.

================
Comment at: lib/Sema/SemaExprCXX.cpp:3039
@@ +3038,3 @@
+          getLangOpts().CPlusPlus11
+              ? (isSFINAEContext()
+                     ? diag::err_deprecated_string_literal_conversion
----------------
Please add a test for the case where this happens in a SFINAE context. For instance:

  int f(int, char*);
  template<typename T> void g(T, decltype(f(T{}, "foo")));
  template<typename T> int &g(...);
  int &r = g(0, 0); // ok in c++11, picks g(...) because g<int> has a substitution failure

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:4494
@@ +4493,3 @@
+def ext_deprecated_string_literal_conversion : ExtWarn<
+  "conversion from string literal to %0 is ill-formed in C++11">, InGroup<DeprecatedWritableStr>;
+def err_deprecated_string_literal_conversion : Error<
----------------
We don't phrase warnings this way elsewhere in the compiler. For consistency, word this as:

  "ISO C++11 does not allow conversion from string literal to %0"


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