[llvm-dev] "[NFC]" Abuse

[Mehdi AMINI via llvm-dev]

On Fri, Jun 18, 2021 at 4:07 AM Luke Drummond <luke.drummond at codeplay.com>
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".


Yes I am ignoring API users, I am on the same line as Nikita here.
We don’t have stable APIs (other than the C one), so I for example I may
change an API that was taking 3 bools to take now a struct parameter
wrapping the 3 bools instead. I’ll tag it NFC.

On the same line as my comment above, if I review a patch without any
tests, I will ask if it NFC.

Best,

—
Mehdi



>
> > We could improve the doc maybe?
> I'm happy to do this legwork but will hold off until something of a
> consensus
> emerges.
>
> All the Best
>
> Luke
> --
> Codeplay Software Ltd.
> Company registered in England and Wales, number: 04567874
> Registered office: Regent House, 316 Beulah Hill, London, SE19 3HF
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210618/b9c13afd/attachment.html>