[llvm-dev] "[NFC]" Abuse

[Nikita Popov via llvm-dev]

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
> LLVM
> > > 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.

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