=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>,
=?utf-8?q?Félix?= Cloutier <fclout...@apple.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/116...@github.com>
erichkeane wrote:
> As an update, I've been adding more tests and I found that I can make
> improvements with follow-up work. Since these aren't stability problems or
> false positives, and we think the change is already big enough, I am
> currently planning to submit them as a separate PR once this lands. The
> things in question that are addressed:
>
> * Passing a format string with the wrong type (for instance, a `NSString`
> format string to a function accepting a `printf` format string) terminates
> verification without emitting a diagnostic. This is an issue with the
> `format` attribute too (it emits a -Wformat-nonliteral warning for it, which
> is bad categorization and unspecific about what needs to happen).
>
> * If you have a function with `format_matches` that calls another
> function with `format_matches` passing its own format argument, Clang doesn't
> verify that the two format strings are compatible.
>
> * `format_matches` diagnostics always point at the format string: for
> instance, directly at the format string argument in `printf("%s", "hello")`,
> but at the `fmt` variable definition in `const char *const fmt = "%s";
> printf(fmt, "hello")`. This can create situations where the diagnostic shows
> the problematic format strings, but not the format function call that
> references them. `Format` attribute checking uses
> `CheckFormatHandler::EmitFormatDiagnostic` to ensure that the format function
> call is consistently there (either directly where the warning points, or with
> a note if the call is elsewhere). Diagnostics for `format_matches` should do
> that too.
>
> * Reusing the "data argument not used by format string" diagnostic in the
> context of `format_matches` is not a great experience, this should be "more
> specifiers in format string than expected".
>
>
> Let me know if you would like me to fold anything from this list into this PR.
2 & 3 are pretty significant to me, but I'd be ok doing 3 as a followup.
4 seems easy enough to just roll into it anyway.
https://github.com/llvm/llvm-project/pull/116708
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
