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

[Eli Bendersky]

On Thu, Feb 21, 2013 at 9:58 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Feb 21, 2013 at 4:50 PM, Eli Bendersky <eliben at google.com> wrote:
>>
>> Author: eliben
>> Date: Thu Feb 21 18:50:48 2013
>> New Revision: 175847
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=175847&view=rev
>> Log:
>> Code cleanup: pass Offset by pointer to parseInstruction to more
>> explicitly
>> convey that it's a INOUT argument.
>
>
> This isn't really a convention we have in the LLVM code base & it's not one
> I'd personally prefer we adopt (though my opinion isn't the only one, of
> course). Mostly because I always wonder whether such a parameter can be null
> (from both the caller & the callee) and what it means if it is.
>
> Is there some other way we could tweak such an API to communicate this?
> Though honestly I've seen enough parse/cursor APIs that this doesn't seem
> /that/ unusual/confusing. Was there a particularly confusing call site you
> saw?
>

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.

Eli