[PATCH] D112056: [clang-format] git-clang-format throws an assertion when removing files as part of the commit

Tue Oct 19 13:38:10 PDT 2021

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

In D112056#3073569 <https://reviews.llvm.org/D112056#3073569>, @MyDeveloperDay wrote:

> In D112056#3073142 <https://reviews.llvm.org/D112056#3073142>, @lhames wrote:
>
>> I think that this should fix the issue I saw, but `clang-format` will still crash when passed a `-lines=0:X` option. Would it make sense to either error out in that tool, or teach the tool to ignore `-lines=0:0` options if they're guaranteed to be no-ops for formatting?
>
> I think my concern with simply ignoring them is that clang-format could be given multiple -lines calls, what should it do? silently ignore the -lines:0:0 or error out on all the other changes, I'd be a little concerned because we then enter into other issues like what happens if someone give -lines:-1:3 should it error out or adjust itself to -lines:1:2
>
> ultimately I think silently ignoring could be more dangerous rather than  asserting or trying to correct it, this way it ensures that people fix up how they are calling it (as in this case), having said that I'm not a massive fan of asserting either.
>
> I think maybe we should fix this as a regression and take sometime to think if we want a more guarded solution separately.

I would be in favor of returning an actual error instead of an assertion (what happens in release mode?). It is common to many programs to just print the error message if one argument was wrong and do not act on the others.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112056/new/

https://reviews.llvm.org/D112056