[cfe-dev] API Removal Notice (4 weeks): getStartLoc, getLocStart, getLocEnd

Mon Aug 20 15:16:02 PDT 2018

On Tue, 14 Aug 2018 at 01:44, Hans Wennborg via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> I'd be fine with that too, and if we want to deprecate and remove an
> API across a release, this would really be the way to do it.
>
> Richard, do you have any thoughts on this?
>

I'm strongly opposed to setting a precedent that we will give out-of-tree
consumers of the Clang AST such a huge amount of time to respond to API
changes -- given that we recently branched Clang 7, we're talking about a
*year* of delay if we don't remove these until Clang 9 is branched. The
four week delay we've been talking about for this patch is already
extraordinary and goes far beyond what I'd expect from someone contributing
an API cleanup. We make major breaking changes to our C++ API all the time,
and we explicitly have *no* stability guarantees for it. And that's a good
thing, both for our own code health and for our community, as it encourages
people to contribute their changes upstream (or to use the
designed-for-stability C API when applicable).

I consider this particular change to be an experiment; I would encourage
anyone who derived material benefit from the delay between deprecation and
removal in this case to let us know.

> On Mon, Aug 13, 2018 at 11:40 AM, David Chisnall
> <David.Chisnall at cl.cam.ac.uk> wrote:
> > Is there a big binary-size win from removing the functions?  It seems as
> if the simplest solution that is friendly to downstream is to defer the
> commit that removes the old functions to immediately after 9.0 is
> branched.  That way, users of 8.x get the deprecation notice and can then
> upgrade everything and have it working in time for 9.0.
> >
> > Adding the new function to 7.0, without the deprecation notice and then
> not including the deprecation notice in 8.0 means that there is no release
> where downstream users that jump from release to release will ever see the
> deprecation.
> >
> > David
> >
> >> On 13 Aug 2018, at 10:21, Stephen Kelly <steveire at gmail.com> wrote:
> >>
> >> Cool, sounds good.
> >>
> >> Yes, one of the commits had a mistake and I fixed it with r339379.
> >>
> >> Thanks,
> >>
> >> Stephen.
> >>
> >>
> >> On Mon, 13 Aug 2018, 10:06 Hans Wennborg, <hans at chromium.org> wrote:
> >> I think David suggested merging also the deprecation notice.
> >>
> >> Apologies, I misread your statement.  Merging just the new API,
> >> without the deprecation markers, and encouraging folks to upgrade in a
> >> release note sounds good to me.
> >>
> >> Looks like that would be r339372, r339373 and r339374. Did I miss
> anything?
> >>
> >>
> >> On Mon, Aug 13, 2018 at 10:52 AM, Stephen Kelly <steveire at gmail.com>
> wrote:
> >> > Hi Hans,
> >> >
> >> > I suggested merging only the new API to 7.0.0. Not doing the port and
> not
> >> > deprecating the old API. Did you miss that?
> >> >
> >> > Thanks,
> >> >
> >> > Stephen.
> >> >
> >> >
> >> > On Mon, 13 Aug 2018, 09:48 Hans Wennborg, <hans at chromium.org> wrote:
> >> >>
> >> >> Hmm, if we merge the deprecation notices to 7.0.0, we'd also need to
> >> >> merge all the clang, clang-tools-extra, etc. updates to avoid the
> >> >> warnings in our own code, at which point we would have merged the
> >> >> whole thing, and I'd prefer not to do that.
> >> >>
> >> >> I think just having a clear release note for 8.0.0 explaining what's
> >> >> changed and what developers need to do is good enough. I believe
> >> >> that's how API changes normally go.
> >> >>
> >> >> On Fri, Aug 10, 2018 at 7:35 PM,  <paul.robinson at sony.com> wrote:
> >> >> > If the totality of the change is deemed too large to backported, it
> >> >> > could
> >> >> > make sense to only backport the new API and made a message to the
> >> >> > release
> >> >> > notes to encourage downstreams to port.
> >> >> >
> >> >> >
> >> >> >
> >> >> > This is a lot kinder to downstream consumers than usual, and I
> fully
> >> >> > support
> >> >> > doing it that way.
> >> >> >
> >> >> > --paulr
> >> >> >
> >> >> >
> >> >> >
> >> >> > From: cfe-dev [mailto:cfe-dev-bounces at lists.llvm.org] On Behalf Of
> >> >> > Stephen
> >> >> > Kelly via cfe-dev
> >> >> > Sent: Friday, August 10, 2018 1:19 PM
> >> >> > To: David Chisnall; Hans Wennborg
> >> >> > Cc: Clang Dev
> >> >> > Subject: Re: [cfe-dev] API Removal Notice (4 weeks): getStartLoc,
> >> >> > getLocStart, getLocEnd
> >> >> >
> >> >> >
> >> >> >
> >> >> > Hi David,
> >> >> >
> >> >> >
> >> >> >
> >> >> > I think that's a release manager decision (cc Hans)
> >> >> >
> >> >> >
> >> >> >
> >> >> > The API doesn't exist in 7.0.0. All of my commits (introduce the
> new
> >> >> > API,
> >> >> > port to it, deprecate the old API) would have to be backported.
> >> >> >
> >> >> >
> >> >> >
> >> >> > In discussions so far, it was not a priority to keep the old API or
> >> >> > notify
> >> >> > particularly sensitively.
> >> >> >
> >> >> >
> >> >> >
> >> >> > If the totality of the change is deemed too large to backported, it
> >> >> > could
> >> >> > make sense to only backport the new API and made a message to the
> >> >> > release
> >> >> > notes to encourage downstreams to port.
> >> >> >
> >> >> >
> >> >> >
> >> >> > Thanks,
> >> >> >
> >> >> >
> >> >> >
> >> >> > Stephen.
> >> >> >
> >> >> > On Fri, 10 Aug 2018, 14:44 David Chisnall, <
> David.Chisnall at cl.cam.ac.uk>
> >> >> > wrote:
> >> >> >
> >> >> > On 10 Aug 2018, at 00:13, Stephen Kelly via cfe-dev
> >> >> > <cfe-dev at lists.llvm.org>
> >> >> > wrote:
> >> >> >>
> >> >> >> All third-party code must be similarly updated.
> >> >> >>
> >> >> >> Currently, the old names remain in the code, but annotated as
> >> >> >> deprecated.
> >> >> >
> >> >> > If you haven’t already, please will you add this change to the set
> to be
> >> >> > backported to the 7.0 branch?  The deprecation notice was really
> useful
> >> >> > - it
> >> >> > told me exactly what I needed to change to make it go away - but
> if it
> >> >> > never
> >> >> > ships in a major release then a lot of downstream users will never
> see
> >> >> > it.
> >> >> >
> >> >> > David
> >
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180820/dc1f3326/attachment.html>