[llvm-dev] "[NFC]" Abuse

[Adam HARRIES via llvm-dev]

> We simply cannot take account of downstream users when crafting these
comments

I wasn't arguing that the upstream tagging system should be relied upon by
downstream consumers, and therefore catered to by upstream maintainers,
apologies for miscommunicating this. My point was more that having such
tags can be useful guidance for downstream consumers, and that having
further granularity there benefits downstream consumers as well as the core
llvm contributors & reviewers. This seems to have also been a core part of
Luke's original email, and something worthy of discussion.

> if a downstream developer had a private patch using that local variable

I would suggest that this is outside of the scope of this discussion, as it
is related to internal implementation details where changes should be
noticed while merging, rather than API or functional change. Downstream
consumers must of course be responsible for ensuring that any internal
details that they privately patch are kept up to date regardless of whether
changes are non-functional or not.

On Thu, 24 Jun 2021 at 13:23, James Henderson <jh7370.2008 at my.bristol.ac.uk>
wrote:

> When I say "user" in this context, I mean someone or something who runs an
> LLVM executable (an LLVM executable being any that are part of the official
> LLVM project). Putting it another way, I would label a change as NFC if and
> only if all such executables given any input produced the exact same output
> as they did before the change.
>
> We simply cannot take account of downstream users when crafting these
> comments, if we're going to have any form of NFC tag at all. Even a
> supposedly trivial change has the potential to cause downstream merge
> conflicts or unexpected behaviour changes, if it just so happens to be in
> the wrong place. Imagine a function with a local variable, whose meaning is
> completely changed, but without impacting the end output of the function -
> if a downstream developer had a private patch using that local variable,
> their code might break, but it would seem to the upstream developer that
> this is an NFC patch, even under the rule of "API changes should not be
> marked as NFC". Worse, the code may not fail at build time, as the types
> could be compatible in some way that just causes a change in behaviour for
> that user.
>
> James
>
>
> On Thu, 24 Jun 2021 at 11:53, Adam HARRIES <aharries at upmem.com> wrote:
>
>> > if a user of an LLVM tool (clang/lld/llvm-objdump/...) can see a
>> difference in behaviour because of the change
>>
>> As a fairly new member of the llvm community, I'm interested in what
>> people mean when they say "users".  For me, there are two distinct groups
>> that I could consider "users" of the llvm-project: Day-to-day developers
>> who use the compiled tools to build separate software, and compiler/tool
>> developers who integrate or build upon llvm to build a piece of software.
>> From reading through this thread it feels like these groups are sometimes
>> being conflated, which leads to disagreements regarding whether or not a
>> change will affect llvm "users". If we only consider group one to be
>> "users", then clearly any change that does not alter the observable
>> behaviours of the binaries can be considered a NFC. However, if we consider
>> group two to be users as well, then the impacts of any change may be a
>> functional change - and I would personally argue that this is especially
>> true for API changes.
>>
>> As a member of group two, I have found that (as I think Luke has) changes
>> which are considered non-functional can sometimes have surprising effects
>> on our downstream components. While it is in no way the llvm project's
>> responsibility to avoid breaking our code, any assistance in the form of
>> comments or more precise tags goes a great way towards helping us to find
>> and diagnose places where our assumptions are inconsistent with the state
>> of the software.
>>
>> Cheers,
>>
>> --
>> *Adam Brouwers-Harries*
>> Compiler Engineer
>> aharries at upmem.com
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210624/39e719d9/attachment.html>