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

[Félix Cloutier via Phabricator via cfe-commits]

fcloutier created this revision.
fcloutier added reviewers: NoQ, ahatanak, aaron.ballman.
Herald added a project: All.
fcloutier requested review of this revision.
Herald added a project: clang.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125254

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaObjC/format-strings-objc.m


Index: clang/test/SemaObjC/format-strings-objc.m
===================================================================
--- clang/test/SemaObjC/format-strings-objc.m
+++ clang/test/SemaObjC/format-strings-objc.m
@@ -60,8 +60,23 @@
 }
 
 // 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) {
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3661,8 +3661,7 @@
       (!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);
@@ -3862,27 +3861,13 @@
   // 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;
   }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3104,7 +3104,7 @@
   "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 "


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125254.428142.patch
Type: text/x-patch
Size: 4299 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220509/88ba7b4c/attachment.bin>