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

Tue Aug 21 13:24:48 PDT 2018

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