Re: [PATCH v3 3/9] ll-merge: make callers responsible for showing warnings

>
> On Thu, Dec 30 2021, Elijah Newren via GitGitGadget wrote:
>
> > Note that my methodology included first modifying ll_merge() to return
> > a struct, so that the compiler would catch all the callers for me and
> > ensure I had modified all of them.  After modifying all of them, I then
> > changed the struct to an enum.
> > [...]
> > -int ll_merge(mmbuffer_t *result_buf,
> > +enum ll_merge_result ll_merge(mmbuffer_t *result_buf,
> >            const char *path,
> >            mmfile_t *ancestor, const char *ancestor_label,
> >            mmfile_t *ours, const char *our_label,
> > diff --git a/ll-merge.h b/ll-merge.h
> > index aceb1b24132..e4a20e81a3a 100644
> > --- a/ll-merge.h
> > +++ b/ll-merge.h
> > @@ -82,13 +82,20 @@ struct ll_merge_options {
> >       long xdl_opts;
> >  };
> >
> > +enum ll_merge_result {
> > +     LL_MERGE_ERROR = -1,
> > +     LL_MERGE_OK = 0,
> > +     LL_MERGE_CONFLICT,
> > +     LL_MERGE_BINARY_CONFLICT,
> > +};
> > +
>
> Isn't the other side of the enum checking missing in many cases?
>
> E.g. ll_ext_merge() returns "enum ll_merge_result" now, and does:
>
>         status = run_command_v_opt(args, RUN_USING_SHELL);
>         ret = (status > 0) ? LL_MERGE_CONFLICT : status;
>
> And grepping at the tip of this series shows:
>
>     $ git grep LL_MERGE_OK
>     ll-merge.c:             ret = LL_MERGE_OK;
>     ll-merge.c:                     ret = LL_MERGE_OK;
>     ll-merge.c:                     ret = LL_MERGE_OK;
>     ll-merge.h:     LL_MERGE_OK = 0,
>
> Similar for LL_MERGE_CONFLICT, the only one that's used outside of the
> file itself and its header is LL_MERGE_BINARY_CONFLICT.
>
> I.e. shouldn't these codepaths:
>
>     git grep -w ll_merge
>
> Be doing a switch() on that new enum? E.g. we lose the type in
> three_way_merge() in apply.c, it seems to me that that function should
> switch over this new enum, and return the "int" that the callers of
> three_way_merge() care about (i.e. just <0, 0, 1, not this enum's -1, 0,
> 1, 2.