threads / patch / 48950

patch, 2 partsPedantic fixes for Apple clang

Subject: [PATCH 0/2] Pedantic fixes for Apple clang

## tl;dr

8 messages between Jul 24, 2018 and Jul 25, 2018. Diffs are folded; open one to read it.

replies: 7people: 4as markdown or json

Beat Bolli· Jul 24, 2018, 21:52 UTC · lore

Following up on my previous series bb/pedantic for gcc, here are two fixes for pedantic compilation under MacOS 10.13.6 (High Sierra) with the command line tools of Xcode Version 9.4.1 (9F2000).

Beat Bolli (2):
  packfile: drop a repeated enum declaration
  remote-odb: un-inline function remote_odb_reinit

 packfile.h   | 1 -
 remote-odb.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

-- 
2.18.0

Beat Bolli· Jul 24, 2018, 21:52 UTC · re: Beat Bolli · lore

[PATCH 2/2] remote-odb: un-inline function remote_odb_reinit

When compiling under Apple LLVM version 9.1.0 (clang-902.0.39.2) with "make DEVELOPER=1 DEVOPTS=pedantic", the compiler says

    remote-odb.c:87:2: error: static function 'remote_odb_do_init' is
    used in an inline function with external linkage
    [-Werror,-Wstatic-in-inline]

Remove the inline specifier that would only make sense if remote_odb_reinit were defined in the header file. Moving it into the header is not possible because the called function remote_odb_do_init is static and thus not visible from the includers of the header.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 remote-odb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to remote-odb.c +1 −1

diff --git a/remote-odb.c b/remote-odb.c
index 847a86505778..49cf8e30aa92 100644
--- a/remote-odb.c
+++ b/remote-odb.c
@@ -82,7 +82,7 @@ static inline void remote_odb_init(void)
 	remote_odb_do_init(0);
 }
 
-inline void remote_odb_reinit(void)
+void remote_odb_reinit(void)
 {
 	remote_odb_do_init(1);
 }

-- 
2.18.0

Jonathan Nieder· Jul 24, 2018, 21:59 UTC · re: Beat Bolli · lore

Re: [PATCH 2/2] remote-odb: un-inline function remote_odb_reinit

Hi,

Beat Bolli wrote:

Show 16 quoted lines

> When compiling under Apple LLVM version 9.1.0 (clang-902.0.39.2) with
> "make DEVELOPER=1 DEVOPTS=pedantic", the compiler says
>
>     remote-odb.c:87:2: error: static function 'remote_odb_do_init' is
>     used in an inline function with external linkage
>     [-Werror,-Wstatic-in-inline]
>
> Remove the inline specifier that would only make sense if
> remote_odb_reinit were defined in the header file. Moving it into the
> header is not possible because the called function remote_odb_do_init is
> static and thus not visible from the includers of the header.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>  remote-odb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

What branch does this apply to?

[...]

Show 8 quoted lines

> --- a/remote-odb.c
> +++ b/remote-odb.c
> @@ -82,7 +82,7 @@ static inline void remote_odb_init(void)
>  	remote_odb_do_init(0);
>  }
>  
> -inline void remote_odb_reinit(void)
> +void remote_odb_reinit(void)

This looks like an oversight in https://public-inbox.org/git/20180713174959.16748-6-chriscool@tuxfamily.org/: there isn't any reason for this function to be inline.

Christian, can you squash it in on your next reroll?

Thanks, Jonathan

Beat Bolli· Jul 24, 2018, 22:03 UTC · re: Jonathan Nieder · lore

Re: [PATCH 2/2] remote-odb: un-inline function remote_odb_reinit

Hi Jonathan

On 24.07.18 23:59, Jonathan Nieder wrote:

Show 22 quoted lines

> Hi,
> 
> Beat Bolli wrote:
> 
>> When compiling under Apple LLVM version 9.1.0 (clang-902.0.39.2) with
>> "make DEVELOPER=1 DEVOPTS=pedantic", the compiler says
>>
>>     remote-odb.c:87:2: error: static function 'remote_odb_do_init' is
>>     used in an inline function with external linkage
>>     [-Werror,-Wstatic-in-inline]
>>
>> Remove the inline specifier that would only make sense if
>> remote_odb_reinit were defined in the header file. Moving it into the
>> header is not possible because the called function remote_odb_do_init is
>> static and thus not visible from the includers of the header.
>>
>> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
>> ---
>>  remote-odb.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> What branch does this apply to?

To pu as of today.

Show 15 quoted lines

> [...]
>> --- a/remote-odb.c
>> +++ b/remote-odb.c
>> @@ -82,7 +82,7 @@ static inline void remote_odb_init(void)
>>  	remote_odb_do_init(0);
>>  }
>>  
>> -inline void remote_odb_reinit(void)
>> +void remote_odb_reinit(void)
> 
> This looks like an oversight in
> https://public-inbox.org/git/20180713174959.16748-6-chriscool@tuxfamily.org/:
> there isn't any reason for this function to be inline.
> 
> Christian, can you squash it in on your next reroll?

That would probably make sense. I didn't check how mature the topics were that caused errors.

Beat

Christian Couder· Jul 25, 2018, 07:29 UTC · re: Beat Bolli · lore

Re: [PATCH 2/2] remote-odb: un-inline function remote_odb_reinit

Hi,

On Wed, Jul 25, 2018 at 12:03 AM, Beat Bolli <dev+git@drbeat.li> wrote:

>
> On 24.07.18 23:59, Jonathan Nieder wrote:
>>
>> Beat Bolli wrote:

Show 11 quoted lines

>>> -inline void remote_odb_reinit(void)
>>> +void remote_odb_reinit(void)
>>
>> This looks like an oversight in
>> https://public-inbox.org/git/20180713174959.16748-6-chriscool@tuxfamily.org/:
>> there isn't any reason for this function to be inline.
>>
>> Christian, can you squash it in on your next reroll?
>
> That would probably make sense. I didn't check how mature the topics
> were that caused errors.

Ok, it is in the next version I will send.

Thanks, Christian.

Junio C Hamano· Jul 25, 2018, 21:16 UTC · re: Christian Couder · lore

Re: [PATCH 2/2] remote-odb: un-inline function remote_odb_reinit

Christian Couder <christian.couder@gmail.com> writes:

Show 21 quoted lines

> Hi,
>
> On Wed, Jul 25, 2018 at 12:03 AM, Beat Bolli <dev+git@drbeat.li> wrote:
>>
>> On 24.07.18 23:59, Jonathan Nieder wrote:
>>>
>>> Beat Bolli wrote:
>
>>>> -inline void remote_odb_reinit(void)
>>>> +void remote_odb_reinit(void)
>>>
>>> This looks like an oversight in
>>> https://public-inbox.org/git/20180713174959.16748-6-chriscool@tuxfamily.org/:
>>> there isn't any reason for this function to be inline.
>>>
>>> Christian, can you squash it in on your next reroll?
>>
>> That would probably make sense. I didn't check how mature the topics
>> were that caused errors.
>
> Ok, it is in the next version I will send.

OK, then I'll ignore this one for now.

Beat Bolli· Jul 24, 2018, 21:52 UTC · re: Beat Bolli · lore

[PATCH 1/2] packfile: drop a repeated enum declaration

When compiling under Apple LLVM version 9.1.0 (clang-902.0.39.2) with "make DEVELOPER=1 DEVOPTS=pedantic", the compiler says

    error: redeclaration of already-defined enum 'object_type' is a GNU
    extension [-Werror,-Wgnu-redeclared-enum]

According to https://en.cppreference.com/w/c/language/declarations (section "Redeclaration"), a repeated declaration after the definition is only legal for structs and unions, but not for enums.

Drop the belated declaration of enum object_type. It seems that each includer of packfile.h includes the definition of the enum before including packfile.h.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 packfile.h | 1 -
 1 file changed, 1 deletion(-)

Show changes to packfile.h +0 −1

diff --git a/packfile.h b/packfile.h
index 51383774ec72..9b6198c4c7e0 100644
--- a/packfile.h
+++ b/packfile.h
@@ -6,7 +6,6 @@
 /* in object-store.h */
 struct packed_git;
 struct object_info;
-enum object_type;
 
 /*
  * Generate the filename to be used for a pack file with checksum "sha1" and

-- 
2.18.0

Jonathan Nieder· Jul 24, 2018, 21:57 UTC · re: Beat Bolli · lore

Re: [PATCH 1/2] packfile: drop a repeated enum declaration

Hi,

Beat Bolli wrote:

Show 10 quoted lines

> --- a/packfile.h
> +++ b/packfile.h
> @@ -6,7 +6,6 @@
>  /* in object-store.h */
>  struct packed_git;
>  struct object_info;
> -enum object_type;
>  
>  /*
>   * Generate the filename to be used for a pack file with checksum "sha1" and

Good catch. This means packfile.h should #include "cache.h" --- otherwise, it is not usable in a file that does

	#include "git-compat-util.h"
	#include "packfile.h"

Thanks and hope that helps, Jonathan