[llvm] r175847 - Code cleanup: pass Offset by pointer to parseInstruction to more explicitly

[David Blaikie]

On Fri, Feb 22, 2013 at 10:12 AM, Eli Bendersky <eliben at google.com> wrote:

> >> This is mainly bringing the code in line with the other DataExtractor
> >> use cases. DataExtractor's getXXX methods take a pointer to offset for
> >> reading it and updating it. My initial code used a reference, and I
> >> wanted it to be more consistent now. As for the coding convention, I
> >> know we don't have an explicit guideline for this, and yet I
> >> personally prefer not to use references in such cases, because looking
> >> at:
> >>
> >> doFoo(param)
> >>
> >> It's really hard to immediately see that param can be modified by
> >> doFoo. But looking at:
> >>
> >> doFoo(&param)
> >>
> >> It's easier.
> >>
> >> But this is a minor argument. The main one is the consistency.
> >
> >
> > Oh, OK - I'll totally buy an argument from consistency if the code you're
> > being consistent with is nearby/obviously in contrast (ie: the two uses
> sit
> > side by side & would confuse users - which I guess is the case here),
> but if
> > we're both in agreement (a preference for reference in/out params) why
> not
> > just fix DataExtractor instead (& that way it'll be consistent with the
> rest
> > of the codebase)? It's not a huge class & the transformation would be
> fairly
> > easy I would think...
> >
>
> To clarify, I'm in favor of keeping the existing "INOUT by pointer"
> convention in DataExtractor, and by extension in nearby code.


Yep, guess I rushed to read what I was hoping to read & not what you
actually wrote, sorry.

So just rephrase the tone of my previous reply & it mostly still applies &
boils down to:

I'd rather creep consistency in the direction of the preferred style.
The style I prefer is not the style you prefer.
The style you prefer isn't really pervasive in the codebase so I'd rather
push the wider style into the smaller areas than the smaller style into
wider areas (& of course my preference here isn't entirely from consistency
- it's also because it's what I like)
I realize Google's coding conventions use this approach, but I believe
they're somewhat the exception - I've not seen this as a common style in
other C++ codebases.

and: pointers possible nullness makes me sad.

I know it's a tradeoff but I sort of wish there was an alternative that is
both legible at call sites & doesn't reduce type safety by adding an
'invalid' state (null) to every such parameter. A simple tagging template
type could be used, but is probably overkill. I'd use it if the only
acceptable alternative was pointer passing for mutable parameters.

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130222/b6e4cfc3/attachment.html>