{"thread":{"id":"61281","subject":"Warning message in remote.c when compiling","startedAt":"2024-04-06T14:22:07Z","lastAt":"2024-04-07T16:47:39Z","messageCount":6,"participants":["prpr 19xx","Dragan Simic","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"492385","messageId":"CAA1Aqdvj6Eyp9jGaAxTf8p0Eh_rCPydOpin3D5QYHy8sqOoOsw@mail.gmail.com","threadId":"61281","inReplyTo":null,"subject":"Warning message in remote.c when compiling","fromName":"prpr 19xx","fromEmail":"prpr19xx@gmail.com","sentAt":"2024-04-06T14:21:55Z","receivedAt":"2024-04-06T14:22:07Z","isPatch":false,"sender":{"key":"prpr19xx@gmail.com","avatar":null},"body":"Hi,\n\nI get this warning message when compiling remote.c:\n\n...\n    CC remote.o\nremote.c:596: warning: 'remotes_remote_get' declared inline after being called\nremote.c:596: warning: previous declaration of 'remotes_remote_get' was here\n    CC replace-object.o\n...\n\nThis is from the \"master\" branch, but it's the same on \"next\". It's\neasily fixed with this patch:\n\ndiff --git a/remote.c b/remote.c\nindex 2b650b8..347f504 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -592,7 +592,7 @@ const char *pushremote_for_branch(struct branch\n*branch, int *explicit)\n                                             branch, explicit);\n }\n\n-static struct remote *remotes_remote_get(struct remote_state *remote_state,\n+static inline struct remote *remotes_remote_get(struct remote_state\n*remote_state,\n                                         const char *name);\n\n const char *remote_ref_for_branch(struct branch *branch, int for_push)\n\nThanks.\n"},{"id":"492393","messageId":"7f0da057773d39add4ede71667e9ff70@manjaro.org","threadId":"61281","inReplyTo":"CAA1Aqdvj6Eyp9jGaAxTf8p0Eh_rCPydOpin3D5QYHy8sqOoOsw@mail.gmail.com","subject":"Re: Warning message in remote.c when compiling","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-06T16:12:34Z","receivedAt":"2024-04-06T16:12:36Z","isPatch":false,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello,\n\nOn 2024-04-06 16:21, prpr 19xx wrote:\n> I get this warning message when compiling remote.c:\n> \n> ...\n>     CC remote.o\n> remote.c:596: warning: 'remotes_remote_get' declared inline after being \n> called\n> remote.c:596: warning: previous declaration of 'remotes_remote_get' was \n> here\n>     CC replace-object.o\n> ...\n\nCould you, please, provide more details about your environment,\ni.e. the operating system and compiler?\n\n> This is from the \"master\" branch, but it's the same on \"next\". It's\n> easily fixed with this patch:\n> \n> diff --git a/remote.c b/remote.c\n> index 2b650b8..347f504 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -592,7 +592,7 @@ const char *pushremote_for_branch(struct branch\n> *branch, int *explicit)\n>                                              branch, explicit);\n>  }\n> \n> -static struct remote *remotes_remote_get(struct remote_state \n> *remote_state,\n> +static inline struct remote *remotes_remote_get(struct remote_state\n> *remote_state,\n>                                          const char *name);\n> \n>  const char *remote_ref_for_branch(struct branch *branch, int for_push)\n> \n> Thanks.\n"},{"id":"492427","messageId":"20240407013843.GE1085004@coredump.intra.peff.net","threadId":"61281","inReplyTo":"7f0da057773d39add4ede71667e9ff70@manjaro.org","subject":"Re: Warning message in remote.c when compiling","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-04-07T01:38:43Z","receivedAt":"2024-04-07T01:38:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 06, 2024 at 06:12:34PM +0200, Dragan Simic wrote:\n\n> Hello,\n> \n> On 2024-04-06 16:21, prpr 19xx wrote:\n> > I get this warning message when compiling remote.c:\n> > \n> > ...\n> >     CC remote.o\n> > remote.c:596: warning: 'remotes_remote_get' declared inline after being\n> > called\n> > remote.c:596: warning: previous declaration of 'remotes_remote_get' was\n> > here\n> >     CC replace-object.o\n> > ...\n> \n> Could you, please, provide more details about your environment,\n> i.e. the operating system and compiler?\n\nI'm also curious about which compiler, but I think it's a reasonable\ncomplaint. We forward-declare the static function, use it, and then\nlater declare it inline. I didn't check to see what the standard says,\nbut it seems like a funny thing to do in general.\n\nIt has been that way for a while; since 56eed3422c (remote: remove\nthe_repository->remote_state from static methods, 2021-11-17), I think.\n\nI don't really see any need to mark the wrapper as inline. It's one\nbasic function call (on top of an interface which requires a callback\nanyway!), and I suspect many compilers would consider inlining anyway,\nsince it's a static function.\n\nDitto for remotes_pushremote_get(), though it doesn't have a forward\ndeclaration.\n\n-Peff\n"},{"id":"492437","messageId":"0d8660bd28ea10d062a3bf71d5d66b8f@manjaro.org","threadId":"61281","inReplyTo":"20240407013843.GE1085004@coredump.intra.peff.net","subject":"Re: Warning message in remote.c when compiling","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2024-04-07T05:10:27Z","receivedAt":"2024-04-07T05:10:34Z","isPatch":false,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"Hello Jeff,\n\nOn 2024-04-07 03:38, Jeff King wrote:\n> On Sat, Apr 06, 2024 at 06:12:34PM +0200, Dragan Simic wrote:\n> \n>> Hello,\n>> \n>> On 2024-04-06 16:21, prpr 19xx wrote:\n>> > I get this warning message when compiling remote.c:\n>> >\n>> > ...\n>> >     CC remote.o\n>> > remote.c:596: warning: 'remotes_remote_get' declared inline after being\n>> > called\n>> > remote.c:596: warning: previous declaration of 'remotes_remote_get' was\n>> > here\n>> >     CC replace-object.o\n>> > ...\n>> \n>> Could you, please, provide more details about your environment,\n>> i.e. the operating system and compiler?\n> \n> I'm also curious about which compiler, but I think it's a reasonable\n> complaint. We forward-declare the static function, use it, and then\n> later declare it inline. I didn't check to see what the standard says,\n> but it seems like a funny thing to do in general.\n\nThe link below seems to provide more details.  The way I see it,\ndeclarations and definitions should match, and the standard seems\nto support that.  Though, not all compilers (or not all versions)\ncomplain in this particular case.\n\nhttps://stackoverflow.com/a/62390378/22330192\n\n> It has been that way for a while; since 56eed3422c (remote: remove\n> the_repository->remote_state from static methods, 2021-11-17), I think.\n> \n> I don't really see any need to mark the wrapper as inline. It's one\n> basic function call (on top of an interface which requires a callback\n> anyway!), and I suspect many compilers would consider inlining anyway,\n> since it's a static function.\n> \n> Ditto for remotes_pushremote_get(), though it doesn't have a forward\n> declaration.\n> \n> -Peff\n"},{"id":"492442","messageId":"xmqqzfu5hgs6.fsf@gitster.g","threadId":"61281","inReplyTo":"20240407013843.GE1085004@coredump.intra.peff.net","subject":"Re: Warning message in remote.c when compiling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-07T05:40:25Z","receivedAt":"2024-04-07T05:40:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I don't really see any need to mark the wrapper as inline. It's one\n> basic function call (on top of an interface which requires a callback\n> anyway!), and I suspect many compilers would consider inlining anyway,\n> since it's a static function.\n>\n> Ditto for remotes_pushremote_get(), though it doesn't have a forward\n> declaration.\n\nYup.  I presonally feel that we should get rid of \"static inline\"\nunless they appear in header files.  The compilers should in general\nbe able to do good enough job finding what to inline than we can (1)\ninitially mark what to inline, and (2) update by dropping \"inline\"\nthat is no longer appropriate as the code evolves.\n\nThanks.\n"},{"id":"492467","messageId":"CAA1AqduFP-eXgVyiqJcJuThBQ8FD6ZfQbcoEUWU_++hNW8rfMQ@mail.gmail.com","threadId":"61281","inReplyTo":"xmqqzfu5hgs6.fsf@gitster.g","subject":"Re: Warning message in remote.c when compiling","fromName":"prpr 19xx","fromEmail":"prpr19xx@gmail.com","sentAt":"2024-04-07T16:47:27Z","receivedAt":"2024-04-07T16:47:39Z","isPatch":false,"sender":{"key":"prpr19xx@gmail.com","avatar":null},"body":"On Sun, 7 Apr 2024 at 06:40, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Jeff King <peff@peff.net> writes:\n>\n> > I don't really see any need to mark the wrapper as inline. It's one\n> > basic function call (on top of an interface which requires a callback\n> > anyway!), and I suspect many compilers would consider inlining anyway,\n> > since it's a static function.\n> >\n> > Ditto for remotes_pushremote_get(), though it doesn't have a forward\n> > declaration.\n>\n> Yup.  I presonally feel that we should get rid of \"static inline\"\n> unless they appear in header files.  The compilers should in general\n> be able to do good enough job finding what to inline than we can (1)\n> initially mark what to inline, and (2) update by dropping \"inline\"\n> that is no longer appropriate as the code evolves.\n\nThe compiler is an ancient gcc 4.2.0 cross-compiler for a\nmipsel-linux-uclibc environment.\nIt doesn't really matter though, as folk seem to agree the definition\nand declaration should match, which I think they should.\nI also agree that having inline probably makes no sense, as the\ncompiler can usually work this stuff out itself.\nSo I don't mind whether all the inlines get removed or they all stay,\nas long as they are all effectively consistent, which they are\ncurrently not, and the compiler righty (to my mind) complains.\n\nThanks.\n"}]}