Re: git: 402dbdd98acc - main - Adjust function definition in arm's mv_common.c to avoid clang 15 warning

On 15 Aug 2022, at 23:23, Jessica Clarke <jrtc27@freebsd.org> wrote:
> 
> =On 15 Aug 2022, at 22:07, Konstantin Belousov <kostikbel@gmail.com> wrote:
>> 
>> On Mon, Aug 15, 2022 at 06:49:17PM +0000, Dimitry Andric wrote:
>>> The branch main has been updated by dim:
>>> 
>>> URL: https://cgit.FreeBSD.org/src/commit/?id=402dbdd98acc7fa94d1d4cd01903e987d2409336
>>> 
>>> commit 402dbdd98acc7fa94d1d4cd01903e987d2409336
>>> Author:     Dimitry Andric <dim@FreeBSD.org>
>>> AuthorDate: 2022-08-15 18:02:13 +0000
>>> Commit:     Dimitry Andric <dim@FreeBSD.org>
>>> CommitDate: 2022-08-15 18:48:33 +0000
>>> 
>>>   Adjust function definition in arm's mv_common.c to avoid clang 15 warning
>>> 
>>>   With clang 15, the following -Werror warning is produced:
>>> 
>>>       sys/arm/mv/mv_common.c:414:20: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
>>>       mv_check_soc_family()
>>>                          ^
>>>                           void
>>> 
>>>   This is because mv_check_soc_family() is declared with a (void) argument
>>>   list, but defined with an empty argument list. Make the definition match
>>>   the declaration.
>>> 
>>>   MFC after:      3 days
>>> ---
>>> sys/arm/mv/mv_common.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/sys/arm/mv/mv_common.c b/sys/arm/mv/mv_common.c
>>> index 6e1d12f8c7a7..c2e25c686583 100644
>>> --- a/sys/arm/mv/mv_common.c
>>> +++ b/sys/arm/mv/mv_common.c
>>> @@ -411,7 +411,7 @@ static int mv_win_cesa_attr_armadaxp(int eng_sel)
>>> }
>>> 
>>> enum soc_family
>>> -mv_check_soc_family()
>>> +mv_check_soc_family(void)
>>> {
>>> 	uint32_t dev, rev;
>>> 
>> I am actually curious about this and other commits.  From the ISO/IEC 9899:2023
>> draft N3047, 6.7.6.3 Function declarators, clause 13:
>> 
>> For a function declarator without a parameter type list: the effect
>> is as if it were declared with a parameter type list consisting of
>> the keyword void. A function declarator provides a prototype for the
>> function 177).
>> 
>> Then the note 177:
>> This implies that a function definition without a parameter list
>> provides a prototype, and that subsequent calls to that function in the
>> same translation unit are constrained not to provide any argument to the
>> function call. Thus a definition of a function without parameter list
>> and one that has such a list consisting of the keyword void are fully
>> equivalent.
>> 
>> And more, in the 6.9.1 Function definitions clause 13, there is an example:
>> typedef int F(void);  // type F is "function with no parameters
>>                     // returning int"
>> int g() { /* ... */ } // RIGHT: g has type compatible with F
>> 
>> So why does clang enforce the warning?
> 
> I’m not sure why this is a warning; an empty parameter list in a
> function declaration that is part of a definition has always been the
> same as (void) (unless you have K&R-style arguments, which the compiler
> can also see). C99 6.11.6 Function declarators does however say:
> 
>> 1 The use of function declarators with empty parentheses (not
>> prototype-format parameter type declarators) is an obsolescent feature.
> 
> So technically warning for pre-C23 is compliant with that, though a bit
> annoying in the definition case given the semantics have stayed the
> same and been un-deprecated. Regardless, it’s probably best practice to
> be consistent and use (void) in the definitions so it matches any
> declarations rather than have this be a special case that can differ.

This changed upstream here:

https://github.com/llvm/llvm-project/commit/11da1b53d8cd3507959022cd790d5a7ad4573d94

where this warning got enabled for -Wstrict-prototypes.

Another related change was:

https://github.com/llvm/llvm-project/commit/385e7df33046d7292612ee1e3ac00a59d8bc0441

Reading back some comments on the llvm IRC channel, I suppose another
method could be to turn off -Wstrict-prototypes for clang >= 15, as it
will already warn on problematic cases of call sites not matching
existing declarations. For example: https://godbolt.org/z/7EPTYhKrY

But I think it is better to have the definitions matching the
declarations exactly. We should sweep through the whole tree and get rid
of all K&R functions too. I believe Warner wanted to attempt that.

-Dimitry