<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, 14 Aug 2018 at 01:44, Hans Wennborg via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'd be fine with that too, and if we want to deprecate and remove an<br>
API across a release, this would really be the way to do it.<br>
<br>
Richard, do you have any thoughts on this?<br></blockquote><div><br></div><div>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).</div><div><br></div><div>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.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Aug 13, 2018 at 11:40 AM, David Chisnall<br>
<<a href="mailto:David.Chisnall@cl.cam.ac.uk" target="_blank">David.Chisnall@cl.cam.ac.uk</a>> wrote:<br>
> 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.<br>
><br>
> 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.<br>
><br>
> David<br>
><br>
>> On 13 Aug 2018, at 10:21, Stephen Kelly <<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>> wrote:<br>
>><br>
>> Cool, sounds good.<br>
>><br>
>> Yes, one of the commits had a mistake and I fixed it with r339379.<br>
>><br>
>> Thanks,<br>
>><br>
>> Stephen.<br>
>><br>
>><br>
>> On Mon, 13 Aug 2018, 10:06 Hans Wennborg, <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
>> I think David suggested merging also the deprecation notice.<br>
>><br>
>> Apologies, I misread your statement.  Merging just the new API,<br>
>> without the deprecation markers, and encouraging folks to upgrade in a<br>
>> release note sounds good to me.<br>
>><br>
>> Looks like that would be r339372, r339373 and r339374. Did I miss anything?<br>
>><br>
>><br>
>> On Mon, Aug 13, 2018 at 10:52 AM, Stephen Kelly <<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>> wrote:<br>
>> > Hi Hans,<br>
>> ><br>
>> > I suggested merging only the new API to 7.0.0. Not doing the port and not<br>
>> > deprecating the old API. Did you miss that?<br>
>> ><br>
>> > Thanks,<br>
>> ><br>
>> > Stephen.<br>
>> ><br>
>> ><br>
>> > On Mon, 13 Aug 2018, 09:48 Hans Wennborg, <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
>> >><br>
>> >> Hmm, if we merge the deprecation notices to 7.0.0, we'd also need to<br>
>> >> merge all the clang, clang-tools-extra, etc. updates to avoid the<br>
>> >> warnings in our own code, at which point we would have merged the<br>
>> >> whole thing, and I'd prefer not to do that.<br>
>> >><br>
>> >> I think just having a clear release note for 8.0.0 explaining what's<br>
>> >> changed and what developers need to do is good enough. I believe<br>
>> >> that's how API changes normally go.<br>
>> >><br>
>> >> On Fri, Aug 10, 2018 at 7:35 PM,  <<a href="mailto:paul.robinson@sony.com" target="_blank">paul.robinson@sony.com</a>> wrote:<br>
>> >> > If the totality of the change is deemed too large to backported, it<br>
>> >> > could<br>
>> >> > make sense to only backport the new API and made a message to the<br>
>> >> > release<br>
>> >> > notes to encourage downstreams to port.<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > This is a lot kinder to downstream consumers than usual, and I fully<br>
>> >> > support<br>
>> >> > doing it that way.<br>
>> >> ><br>
>> >> > --paulr<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > From: cfe-dev [mailto:<a href="mailto:cfe-dev-bounces@lists.llvm.org" target="_blank">cfe-dev-bounces@lists.llvm.org</a>] On Behalf Of<br>
>> >> > Stephen<br>
>> >> > Kelly via cfe-dev<br>
>> >> > Sent: Friday, August 10, 2018 1:19 PM<br>
>> >> > To: David Chisnall; Hans Wennborg<br>
>> >> > Cc: Clang Dev<br>
>> >> > Subject: Re: [cfe-dev] API Removal Notice (4 weeks): getStartLoc,<br>
>> >> > getLocStart, getLocEnd<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > Hi David,<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > I think that's a release manager decision (cc Hans)<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > The API doesn't exist in 7.0.0. All of my commits (introduce the new<br>
>> >> > API,<br>
>> >> > port to it, deprecate the old API) would have to be backported.<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > In discussions so far, it was not a priority to keep the old API or<br>
>> >> > notify<br>
>> >> > particularly sensitively.<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > If the totality of the change is deemed too large to backported, it<br>
>> >> > could<br>
>> >> > make sense to only backport the new API and made a message to the<br>
>> >> > release<br>
>> >> > notes to encourage downstreams to port.<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > Thanks,<br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > Stephen.<br>
>> >> ><br>
>> >> > On Fri, 10 Aug 2018, 14:44 David Chisnall, <<a href="mailto:David.Chisnall@cl.cam.ac.uk" target="_blank">David.Chisnall@cl.cam.ac.uk</a>><br>
>> >> > wrote:<br>
>> >> ><br>
>> >> > On 10 Aug 2018, at 00:13, Stephen Kelly via cfe-dev<br>
>> >> > <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>><br>
>> >> > wrote:<br>
>> >> >><br>
>> >> >> All third-party code must be similarly updated.<br>
>> >> >><br>
>> >> >> Currently, the old names remain in the code, but annotated as<br>
>> >> >> deprecated.<br>
>> >> ><br>
>> >> > If you haven’t already, please will you add this change to the set to be<br>
>> >> > backported to the 7.0 branch?  The deprecation notice was really useful<br>
>> >> > - it<br>
>> >> > told me exactly what I needed to change to make it go away - but if it<br>
>> >> > never<br>
>> >> > ships in a major release then a lot of downstream users will never see<br>
>> >> > it.<br>
>> >> ><br>
>> >> > David<br>
><br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>