<div dir="ltr"><div dir="ltr">On Sun, Jun 20, 2021 at 4:41 PM Joerg Sonnenberger via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Jun 17, 2021 at 03:13:16PM -0400, John McCall via llvm-dev wrote:<br>
> Yeah, I expect NFC changes to be purely internal implementation<br>
> details.  Changing the public interface (even to add a feature)<br>
> isn’t NFC; neither is anything that can affect output.<br>
<br>
I would consider an even more restricted subset: NFC changes would be<br>
trivial proven to be just that. That means non-trivial algorithmic<br>
changes are certainly not NFC. My point is that the tag should be<br>
applied with care only and if there is any doubt, it should not be used<br>
at all.<br></blockquote><div><br>I disagree pretty strongly there - intent is important and all any patch comment can describe.<br><br>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).<br><br>- Dave </div></div></div>