Re: [PATCH 1/2] pkt-line: fix declaration of `set_packet_header()`
- Date: Tue, 14 May 2019 10:44:39 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH 1/2] pkt-line: fix declaration of `set_packet_header()`
On Tue, May 14, 2019 at 10:43:06AM -0400, Jeff King wrote:
> On Tue, May 14, 2019 at 02:57:01PM +0200, Johannes Schindelin wrote:
> > > But the parameter treated as a constant without getting modified
> > > during the invocation of the function is an implementation detail of
> > > the function; there is no point exposing that implementation detail
> > > to its callers. It does not even help the compilers handling the
> > > caller's compilation unit---the parameter is passed by value, so the
> > > caller knows that the callee would not modify it without "const"
> > > there.
> > >
> > > Does the language even allow flagging "const int in the definition,
> > > int in the declaration" as a warning-worthy discrepancy?
> > Apparently it does, as MS Visual C does issue a warning (and with `/Wall`,
> > it fails).
> > In any case, I don't think that it makes sense to have a function
> > declaration whose signature differs from the definition's.
> I actually agree with Junio's point that in an ideal world the
> declaration should omit details that are not relevant to the caller. But
> clearly we do not live in that world, and this is a small enough point
> that we should fix it in one direction or the other.
> I do have a slight preference for going the _other_ way. There is no
> need to mark the parameter as const in the definition. It is passed by
> value, so nobody except the function body cares either way. And we have
> many function bodies where value-passed parameters (or local variables!)
> are not marked as const, even though they are only assigned to once.
> I don't think that annotation is telling much to any reader of the code,
> nor to a decent optimizing compiler.
To be clear, I can live with your patch as-is, and I don't think this
one site overly matters. Mostly I do not want to see "let's declare
pass-by-value parameters as const" picked up as a general concept, and
it seems easiest to try to squash the first instance of it. :)