<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jun 18, 2021 at 1:07 PM Luke Drummond via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><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 7:15 PM BST, Mehdi AMINI wrote:<br>
> On Thu, Jun 17, 2021 at 10:15 AM David Blaikie via llvm-dev <<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
> > Got links to the reviews? Hard to discuss in the abstract.<br>
> ><br>
> > But generally, if it is intended to change observable behavior of the LLVM<br>
> > program (if you have to modify a lit test, that's not NFC, if one could<br>
> > craft a test (that doesn't invoke UB, though that shouldn't be possible<br>
> > through the command line interface, etc) that would need to be modified<br>
> > when the patch is committed - then it's not NFC).<br>
> ><br>
><br>
> That's my litmus test: I see NFC is an indication that no test changes and<br>
> none are expected to be provided. The functional behavior of the compiler is<br>
> unchanged. I use NFC on API changes and refactoring when it fits this<br>
> description.<br>
<br>
I think this is a bit liberal as it ignores API users - unless I'm<br>
misunderstanding your statement about what you mean by "API changes".<br></blockquote></div><div class="gmail_quote"><br></div><div class="gmail_quote">For what it's worth, my understanding was that NFC can also include API changes, as long as they are, well, non-functional. In LLVM pretty much everything is part of the public API, so non-functional refactoring will often touch the API.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote">To give an example, moving some helper from Transform/Utils to Analysis would be a classical NFC change to me, even if it obviously affects the public API. Another classical NFC change is to rename a function in line with the new naming guidelines. The NFC-ness of that change should not depend on whether that function happens to be exported or not.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote">Regards,<br></div><div class="gmail_quote">Nikita<br></div></div>