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

Tue Aug 21 14:51:16 PDT 2018

I think this topic is worthy of a new thread with a new subject to 
attack a wider/different audience.

Personally, I both like and have serious hesitations about the implicit 
proposal of using deprecation and backporting deprecations.  I 
definitely do not see consensus here and if we want to propose such a 
thing, it deserves some serious discussion.

Philip

On 08/21/2018 01:24 PM, Hans Wennborg via cfe-dev wrote:
> On Mon, Aug 20, 2018 at 3:00 PM, Stephen Kelly via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
>> Hi,
>>
>> I'm generally fine with that, but
>>
>> * That is not how cfe generally operates regarding API changes
>> * There is no consensus for doing that
>> * There hasn't been any discussion which you might expect for such a change
>> in how cfe operates.
>>
>> I am not really comfortable with using a lack of consensus (and lack of
>> discussion) to create new precedent of policy about how Clang does API
>> removals. Currently the policy is 'just do it'. I come from experience in
>> projects which are very conservative (Qt, CMake) so I wanted to at least be
>> structured about the removal and notify (with this thread) about it.
>>
>> The suggestion is to change existing policy quite dramatically, and I don't
>> see consensus for doing that.
>>
>> I'm inclined to stick with the plan I notified already. There is already
>> implicit consensus (not unanimous approval, which is a different and
>> not-sought-after thing) to do that. Thoughts?
>>
>> By the way, I still think that regardless of when the old API gets removed,
>> it makes sense to cherry-pick the new API to 7.0.0 if it's not already too
>> late.
> I've merged the new API to the 7.0 branch in r340332 per the original plan.
>
> Will you write something for the release notes?
>
> Thanks,
> Hans
>
>
>> On 14/08/18 09:43, Hans Wennborg via cfe-dev 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?
>>>
>>> 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
>>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev