Re: [PATCH 1/7] merge-ort: extract handling of priv member into reusable function

> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +static void move_opt_priv_to_result_priv(struct merge_options *opt,
> > +					 struct merge_result *result)
> > +{
> > +	/*
> > +	 * opt->priv and result->priv are a bit weird.  opt->priv contains
> > +	 * information that we can re-use in subsequent merge operations to
> > +	 * enable our cached renames optimization.  The best way to provide
> > +	 * that to subsequent merges is putting it in result->priv.
> > +	 * However, putting it directly there would mean retrofitting lots
> > +	 * of functions in this file to also take a merge_result pointer,
> > +	 * which is ugly and annoying.  So, we just make sure at the end of
> > +	 * the merge (the outer merge if there are internal recursive ones)
> > +	 * to move it.
> > +	 */
> > +	assert(opt->priv && !result->priv);
> > +	if (!opt->priv->call_depth) {
> > +		result->priv = opt->priv;
> > +		result->_properly_initialized = RESULT_INITIALIZED;
> > +		opt->priv = NULL;
> > +	}
> > +}
> > +
> >  /*
> >   * Originally from merge_trees_internal(); heavily adapted, though.
> >   */
> > @@ -5060,11 +5082,7 @@ static void merge_ort_nonrecursive_internal(struct merge_options *opt,
> >  		/* existence of conflicted entries implies unclean */
> >  		result->clean &= strmap_empty(&opt->priv->conflicted);
> >  	}
> > -	if (!opt->priv->call_depth) {
> > -		result->priv = opt->priv;
> > -		result->_properly_initialized = RESULT_INITIALIZED;
> > -		opt->priv = NULL;
> > -	}
> > +	move_opt_priv_to_result_priv(opt, result);
> >  }
>
> I have a feeling that making it the caller's responsibility to check
> "are we doing the outermost merge?"  and not the callee's problem
> would result in a better code organization.  If we write
>
> 	if (!opt->priv->call_depth)
> 		move_opt_priv_to_result_priv(opt, result);
>
> then for this call site, it is still crystal clear that this will
> happen only at the outermost level.  The new caller you add in the
> next step would also be simpler to reason about.