On Fri, Nov 09, 2018 at 01:58:01PM -0800, Stefan Beller wrote:
> On Thu, Nov 8, 2018 at 9:18 PM Junio C Hamano <gits...@pobox.com> wrote:
> > Are they wrong changes (e.g. a carelessly written read_cache() to
> > read_index(&the_index) conversion may munge the implementation of
> > read_cache(...) { return read_index(&the_index, ...); } and make
> > inifinite recursion)? Or are they "correct but not immediately
> > necessary" (e.g. because calling read_cache() does not hurt until
> > that function gets removed, so rewriting the callers to call
> > read_index() with &the_index may be correct but not immediately
> > necessary)?
>
> the latter. I assume correctness (of the semantic patch) to be a given,
I'm afraid we can't assume that. As far as correctness is concerned,
I think semantic patches are not different from any other code we
write, i.e. they are potentially buggy. Perhaps even more so than the
"usual" Git code, because we have long experience writing C and shell
code (with all their quirks and error-proneness), but none of us is
really an expert in writing semantic patches.
Cases in point:
- 6afedba8c9 (object_id.cocci: match only expressions of type
'struct object_id', 2018-10-15)
- 279ffad17d (coccinelle: avoid wrong transformation suggestions
from commit.cocci, 2018-04-30)
- cd9a4b6d93 (cocci: use format keyword instead of a literal string,
2018-01-19); though this one is probably a bug in Coccinelle
itself
- c2bb0c1d1e (cocci: avoid self-references in object_id
transformations, 2016-11-01)
