* Warning message in remote.c when compiling
@ 2024-04-06 14:21 prpr 19xx
2024-04-06 16:12 ` Dragan Simic
0 siblings, 1 reply; 6+ messages in thread
From: prpr 19xx @ 2024-04-06 14:21 UTC (permalink / raw
To: git
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.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Warning message in remote.c when compiling
2024-04-06 14:21 Warning message in remote.c when compiling prpr 19xx
@ 2024-04-06 16:12 ` Dragan Simic
2024-04-07 1:38 ` Jeff King
0 siblings, 1 reply; 6+ messages in thread
From: Dragan Simic @ 2024-04-06 16:12 UTC (permalink / raw
To: prpr 19xx; +Cc: git
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?
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warning message in remote.c when compiling
2024-04-06 16:12 ` Dragan Simic
@ 2024-04-07 1:38 ` Jeff King
2024-04-07 5:10 ` Dragan Simic
2024-04-07 5:40 ` Junio C Hamano
0 siblings, 2 replies; 6+ messages in thread
From: Jeff King @ 2024-04-07 1:38 UTC (permalink / raw
To: Dragan Simic; +Cc: prpr 19xx, git
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.
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warning message in remote.c when compiling
2024-04-07 1:38 ` Jeff King
@ 2024-04-07 5:10 ` Dragan Simic
2024-04-07 5:40 ` Junio C Hamano
1 sibling, 0 replies; 6+ messages in thread
From: Dragan Simic @ 2024-04-07 5:10 UTC (permalink / raw
To: Jeff King; +Cc: prpr 19xx, git
Hello Jeff,
On 2024-04-07 03:38, Jeff King wrote:
> 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
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warning message in remote.c when compiling
2024-04-07 1:38 ` Jeff King
2024-04-07 5:10 ` Dragan Simic
@ 2024-04-07 5:40 ` Junio C Hamano
2024-04-07 16:47 ` prpr 19xx
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2024-04-07 5:40 UTC (permalink / raw
To: Jeff King; +Cc: Dragan Simic, prpr 19xx, git
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.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warning message in remote.c when compiling
2024-04-07 5:40 ` Junio C Hamano
@ 2024-04-07 16:47 ` prpr 19xx
0 siblings, 0 replies; 6+ messages in thread
From: prpr 19xx @ 2024-04-07 16:47 UTC (permalink / raw
To: Junio C Hamano; +Cc: Jeff King, Dragan Simic, git
On Sun, 7 Apr 2024 at 06:40, Junio C Hamano <gitster@pobox.com> wrote:
>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-07 16:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-06 14:21 Warning message in remote.c when compiling prpr 19xx
2024-04-06 16:12 ` Dragan Simic
2024-04-07 1:38 ` Jeff King
2024-04-07 5:10 ` Dragan Simic
2024-04-07 5:40 ` Junio C Hamano
2024-04-07 16:47 ` prpr 19xx
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).