threads / discuss / 61281

Warning message in remote.c when compiling

Subject: Warning message in remote.c when compiling

## tl;dr

6 messages between Apr 6, 2024 and Apr 7, 2024.

replies: 5people: 4as markdown or json

prpr 19xx· Apr 6, 2024, 14:21 UTC · lore

Hi,

I get this warning message when compiling remote.c:

...
    CC remote.o
remote.c:596: warning: 'remotes_remote_get' declared inline after being called
remote.c:596: warning: previous declaration of 'remotes_remote_get' was here
    CC replace-object.o
...

This is from the "master" branch, but it's the same on "next". It's easily fixed with this patch:

diff --git a/remote.c b/remote.c
index 2b650b8..347f504 100644
--- a/remote.c
+++ b/remote.c
@@ -592,7 +592,7 @@ const char *pushremote_for_branch(struct branch
*branch, int *explicit)
                                             branch, explicit);
 }

-static struct remote *remotes_remote_get(struct remote_state *remote_state,
+static inline struct remote *remotes_remote_get(struct remote_state
*remote_state,
                                         const char *name);

 const char *remote_ref_for_branch(struct branch *branch, int for_push)

Thanks.

Dragan Simic· Apr 6, 2024, 16:12 UTC · re: prpr 19xx · lore

Re: Warning message in remote.c when compiling

Hello,

On 2024-04-06 16:21, prpr 19xx wrote:

Show 10 quoted lines

> I get this warning message when compiling remote.c:
> 
> ...
>     CC remote.o
> remote.c:596: warning: 'remotes_remote_get' declared inline after being 
> called
> remote.c:596: warning: previous declaration of 'remotes_remote_get' was 
> here
>     CC replace-object.o
> ...

Could you, please, provide more details about your environment, i.e. the operating system and compiler?

Show 21 quoted lines

> This is from the "master" branch, but it's the same on "next". It's
> easily fixed with this patch:
> 
> diff --git a/remote.c b/remote.c
> index 2b650b8..347f504 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -592,7 +592,7 @@ const char *pushremote_for_branch(struct branch
> *branch, int *explicit)
>                                              branch, explicit);
>  }
> 
> -static struct remote *remotes_remote_get(struct remote_state 
> *remote_state,
> +static inline struct remote *remotes_remote_get(struct remote_state
> *remote_state,
>                                          const char *name);
> 
>  const char *remote_ref_for_branch(struct branch *branch, int for_push)
> 
> Thanks.

Jeff King· Apr 7, 2024, 01:38 UTC · re: Dragan Simic · lore

Re: Warning message in remote.c when compiling

On Sat, Apr 06, 2024 at 06:12:34PM +0200, Dragan Simic wrote:

Show 16 quoted lines

> Hello,
> 
> On 2024-04-06 16:21, prpr 19xx wrote:
> > I get this warning message when compiling remote.c:
> > 
> > ...
> >     CC remote.o
> > remote.c:596: warning: 'remotes_remote_get' declared inline after being
> > called
> > remote.c:596: warning: previous declaration of 'remotes_remote_get' was
> > here
> >     CC replace-object.o
> > ...
> 
> Could you, please, provide more details about your environment,
> i.e. the operating system and compiler?

I'm also curious about which compiler, but I think it's a reasonable complaint. We forward-declare the static function, use it, and then later declare it inline. I didn't check to see what the standard says, but it seems like a funny thing to do in general.

It has been that way for a while; since 56eed3422c (remote: remove the_repository->remote_state from static methods, 2021-11-17), I think.

I don't really see any need to mark the wrapper as inline. It's one basic function call (on top of an interface which requires a callback anyway!), and I suspect many compilers would consider inlining anyway, since it's a static function.

Ditto for remotes_pushremote_get(), though it doesn't have a forward declaration.

-Peff

Dragan Simic· Apr 7, 2024, 05:10 UTC · re: Jeff King · lore

Re: Warning message in remote.c when compiling

Hello Jeff,

On 2024-04-07 03:38, Jeff King wrote:

Show 23 quoted lines

> On Sat, Apr 06, 2024 at 06:12:34PM +0200, Dragan Simic wrote:
> 
>> Hello,
>> 
>> On 2024-04-06 16:21, prpr 19xx wrote:
>> > I get this warning message when compiling remote.c:
>> >
>> > ...
>> >     CC remote.o
>> > remote.c:596: warning: 'remotes_remote_get' declared inline after being
>> > called
>> > remote.c:596: warning: previous declaration of 'remotes_remote_get' was
>> > here
>> >     CC replace-object.o
>> > ...
>> 
>> Could you, please, provide more details about your environment,
>> i.e. the operating system and compiler?
> 
> I'm also curious about which compiler, but I think it's a reasonable
> complaint. We forward-declare the static function, use it, and then
> later declare it inline. I didn't check to see what the standard says,
> but it seems like a funny thing to do in general.

The link below seems to provide more details. The way I see it, declarations and definitions should match, and the standard seems to support that. Though, not all compilers (or not all versions) complain in this particular case.

https://stackoverflow.com/a/62390378/22330192

Show 12 quoted lines

> It has been that way for a while; since 56eed3422c (remote: remove
> the_repository->remote_state from static methods, 2021-11-17), I think.
> 
> I don't really see any need to mark the wrapper as inline. It's one
> basic function call (on top of an interface which requires a callback
> anyway!), and I suspect many compilers would consider inlining anyway,
> since it's a static function.
> 
> Ditto for remotes_pushremote_get(), though it doesn't have a forward
> declaration.
> 
> -Peff

Junio C Hamano· Apr 7, 2024, 05:40 UTC · re: Jeff King · lore

Re: Warning message in remote.c when compiling

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> I don't really see any need to mark the wrapper as inline. It's one
> basic function call (on top of an interface which requires a callback
> anyway!), and I suspect many compilers would consider inlining anyway,
> since it's a static function.
>
> Ditto for remotes_pushremote_get(), though it doesn't have a forward
> declaration.

Yup. I presonally feel that we should get rid of "static inline" unless they appear in header files. The compilers should in general be able to do good enough job finding what to inline than we can (1) initially mark what to inline, and (2) update by dropping "inline" that is no longer appropriate as the code evolves.

Thanks.

prpr 19xx· Apr 7, 2024, 16:47 UTC · re: Junio C Hamano · lore

Re: Warning message in remote.c when compiling

On Sun, 7 Apr 2024 at 06:40, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

>
> Jeff King <peff@peff.net> writes:
>
> > I don't really see any need to mark the wrapper as inline. It's one
> > basic function call (on top of an interface which requires a callback
> > anyway!), and I suspect many compilers would consider inlining anyway,
> > since it's a static function.
> >
> > Ditto for remotes_pushremote_get(), though it doesn't have a forward
> > declaration.
>
> Yup.  I presonally feel that we should get rid of "static inline"
> unless they appear in header files.  The compilers should in general
> be able to do good enough job finding what to inline than we can (1)
> initially mark what to inline, and (2) update by dropping "inline"
> that is no longer appropriate as the code evolves.

The compiler is an ancient gcc 4.2.0 cross-compiler for a mipsel-linux-uclibc environment. It doesn't really matter though, as folk seem to agree the definition and declaration should match, which I think they should. I also agree that having inline probably makes no sense, as the compiler can usually work this stuff out itself. So I don't mind whether all the inlines get removed or they all stay, as long as they are all effectively consistent, which they are currently not, and the compiler righty (to my mind) complains.

Thanks.