[llvm-dev] "[NFC]" Abuse

[David Blaikie via llvm-dev]

On Sun, Jun 20, 2021 at 4:41 PM Joerg Sonnenberger via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> On Thu, Jun 17, 2021 at 03:13:16PM -0400, John McCall via llvm-dev wrote:
> > Yeah, I expect NFC changes to be purely internal implementation
> > details.  Changing the public interface (even to add a feature)
> > isn’t NFC; neither is anything that can affect output.
>
> I would consider an even more restricted subset: NFC changes would be
> trivial proven to be just that. That means non-trivial algorithmic
> changes are certainly not NFC. My point is that the tag should be
> applied with care only and if there is any doubt, it should not be used
> at all.
>

I disagree pretty strongly there - intent is important and all any patch
comment can describe.

If that change was adopted, I'd want another flag for the "I intend this
not to change the observable behavior, but it's not trivially proven" -
especially when reviewing a patch that is missing test coverage. If someone
hasn't made a claim that this doesn't change behavior, then I expect that
the inverse is true (it does change behavior) and a test should be present.
And once we have that flag, I'm not sure the difference between "I intend
this to not change behavior, but it passes some threshold of triviality
that makes it not 'guaranteed'" and things below that threshold is
sufficiently valuable to encode in the commit message (& haggle over which
things are over or under that threshold).

- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210621/2a78047d/attachment.html>