[llvm-dev] "[NFC]" Abuse

[Philip Reames via llvm-dev]

On 6/21/21 10:27 AM, David Blaikie via llvm-dev wrote:
> On Sun, Jun 20, 2021 at 4:41 PM Joerg Sonnenberger via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto: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).

+1

Philip

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