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

[David Chisnall via cfe-dev]

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