[clang] Allow all string types for all attribute(format) styles

[fcloutier]

Summary

It's not unusual to see functions like this:

void log(const char *fmt, ...) {
    va_list ap;
    va_start(ap, fmt);
    NSLogv([NSString stringWithCString:fmt], ap);
    va_end(ap);
}

Currently, such a function can't be annotated with __attribute__((format)). It can't do __attribute__((format(printf))) because printf does not accept the %@ format specifier (while this function does support it) and it can't do __attribute__((format(NSString))) because the format argument is not a NSString *. It's not always reasonable to ask the owners of these functions to change the type of the format argument as it can be an ABI-breaking change. The only viable thing to do is to not annotate log at all, which precludes using -Wformat-nonliteral (as it will trigger on the NSLogv line) and may hide format bugs in users of log.

This patch allows all string types for all format kinds and leaves it up to the user to convert the format string with a function that has the correct __attribute__((format_arg)) annotation. The example above can now be annotated with __attribute__((format(NSString, 1, 2))) and pass -Wformat-nonliteral, provided that [NSString stringWithCString:] is modified to have __attribute__((format_arg(1))).

It is still a diagnostic to mix format strings with different format styles, so for instance you can't pass a printf-style format string to [NSString stringWithFormat:] even if you've converted it to a NSString *:

void log(const char *fmt, ...) __attribute__((format(printf, 1, 2))) {
    va_list ap;
    va_start(ap, fmt);
    NSLogv([NSString stringWithCString:fmt], ap); // warning: format string is not a string literal [-Wformat-nonliteral]
    va_end(ap);
}

rdar://89060618

[ahatanak]

LGTM with one minor comment.

clang/lib/Sema/SemaDeclAttr.cpp

3864

You can fold this into the if below.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_ownership_returns_index_mismatch : Error<
   "'ownership_returns' attribute index does not match; here it is %0">;
 def note_ownership_returns_index_mismatch : Note<
   "declared with index %0 here">;
 def err_format_strftime_third_parameter : Error<
   "strftime format attribute requires 3rd parameter to be 0">;
 def err_format_attribute_requires_variadic : Error<
   "format attribute requires variadic function">;
-def err_format_attribute_not : Error<"format argument not %0">;
+def err_format_attribute_not : Error<"format argument not a string type">;
 def err_format_attribute_result_not : Error<"function does not return %0">;
 def err_format_attribute_implicit_this_format_string : Error<
   "format attribute cannot specify the implicit this argument as the format "
   "string">;
 def err_callback_attribute_no_callee : Error<
   "'callback' attribute specifies no callback callee">;
 def err_callback_attribute_invalid_callee : Error<
   "'callback' attribute specifies invalid callback callee">;

clang/lib/Sema/SemaDeclAttr.cpp

   QualType Ty = getFunctionOrMethodParamType(D, Idx.getASTIndex());
 
   bool NotNSStringTy = !isNSStringType(Ty, S.Context);
   if (NotNSStringTy &&
       !isCFStringType(Ty, S.Context) &&
       (!Ty->isPointerType() ||
        !Ty->castAs<PointerType>()->getPointeeType()->isCharType())) {
     S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-        << "a string type" << IdxExpr->getSourceRange()
-        << getFunctionOrMethodParamRange(D, 0);
+        << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, 0);
     return;
   }
   Ty = getFunctionOrMethodResultType(D);
   // replace instancetype with the class type
   auto Instancetype = S.Context.getObjCInstanceTypeDecl()->getTypeForDecl();
   if (Ty->getAs<TypedefType>() == Instancetype)
     if (auto *OMD = dyn_cast<ObjCMethodDecl>(D))
       if (auto *Interface = OMD->getClassInterface())

       return;
     }
     ArgIdx--;
   }
 
   // make sure the format string is really a string
   QualType Ty = getFunctionOrMethodParamType(D, ArgIdx);
 
-  if (Kind == CFStringFormat) {
-    if (!isCFStringType(Ty, S.Context)) {
-      S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-        << "a CFString" << IdxExpr->getSourceRange()
-        << getFunctionOrMethodParamRange(D, ArgIdx);
-      return;
-    }
-  } else if (Kind == NSStringFormat) {
-    // FIXME: do we need to check if the type is NSString*?  What are the
-    // semantics?
-    if (!isNSStringType(Ty, S.Context, /*AllowNSAttributedString=*/true)) {
-      S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-        << "an NSString" << IdxExpr->getSourceRange()
-        << getFunctionOrMethodParamRange(D, ArgIdx);
-      return;
-    }
-  } else if (!Ty->isPointerType() ||
-             !Ty->castAs<PointerType>()->getPointeeType()->isCharType()) {
+  bool NotNSStringTy = !isNSStringType(Ty, S.Context, true);
+  if (NotNSStringTy &&
+      !isCFStringType(Ty, S.Context) &&
+      (!Ty->isPointerType() ||
+       !Ty->castAs<PointerType>()->getPointeeType()->isCharType())) {
     S.Diag(AL.getLoc(), diag::err_format_attribute_not)
-      << "a string type" << IdxExpr->getSourceRange()
-      << getFunctionOrMethodParamRange(D, ArgIdx);
+      << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, ArgIdx);
     return;
   }
 
   // check the 3rd argument
   Expr *FirstArgExpr = AL.getArgAsExpr(2);
   uint32_t FirstArg;
   if (!checkUInt32Argument(S, AL, FirstArgExpr, FirstArg, 3))
     return;

clang/test/SemaObjC/format-strings-objc.m

 //===----------------------------------------------------------------------===//
 
 void check_nslog(unsigned k) {
   NSLog(@"%d%%", k); // no-warning
   NSLog(@"%s%lb%d", "unix", 10, 20); // expected-warning {{invalid conversion specifier 'b'}} expected-warning {{data argument not used by format string}}
 }
 
 // Check type validation
-extern void NSLog2(int format, ...) __attribute__((format(__NSString__, 1, 2))); // expected-error {{format argument not an NSString}}
-extern void CFStringCreateWithFormat2(int *format, ...) __attribute__((format(CFString, 1, 2))); // expected-error {{format argument not a CFString}}
+extern void NSLog2(int format, ...) __attribute__((format(__NSString__, 1, 2))); // expected-error {{format argument not a string type}}
+extern void CFStringCreateWithFormat2(int *format, ...) __attribute__((format(CFString, 1, 2))); // expected-error {{format argument not a string type}}
+
+// Check interoperability of strings
+extern void NSLog3(const char *, ...) __attribute__((format(__NSString__, 1, 2)));
+extern void CFStringCreateWithFormat3(const char *, ...) __attribute__((format(__NSString__, 1, 2)));
+extern void printf2(NSString *format, ...) __attribute__((format(printf, 1, 2)));
+
+extern NSString *CStringToNSString(const char *) __attribute__((format_arg(1)));
+
+void NSLog3(const char *fmt, ...) {
+  NSString *const nsFmt = CStringToNSString(fmt);
+  va_list ap;
+  va_start(ap, fmt);
+  NSLogv(nsFmt, ap);
+  va_end(ap);
+}
 
 // <rdar://problem/7068334> - Catch use of long long with int arguments.
 void rdar_7068334(void) {
   long long test = 500;  
   printf("%i ",test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
   NSLog(@"%i ",test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
   CFStringCreateWithFormat(CFSTR("%i"),test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
 }