[PATCH] D13446: [PATCH] Add checker discouraging definition of variadic function definitions in C++

[Aaron Ballman via cfe-commits]

On Tue, Oct 6, 2015 at 4:51 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Tue, Oct 6, 2015 at 4:49 PM, Joerg Sonnenberger via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>> On Tue, Oct 06, 2015 at 04:20:05PM -0400, Aaron Ballman via cfe-commits wrote:
>>> On Tue, Oct 6, 2015 at 4:12 PM, Joerg Sonnenberger via cfe-commits
>>> <cfe-commits at lists.llvm.org> wrote:
>>> > On Mon, Oct 05, 2015 at 07:36:08PM +0000, Aaron Ballman via cfe-commits wrote:
>>> >> C-style variadic functions (using an ellipsis) can be dangerous in C++
>>> >> due to the inherit lack of type safety with argument passing. Better
>>> >> alternatives exist, such as function currying (like STL stream objects
>>> >> use), or function parameter packs. This patch adds a checker to
>>> >> diagnose definitions of variadic functions in C++ code, but still
>>> >> allows variadic function declarations, as those can be safely used to
>>> >> good effect for SFINAE patterns.
>>> >
>>> > I would restrict this a bit to exclude function definitions with C
>>> > linkage. If you have such a ABI requirement, you normally can't replace
>>> > it with any of the alternatives.
>>>
>>> Under what circumstances would you run into this issue with C++ code?
>>> The only circumstance I can think of is if you have a C API that takes
>>> a function pointer to a varargs function as an argument. Is that the
>>> situation you are thinking of, or are there others?
>>
>> Consider a stdio implementation written in C++? Wouldn't the
>> implementation of e.g. vprintf trigger exactly this warning?
>
> It would, but it would also trigger a *lot* of warnings that aren't
> directed at implementers as well, such as anything about relying on
> implementation-defined or undefined behavior (like the implementation
> of offsetof(), as a trivial example). It's a good point to keep in
> mind though.

Thank you for the suggestion, Joerg -- I've implemented it in r249555,
and updated the CERT rule wording to match.

~Aaron