[llvm-dev] "[NFC]" Abuse

Nikita Popov via llvm-dev llvm-dev at lists.llvm.org
Fri Jun 18 04:13:47 PDT 2021

On Fri, Jun 18, 2021 at 1:07 PM Luke Drummond via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> On Thu Jun 17, 2021 at 7:15 PM BST, Mehdi AMINI wrote:
> > On Thu, Jun 17, 2021 at 10:15 AM David Blaikie via llvm-dev <
> > llvm-dev at lists.llvm.org> wrote:
> >
> > > Got links to the reviews? Hard to discuss in the abstract.
> > >
> > > But generally, if it is intended to change observable behavior of the
> > > program (if you have to modify a lit test, that's not NFC, if one could
> > > craft a test (that doesn't invoke UB, though that shouldn't be possible
> > > through the command line interface, etc) that would need to be modified
> > > when the patch is committed - then it's not NFC).
> > >
> >
> > That's my litmus test: I see NFC is an indication that no test changes
> and
> > none are expected to be provided. The functional behavior of the
> compiler is
> > unchanged. I use NFC on API changes and refactoring when it fits this
> > description.
> I think this is a bit liberal as it ignores API users - unless I'm
> misunderstanding your statement about what you mean by "API changes".

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.

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210618/9d7aa5cb/attachment.html>

More information about the llvm-dev mailing list