llvm dev - Jul 2020 - Explicitly spelling out the lack of stability for the C++ API in the Developer Policy?

On Thu, Jul 23, 2020 at 8:29 AM Nicolai Hähnle via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi Varun,
>
> On Wed, Jul 22, 2020 at 2:17 AM Varun Gandhi via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > * Stability Guarantees: The C++ API is does not guarantee any
stability.
> Changes may be made without any notice about deprecation and alternate APIs
> for the same functionality may not be included. Downstream projects using
> the C++ API are expected to keep up with changes.
>
> I'm generally on board with this, certainly between LLVM releases, but
> I feel like it would be friendlier to use (potentially short-lived)
> deprecation as a tool for LLVM trunk.
>
> We maintain an out-of-tree compiler[0] and try to be good citizens by
> following LLVM trunk very closely. It is always frustrating when a
> very central part of LLVM (like the Builders, or Instructions) have a
> "flag-day" change, where our frontend must be changed in a way
where
> the new version doesn't work with LLVM trunk that is even a few days
> old, and the old version doesn't work with current LLVM trunk.
>
> In many, many cases it is almost zero effort for the person making the
> chance in LLVM to split it up into a sequence of logical changes:
>
> 1) Add the new API.
> 2) Use it in llvm-project.
> 3) Add LLVM_ATTRIBUTE_DEPRECATED to the old API.
> 4) Remove the old API.
>
> 1-3 could be in a single commit, but having a few weeks between them
> and point 4 helps _massively_.
>
I don't see this as a "almost zero effort", I see this as a
potentially
*heavy* effort actually.

I am also fairly wary of the side-effect of such expectation in that it
will:
- discourage refactoring / cleanup, leading to an overall more
cumbersome/convoluted  API surface and overall codebase.
- encourage to "work-around" the process by creating duplication of
features because designing around deprecation is "work".

So I am against any policy or encouragement to endorse this right now.

(I also agree with David that this would be a change in practice and as
such likely deserves its own RFC thread).

Best,

-- 
Mehdi


> It allows us to keep compiling against LLVM trunk in our CI, while one
> person goes and fixes up our use of the API (which we can detect
> automatically thanks to the warning or -Werror). It also makes it
> easier for us to bisect regressions across such API changes.
>
> With the alternative, where 1-4 are all in a single commit, our
> integration with LLVM trunk is blocked until somebody can fix it --
> which is usually as quick as 1 or 2 days, but during that time window
> we don't catch any _other_ regressions in LLVM trunk that might affect
> us.
>
> So please, let's make it a common rule to use this two-step,
> transactional approach to changes in APIs that are relatively
"core"
> (which mostly means llvm/IR, llvm/ADT, llvm/Support, perhaps with a
> side of llvm/Analysis). I am perfectly fine with this rule being
> broken occasionally, for changes where it would be exceedingly tricky
> to do them in a non-flag-day way. But in our experience, most of the
> changes that would actually affect an out-of-tree frontend aren't this
> tricky.
>
> Cheers,
> Nicolai
>
>
> > * Release stability: The C++ API does not make any stability
guarantees
> for the release branch.
> > * Testing: Patches to the C++ API are expected to come with tests just
> like any other patch.
> > * Including new things into the API: [TODO: I'm not sure what
should go
> here].
> > * Documentation: Changes to the C++ API are not expected to be
> documented in the release notes.
> > ---
> >
> > Clang does have a page with information about its own C++ API (
> https://clang.llvm.org/docs/Tooling.html) which is more informative, but
> I think it would useful to have this information on the Developer Policy
> page for the whole of LLVM.
> >
> > Does this addition sound reasonable?
> >
> > Varun
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> aber vergiss niemals, wie sie sein sollte.
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200723/c0cf6d57/attachment.html>

On Fri, Jul 24, 2020 at 12:14 AM Mehdi AMINI <joker.eph at gmail.com>
wrote:
> On Thu, Jul 23, 2020 at 8:29 AM Nicolai Hähnle via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>> Hi Varun,
>>
>> On Wed, Jul 22, 2020 at 2:17 AM Varun Gandhi via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>> > * Stability Guarantees: The C++ API is does not guarantee any
stability. Changes may be made without any notice about deprecation and
alternate APIs for the same functionality may not be included. Downstream
projects using the C++ API are expected to keep up with changes.
>>
>> I'm generally on board with this, certainly between LLVM releases,
but
>> I feel like it would be friendlier to use (potentially short-lived)
>> deprecation as a tool for LLVM trunk.
>>
>> We maintain an out-of-tree compiler[0] and try to be good citizens by
>> following LLVM trunk very closely. It is always frustrating when a
>> very central part of LLVM (like the Builders, or Instructions) have a
>> "flag-day" change, where our frontend must be changed in a
way where
>> the new version doesn't work with LLVM trunk that is even a few
days
>> old, and the old version doesn't work with current LLVM trunk.
>>
>> In many, many cases it is almost zero effort for the person making the
>> chance in LLVM to split it up into a sequence of logical changes:
>>
>> 1) Add the new API.
>> 2) Use it in llvm-project.
>> 3) Add LLVM_ATTRIBUTE_DEPRECATED to the old API.
>> 4) Remove the old API.
>>
>> 1-3 could be in a single commit, but having a few weeks between them
>> and point 4 helps _massively_.
>
>
> I don't see this as a "almost zero effort", I see this as a
potentially *heavy* effort actually.
What do you base this belief on?

> I am also fairly wary of the side-effect of such expectation in that it
will:
> - discourage refactoring / cleanup, leading to an overall more
cumbersome/convoluted  API surface and overall codebase.
> - encourage to "work-around" the process by creating duplication
of features because designing around deprecation is "work".
The single most discouraging thing about refactoring / cleanup in LLVM
is that there are very, very few reviewers willing to say "Yes".

Besides, I think you misunderstood: the point isn't to *forbid*
flag-day changes. The point is to encourage thinking about how to do
refactoring better. Sometimes a flag-day change is required, and
that's fine, but in the vast majority of cases it isn't.

We have seen this in practice this year with the Align changes and the
SVE changes, and it generally works well (git log -S
LLVM_ATTRIBUTE_DEPRECATED shows the related changes -- there aren't
many of them, but there aren't many changes with the potential of
breaking a frontend build in the first place).

I'm simply saying that we should document established practice that
exists in the LLVM community today.

Cheers,
Nicolai


>
> Best,
>
> --
> Mehdi
>
>
>>
>> It allows us to keep compiling against LLVM trunk in our CI, while one
>> person goes and fixes up our use of the API (which we can detect
>> automatically thanks to the warning or -Werror). It also makes it
>> easier for us to bisect regressions across such API changes.
>>
>> With the alternative, where 1-4 are all in a single commit, our
>> integration with LLVM trunk is blocked until somebody can fix it --
>> which is usually as quick as 1 or 2 days, but during that time window
>> we don't catch any _other_ regressions in LLVM trunk that might
affect
>> us.
>>
>> So please, let's make it a common rule to use this two-step,
>> transactional approach to changes in APIs that are relatively
"core"
>> (which mostly means llvm/IR, llvm/ADT, llvm/Support, perhaps with a
>> side of llvm/Analysis). I am perfectly fine with this rule being
>> broken occasionally, for changes where it would be exceedingly tricky
>> to do them in a non-flag-day way. But in our experience, most of the
>> changes that would actually affect an out-of-tree frontend aren't
this
>> tricky.
>>
>> Cheers,
>> Nicolai
>>
>>
>> > * Release stability: The C++ API does not make any stability
guarantees for the release branch.
>> > * Testing: Patches to the C++ API are expected to come with tests
just like any other patch.
>> > * Including new things into the API: [TODO: I'm not sure what
should go here].
>> > * Documentation: Changes to the C++ API are not expected to be
documented in the release notes.
>> > ---
>> >
>> > Clang does have a page with information about its own C++ API
(https://clang.llvm.org/docs/Tooling.html) which is more informative, but I
think it would useful to have this information on the Developer Policy page for
the whole of LLVM.
>> >
>> > Does this addition sound reasonable?
>> >
>> > Varun
>> > _______________________________________________
>> > LLVM Developers mailing list
>> > llvm-dev at lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>>
>> --
>> Lerne, wie die Welt wirklich ist,
>> aber vergiss niemals, wie sie sein sollte.
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.

On Fri, Jul 24, 2020 at 9:35 AM Nicolai Hähnle <nhaehnle at gmail.com>
wrote:
> On Fri, Jul 24, 2020 at 12:14 AM Mehdi AMINI <joker.eph at gmail.com>
wrote:
> > On Thu, Jul 23, 2020 at 8:29 AM Nicolai Hähnle via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >> Hi Varun,
> >>
> >> On Wed, Jul 22, 2020 at 2:17 AM Varun Gandhi via llvm-dev
> >> <llvm-dev at lists.llvm.org> wrote:
> >> > * Stability Guarantees: The C++ API is does not guarantee any
> stability. Changes may be made without any notice about deprecation and
> alternate APIs for the same functionality may not be included. Downstream
> projects using the C++ API are expected to keep up with changes.
> >>
> >> I'm generally on board with this, certainly between LLVM
releases, but
> >> I feel like it would be friendlier to use (potentially
short-lived)
> >> deprecation as a tool for LLVM trunk.
> >>
> >> We maintain an out-of-tree compiler[0] and try to be good citizens
by
> >> following LLVM trunk very closely. It is always frustrating when a
> >> very central part of LLVM (like the Builders, or Instructions)
have a
> >> "flag-day" change, where our frontend must be changed in
a way where
> >> the new version doesn't work with LLVM trunk that is even a
few days
> >> old, and the old version doesn't work with current LLVM trunk.
> >>
> >> In many, many cases it is almost zero effort for the person making
the
> >> chance in LLVM to split it up into a sequence of logical changes:
> >>
> >> 1) Add the new API.
> >> 2) Use it in llvm-project.
> >> 3) Add LLVM_ATTRIBUTE_DEPRECATED to the old API.
> >> 4) Remove the old API.
> >>
> >> 1-3 could be in a single commit, but having a few weeks between
them
> >> and point 4 helps _massively_.
> >
> >
> > I don't see this as a "almost zero effort", I see this
as a potentially
> *heavy* effort actually.
>
> What do you base this belief on?
>
The experience of refactoring some large components in LLVM, contrasted
with working on other codebases where I couldn't actually do this just like
in LLVM and so we just didn't do it because the change in cost/benefit.

>
>
> > I am also fairly wary of the side-effect of such expectation in that
it
> will:
> > - discourage refactoring / cleanup, leading to an overall more
> cumbersome/convoluted  API surface and overall codebase.
> > - encourage to "work-around" the process by creating
duplication of
> features because designing around deprecation is "work".
>
> The single most discouraging thing about refactoring / cleanup in LLVM
> is that there are very, very few reviewers willing to say "Yes".
>
By increasing the amount of churn in the codebase and the number of patches
for a refactoring and the mental effort to assess what can break and what
can/can't be made

>
> Besides, I think you misunderstood: the point isn't to *forbid*
> flag-day changes. The point is to encourage thinking about how to do
> refactoring better. Sometimes a flag-day change is required, and
> that's fine, but in the vast majority of cases it isn't.
>
No I perfectly understood, I'm still not in favor of codifying an
encouragement in this direction: I'm not eager to have reviewers ask me to
change my patch to follow the scheme you describe for stability purposes.

>
> We have seen this in practice this year with the Align changes and the
> SVE changes, and it generally works well (git log -S
> LLVM_ATTRIBUTE_DEPRECATED shows the related changes -- there aren't
> many of them, but there aren't many changes with the potential of
> breaking a frontend build in the first place).
>
> I'm simply saying that we should document established practice that
> exists in the LLVM community today.

If an author *and* a reviewers agreed at the start to take this route
because it is more convenient for landing the changes: this is perfectly
fine.
I would do it if the motivation was to land the change more easily (easier
to craft, easier to review, etc.), but this isn't the same thing as
"providing stability for a fork of LLVM" (I don't believe this is
just
"documenting what is an established practice today") .

-- 
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200724/d6a89ab0/attachment.html>