[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112579#3097890 <https://reviews.llvm.org/D112579#3097890>, @fcloutier wrote:

> Thanks for looking, Aaron.

Thank you for the detailed response!

> Type checking may well actually need to be tested better for the case where variadic argument promotions don't happen, but there's a fairly finite number of ways it could go wrong, and I still think that's the easier way to go.

I think this is the current thing for us to test and try out. It shouldn't be impossible to do somewhat exhaustive testing (there's only so many format specifiers and length modifiers to worry about), but it's those edge cases that have me worried.

> I don't think that we can easily distinguish between parameters created by variadic templates and fixed parameters from a function without variadic templates.

Agreed. This should be checking the instantiations, so by that point, the variadic template is really more like a fixed parameter list anyway.

> I also think that most people who write functions like llvm::format aggressively do not want to be in charge of type-checking the format string themselves, and would much rather defer to Clang's existing type checker. If Clang allows the format attribute to type-check parameters created by variadic templates, it follows that the path of least resistance is to allow it on functions with fixed arguments.

Also agreed.

> With that said, there are still use cases for allowing format strings in functions with fixed arguments. (Interestingly, it would not be possible to mix fixed format arguments with variadic format arguments in a C function, so maybe we can prevent that altogether, but it does not remove from the general usefulness of the feature). You'll have to take my word for this, but it's one of the handful of blockers that we have for very broad adoption of -Wformat-nonliteral. The story usually goes like this: there's some function `NSString *foo(NSString *fmt, NSMyFoo *obj1, NSMyBar *obj2)` that needs to do something with `obj1` and `obj2` before it decides whether it actually wants to print them, return something unrelated altogether, throw an exception, etc. General limitations of the C language make it difficult to untangle the print parts from the logic parts for reasons which, to me, align rather closely to compiler capriciousness given how close it is to supporting the feature. This is more common in Objective-C code bases as %@ is a universal format specifier for all object types.
>
> Another way we could approach this problem is to have a new attribute to say that a format argument must have a string that conforms to another constant format string. For instance, I think that `NSString *foo(NSString *fmt, NSMyFoo *obj1, NSMyBar *obj2) __attribute__((format_like(1, @"hello %@ %@!")))`, by the compiler checks that `fmt` has specifiers equivalent to %@ %@, would give us roughly the same safety improvements. It would allow for even more fun things, like passing a format string to a function that will then itself supply arguments to a formatting function.
>
> What do you think?

I think the second option may be an idea worth exploring, but if we can avoid adding another attribute, that's usually preferred. I think that if we add sufficient test coverage, it'd make sense to use the existing attribute. As Arthur pointed out, ultimately this stuff is expected to be passed to `printf` (et al) and so long as the attribute continues to honor pointing out issues with that goal, I think it's a reasonable one to use in these situations. I think we only need to consider a secondary attribute if we find that the semantics we need are sufficiently different to warrant it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112579/new/

https://reviews.llvm.org/D112579