git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] merge-ort: fix slightly overzealous assertion for rename-to-self

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 13, 2025, 16:55 UTC
Message-ID
<xmqqbju4q39j.fsf@gitster.g>
In-Reply-To
<CABPp-BF7iTvqKFvuOZ5wZ3vZADjCvB63iKR-4D5+WeTxnZGThg@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 17 quoted lines
> On Wed, Mar 12, 2025 at 4:18 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Elijah Newren <newren@gmail.com> writes:
>>
>> > But it might be worth mentioning that having side-effects in
>> > assertions is a huge no-no, and I understand that when folks have to
>> > debug one of those (I had to in a separate project years ago which was
>> > kind of nasty), that they sometimes jump to the conclusion that
>> > assertions are bad.
>>
>> Yes, assert() invites such mistakes.  Why not avoid them when there
>> are better alternatives like "if (condition) BUG()"?
>
> I mean, I just gave my reasons above which you snipped out.  But if
> you feel it is important for folks to move away from assert(), would
> you like to see someone create a better alternative to assert, such as
> BUG_ON(condition), so that they have reason to want to switch?

You said "BUG_ON()" is better than "if (condition) BUG()", but I do not see a reason why. It also shares the same problem with assert() if we make it honor NDEBUG.

Thanks.
Previous: Elijah NewrenNext: Elijah Newren
Message 8 of 11 in “merge-ort: fix a crash in process_renames for a file transitively renamed to itself”
  1. 0/2 merge-ort: fix a crash in process_renames for a file transitively renamed to itselfElijah Newren via GitGitGadget, Mar 6, 2025
  2. 1/2 t6423: add a testcase causing a failed assertion in process_renamesDmitry Goncharov via GitGitGadget, Mar 6, 2025
  3. 2/2 merge-ort: fix slightly overzealous assertion for rename-to-selfElijah Newren via GitGitGadget, Mar 6, 2025
  4. Taylor BlauMar 12, 2025
  5. Elijah NewrenMar 12, 2025
  6. Junio C HamanoMar 12, 2025
  7. Elijah NewrenMar 13, 2025
  8. Junio C HamanoMar 13, 2025
  9. Elijah NewrenMar 13, 2025
  10. Junio C HamanoMar 13, 2025
  11. Elijah NewrenMar 14, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.