Explicitly spelling out the lack of stability for the C++ API in the Developer Policy?

The Developer Policy document (https://llvm.org/docs/DeveloperPolicy.html) contains a Section “C API Changes”. There is no corresponding section for C++ API Changes. LLVM is somewhat different from most libraries in that the main language is C++ but the C++ API is not guaranteed to be stable in any shape or form from what I understand.

I think it would be useful to have a “C++ API Changes” section to Developer Policy spelling this out. Copying the style of the C API Changes section, it could look something like:

This sounds reasonable to me, I am generally +1 on documenting things that are well known but not written down.

Thanks Varun!

-Chris

Something that would be good to get clarity on:

The RISC-V backend recently had a bugfix patch that got backported to the 10.0.1 branch. The original patch introduced a new virtual method in TargetLowering.h, and the backported patch 1 was rewritten to avoid changing the ABI of libLLVM.so.

This feels like some kind of policy decision about the C++ ABI beyond “it’s entirely unstable” - though I understand we may not want to commit to doing this for every backported bug fix.

It would be good to understand where this line is, if we’re going to clarify the documentation.

Sam

I believe the policy is to provide ABI stability across the patch releases so that any downstream distribution that ships LLVM X can also ship LLVM X.0.Y and pick up bug fixes. There is no stability guarantee even at the source level between LLVM X and LLVM X+1, though where possible it's considered polite to deprecate things and provide a migration path. In practice, quite a lot of LLVM now has backwards API (source) compatibility across versions but still retains the option of breaking this in any release.

David

I believe the policy is to provide ABI stability across the patch releases so that any downstream distribution that ships LLVM X can also ship LLVM X.0.Y and pick up bug fixes. There is no stability guarantee even at the source level between LLVM X and LLVM X+1, though where possible it's considered polite to deprecate things and provide a migration path. In practice, quite a lot of LLVM now has backwards API (source) compatibility across versions but still retains the option of breaking this in any release.

David

This is also my understanding. We maintain ABI stability across patch releases.

-Hal

Hi Varun,

* 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_.

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

I agree that deprecation is much more friendly but the last time I proposed it there was a lot of push-back. Given this thread is about documenting the existing policy, rather than changing the policy, I I'd recommend starting a new RFC thread about deprecation.

David

Hi Nicolai,

I think there are two distinct, somewhat loosely connected questions here:

  1. Spelling out the (existing, implicit) policy explicitly in the documentation.
  2. What the actual policy is and potentially changing it to make things easier for downstream.

I started this thread with the intention of tackling 1 and not 2. I think the conversation around 2 is much harder as it will require consensus-building between a large number of people, especially given that what you are suggesting seems to be somewhat of a departure from current practices. So I think it would better to have a separate thread on changing the policy.

If there is agreement on changing the policy, we can certainly go back and update the policy document to reflect the changes.

Varun

I think there are two distinct, somewhat loosely connected questions here:

1. Spelling out the (existing, implicit) policy explicitly in the documentation.
2. What the actual policy is and potentially changing it to make things easier for downstream.

I started this thread with the intention of tackling 1 and not 2. I think the conversation around 2 is much harder as it will require consensus-building between a large number of people, especially given that what you are suggesting seems to be somewhat of a departure from current practices. So I think it would better to have a separate thread on changing the policy.

The existing, implicit policy is non-existent though. Folks have been
using the deprecation approach that I described in my earlier email
already, it just hasn't been applied consistently.

So to pretend that the existing policy is that there is no attention
paid to deprecation would not be correct. If the goal is to restrict
this discussion to your point 1, that's fair, but then you do need to
mention that care for deprecation is sometimes being done, just not
consistently. I'd be happy to start a separate thread afterwards to
suggest amending that to making it tighter.

Cheers,
Nicolai

> I think there are two distinct, somewhat loosely connected questions here:
>
> 1. Spelling out the (existing, implicit) policy explicitly in the documentation.
> 2. What the actual policy is and potentially changing it to make things easier for downstream.
>
> I started this thread with the intention of tackling 1 and not 2. I think the conversation around 2 is much harder as it will require consensus-building between a large number of people, especially given that what you are suggesting seems to be somewhat of a departure from current practices. So I think it would better to have a separate thread on changing the policy.

The existing, implicit policy is non-existent though. Folks have been
using the deprecation approach that I described in my earlier email
already, it just hasn't been applied consistently.

So to pretend that the existing policy is that there is no attention
paid to deprecation would not be correct.

I think the policy is/would accurately be described as "there is no
guarantee of C++ API stability, except across patch releases" is an
accurate statement of the contract between LLVM and things outside
LLVM. That sometimes folks make changes that make such migration less
harsh doesn't negate that statement/policy - LLVM doesn't guarantee
it, and downstream consumers can't (currently) depend on it.

The policy is about the contract between LLVM and things outside LLVM
- it doesn't mean LLVM contributions can't be multi-stage. Usually
they're done that way when the LLVM migration itself is non-atomic for
any reason (to reduce the volatility of one patch/make it an
incremental migration) - but I could believe it if folks have/would
continue to sometimes use such approaches even for purely external
migration (often for their own out-of-tree needs).

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,

Hi Varun,

> * 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

Hi Varun,

  • 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”) .

I can see both sides of this. Deprecation has a lot of value that Nicolai points out and some people do use it. I don’t think it is possible to get to perfect “deprecation cycles” and even outside perfection an overly-broad application of this would just be expensive. Some things (e.g. core IR) simply matter more than others.

Perhaps a way too slice this is to phrase it along the lines of:

  1. There is no guarantee.
  2. That said, for core IR changes it is nice to stage them for XYZ reasons.
  3. If you do so, “this is the right way".

-Chris

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.

I can see both sides of this. Deprecation has a lot of value that Nicolai points out and some people do use it. I don’t think it is possible to get to perfect “deprecation cycles” and even outside perfection an overly-broad application of this would just be expensive. Some things (e.g. core IR) simply matter more than others.

Perhaps a way too slice this is to phrase it along the lines of:

1) There is no guarantee.
2) That said, for core IR changes it is nice to stage them for XYZ reasons.
3) If you do so, “this is the right way".

That sounds good to me.

Cheers,
Nicolai