Re: Fix uninitialized variables when Source_file::get_counts returns early due to !contains (pos_str0) (issue 4940047)

[Ian Hulin]

Hi  Graham, Carl,

On Tue 23 Aug 2011 19:34:37 BST, Carl Sorensen wrote:
> On 8/23/11 12:21 PM, "address@hidden" <address@hidden> wrote:
> 
>> LGTM
>>
>> Maybe we should have some GOP rules for C++ about this?
>> "Only have multiple exit points from routines if you absolutely have to.
> 
> Multiple exit points is a standard idiom of the LilyPond code.  Basically,
> the idiom is "If this routine doesn't apply for some reason, leave now",
> instead of "If this routine doesn't apply, skip to the end" or "If this
> routine applies, do the body".
> 
> As imbedded as this idiom is in our code, I don't think it would be wise to
> change it.
> 
... a sort of "legal precedent" argument.  Just because it's precedent, 
doesn't make it right.
"leave now" is an optimisation: it means you're assuming everything is 
safe and you know for certain that the code you have executed up to the 
point of exit *including any routines it has called* has done no harm.
Branching to a final single exit point allows you to clean up any 
side-effects/damage/half-completed stuff your code changed thus far.
Current practice didn't work in this case.  

I'm not asking for a grand re-write on this, but for single-exit to be 
the preferred style for new code and patches where this would not 
provoke changes on a GCR (Grand Code Re-write) scale.

>> Make sure any output parameters are declared and initialized at the top
>> of a routine so that however a routine exits, they are left in a defined
>> state"
> 
> IIRC, we used to have a statement that said to declare variables as close as
> possible to where they are used.  I generally agree with that.
> 
> However, I think a simple statement that says "variables should be
> initialized when they are declared" would be clearly welcome.  And fixes to
> the code to ensure this would also be welcome.
> 
OK, maybe take the point above as a stylistic preference.  

Declaring variables nearest to where they are used + insisting they are 
initialised may work, providing we spell it out the if they are 
declared as parameters to the routine, that's the first place they're 
used, so if they're output parameters, they need to be initialized 
either at the top of the routine, or in the parameter declaration.

Tightening up this statement may do the trick.  We need to feed back 
things we pick up on like this into the development/reviewing process.

Cheers,
Ian