From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] diff-lib: add idx/tree sanity check to oneway_diff
Date: Tue, 28 Jul 2026 13:12:49 -0400 [thread overview]
Message-ID: <20260728171249.GA643101@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqo6frdqy5.fsf@gitster.g>
On Tue, Jul 28, 2026 at 09:22:42AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> >> We could help humans that the BUG is not expected to fire and only
> >> to help static analysis by a crafted message, perhaps?
> >>
> >> if (!idx && !tree)
> >> BUG("Hey, Coverity, this does not happen");
> >
> > If we are helping humans we can probably afford to be a little more
> > eloquent. ;)
>
> I do not mind eloquence but does the comment clearly say this is
> primarily for unconfusing static analyzers? My first reaction to
> the message was "OK, you explained very well why this condition
> would never happen, but then why do you need to check and BUG() on
> it???"
I guess by the time I did all of the digging and thinking, my thought
was that it _wasn't_ primarily for static analyzers, but to capture the
output of that research. But then yeah, we don't really need the actual
BUG(), but without it, it feels strange to even have the comment at all.
I think that's why I waffled on sending the patch at all.
> But I guess the point is a future modification may invalidate this,
> in which case I agree with the comment. If it is hard for static
> analysers to get it right, it probably is equally difficult to grok
> for AI agents many people seem to be using to draft their changes
> these days ;-).
Yeah, I guess it may help them, too. :)
-Peff
next prev parent reply other threads:[~2026-07-28 17:12 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-15 6:05 [PATCH] diff: ignore unmerged paths outside prefix with --relative --cached Jeff King
2026-07-15 15:17 ` Junio C Hamano
2026-07-26 8:45 ` [PATCH 0/2] diff-lib relative-path cleanups Jeff King
2026-07-26 8:46 ` [PATCH 1/2] diff-lib: drop stale comment about advancing o->pos Jeff King
2026-07-26 8:47 ` [PATCH 2/2] diff-lib: skip paths outside prefix in oneway_diff() Jeff King
2026-07-27 9:39 ` Jeff King
2026-07-28 0:43 ` Junio C Hamano
2026-07-28 15:14 ` [PATCH] diff-lib: add idx/tree sanity check to oneway_diff Jeff King
2026-07-28 16:22 ` Junio C Hamano
2026-07-28 17:12 ` Jeff King [this message]
2026-07-26 20:02 ` [PATCH 0/2] diff-lib relative-path cleanups Junio C Hamano
2026-07-24 21:30 ` [PATCH] diff: ignore unmerged paths outside prefix with --relative --cached Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260728171249.GA643101@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox