Page MenuHomePhabricator

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

Authored by fcloutier on May 9 2022, 11:55 AM.

Details

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

Diff Detail

Event Timeline

fcloutier created this revision.May 9 2022, 11:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2022, 11:55 AM
fcloutier requested review of this revision.May 9 2022, 11:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 9 2022, 11:55 AM
ahatanak accepted this revision.May 12 2022, 10:17 AM

LGTM with one minor comment.

clang/lib/Sema/SemaDeclAttr.cpp
3864

You can fold this into the if below.

This revision is now accepted and ready to land.May 12 2022, 10:17 AM
This revision was landed with ongoing or failed builds.May 12 2022, 11:12 AM
This revision was automatically updated to reflect the committed changes.