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

[Richard Smith]

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:4488-4489
@@ -4487,2 +4487,4 @@
   "conversion from string literal to %0 is deprecated">, InGroup<DeprecatedWritableStr>;
+def ext_warn_deprecated_string_literal_conversion : ExtWarn<
+  "conversion from string literal to %0 is ill-formed in C++11">, InGroup<DeprecatedWritableStr>;
 def err_realimag_invalid_type : Error<"invalid type %0 to %1 operator">;
----------------
We spell these as `ext_deprecrated_...` (drop the `warn_`).

================
Comment at: lib/Sema/SemaOverload.cpp:1180-1182
@@ +1179,5 @@
+                                   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 &&
----------------
Do you really need to check these things? I would have thought checking `DeprecatedStirngLiteralToCharPtr` would be enough.

================
Comment at: lib/Sema/SemaOverload.cpp:3297-3305
@@ -3284,6 +3296,11 @@
   //   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;
 
----------------
This looks suspicious: is it not possible for two conversions involving a string-literal-to-char* conversion to have different ranks? For instance, what if one is a user-defined conversion sequence that converts string literal -> char* -> class type, and the other just converts string literal -> char*?

I think what you want is to start this function with something like:

  if (getLangOpts().CPlusPlus11 && !getLangOpts().WritableStrings &&
      ICS1.DeprecatedStringLiteralToCharPtr != ICS2.DeprecatedStringLiteralToCharPtr)
    return ICS1.DeprecatedStringLiteralToCharPtr ? ICS::Worse : ICS::Better;

(That is, any ill-formed conversion is worse than any non-ill-formed conversion.)

================
Comment at: lib/Sema/SemaExprCXX.cpp:3024-3027
@@ +3023,6 @@
+        !getLangOpts().WritableStrings) {
+      Diag(From->getLocStart(),
+           getLangOpts().CPlusPlus11
+               ? diag::ext_warn_deprecated_string_literal_conversion
+               : diag::warn_deprecated_string_literal_conversion)
+          << ToType.getNonReferenceType();
----------------
You need to issue an Error here, not just an ExtWarn, if we're in a SFINAE context.


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