llvm dev - Feb 2016 - Question about an error we're now starting to get on LLVM 3.8.0rc2since

> From: mehdi.amini at apple.com [mailto:mehdi.amini at apple.com] 
> Sent: Wednesday, February 10, 2016 1:51 PM
> To: Duncan P. N. Exon Smith
> Subject: Re: [llvm-dev] Question about an error we're now starting to
get on LLVM 3.8.0rc2since
>> 
>> You're looking for r252380.  Relevant excerpt:
>> 
>>    Note: if you have out-of-tree code, it should be fairly easy to
revert
>>    this patch downstream while you update your out-of-tree call sites.
>>    Note that these conversions are occasionally latent bugs (that may
>>    happen to "work" now, but only because of getting lucky
with UB;
>>    follow-ups will change your luck).  When they are valid, I suggest
using
>>    `->getIterator()` to go from pointer to iterator, and `&*` to
go from
>>    iterator to pointer.
>> 
>>> 
>>>        Is this change indeed intended as a visible API change to
source code generating references to argument list values?  If so, can you point
me to a description of how I should >>> change our code?  Should I bug
someone else about this problem?  Should this API change be documented in the
3.8.0 release notes?
>> 
>> Release notes is a good idea; somehow I never think of that.  Hans, is
it
>> too late?
> I wonder if it isn't it a bit to much detailed to put this in the
release notes? I mean we change the C++ API all the time...
> Here is the level of details we went with in 3.7:
http://llvm.org/releases/3.7.0/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release
>
>
> -- 
> Mehdi
I strongly recommend that C++ API changes, from one release to the next, be
documented in the release notes.  I venture to say that a large fraction of
out-of-tree LLVM users (like us) are exposed to API changes only at release
times.  I know of no other central place where they are documented.  Not all API
changes are discussed in llvm-dev.  Even when they are, the API changes are
often buried in a lot of additional design detail not relevant to the source
code of the callers.

To maximize the release candidate testing time for us out-of-tree users, I also
recommend introducing any API change documentation as early as possible - in RC1
if possible.

For the 3.7.0 release, it took me the better part of a month to track down and
remedy the changes to our sources that were necessary, and by then, it was too
late to reflect any of our findings in the final release.  We even had to go
back and re-implement some fixes that we initially just worked around.  We just
finished with those when 3.8.0rc2 came out.  The initial elapsed time to get 3.7
up and running in our environment would have been only a couple of days, if the
API changes had been highlighted early.

In contrast, this change to formal Argument referencing appears to be the only
API change that affected us for the 3.8 release.  So I doubt that the net burden
on the committers is actually significant.

The text of the change documentation need not be large: Just an example of the
caller change necessary is usually sufficient, perhaps with a pointer to any
prior discussions that motivated the change, and reference to what headers are
affected.  As Duncan did here.

Thanks for the consideration!
	-Kevin

> On Feb 10, 2016, at 11:32 AM, Harris, Kevin <Kevin.Harris at
unisys.com> wrote:
> 
>> From: mehdi.amini at apple.com [mailto:mehdi.amini at apple.com] 
>> Sent: Wednesday, February 10, 2016 1:51 PM
>> To: Duncan P. N. Exon Smith
>> Subject: Re: [llvm-dev] Question about an error we're now starting
to get on LLVM 3.8.0rc2since
> 
> 
>>> 
>>> You're looking for r252380.  Relevant excerpt:
>>> 
>>>   Note: if you have out-of-tree code, it should be fairly easy to
revert
>>>   this patch downstream while you update your out-of-tree call
sites.
>>>   Note that these conversions are occasionally latent bugs (that
may
>>>   happen to "work" now, but only because of getting lucky
with UB;
>>>   follow-ups will change your luck).  When they are valid, I
suggest using
>>>   `->getIterator()` to go from pointer to iterator, and `&*`
to go from
>>>   iterator to pointer.
>>> 
>>>> 
>>>>       Is this change indeed intended as a visible API change to
source code generating references to argument list values?  If so, can you point
me to a description of how I should >>> change our code?  Should I bug
someone else about this problem?  Should this API change be documented in the
3.8.0 release notes?
>>> 
>>> Release notes is a good idea; somehow I never think of that.  Hans,
is it
>>> too late?
> 
>> I wonder if it isn't it a bit to much detailed to put this in the
release notes? I mean we change the C++ API all the time...
> 
>> Here is the level of details we went with in 3.7:
http://llvm.org/releases/3.7.0/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release
>> 
>> 
>> -- 
>> Mehdi
> 
> I strongly recommend that C++ API changes, from one release to the next, be
documented in the release notes.  I venture to say that a large fraction of
out-of-tree LLVM users (like us) are exposed to API changes only at release
times.  I know of no other central place where they are documented.  Not all API
changes are discussed in llvm-dev. Even when they are, the API changes are often
buried in a lot of additional design detail not relevant to the source code of
the callers.
> 
> To maximize the release candidate testing time for us out-of-tree users, I
also recommend introducing any API change documentation as early as possible -
in RC1 if possible.
> 
> For the 3.7.0 release, it took me the better part of a month to track down
and remedy the changes to our sources that were necessary, and by then, it was
too late to reflect any of our findings in the final release.  We even had to go
back and re-implement some fixes that we initially just worked around.  We just
finished with those when 3.8.0rc2 came out.  The initial elapsed time to get 3.7
up and running in our environment would have been only a couple of days, if the
API changes had been highlighted early.
> 
> In contrast, this change to formal Argument referencing appears to be the
only API change that affected us for the 3.8 release.  So I doubt that the net
burden on the committers is actually significant.
> 
> The text of the change documentation need not be large: Just an example of
the caller change necessary is usually sufficient, perhaps with a pointer to any
prior discussions that motivated the change, and reference to what headers are
affected.  As Duncan did here.
I think such a shift in the policy would require at least an dedicated RFC on
llvm-dev and a change in the developer guide.

-- 
Mehdi

> On 2016-Feb-10, at 11:34, Mehdi Amini <mehdi.amini at apple.com>
wrote:
> 
>> 
>> On Feb 10, 2016, at 11:32 AM, Harris, Kevin <Kevin.Harris at
unisys.com> wrote:
>> 
>>> From: mehdi.amini at apple.com [mailto:mehdi.amini at apple.com] 
>>> Sent: Wednesday, February 10, 2016 1:51 PM
>>> To: Duncan P. N. Exon Smith
>>> Subject: Re: [llvm-dev] Question about an error we're now
starting to get on LLVM 3.8.0rc2since
>> 
>> 
>>>> 
>>>> You're looking for r252380.  Relevant excerpt:
>>>> 
>>>>  Note: if you have out-of-tree code, it should be fairly easy
to revert
>>>>  this patch downstream while you update your out-of-tree call
sites.
>>>>  Note that these conversions are occasionally latent bugs (that
may
>>>>  happen to "work" now, but only because of getting
lucky with UB;
>>>>  follow-ups will change your luck).  When they are valid, I
suggest using
>>>>  `->getIterator()` to go from pointer to iterator, and
`&*` to go from
>>>>  iterator to pointer.
>>>> 
>>>>> 
>>>>>      Is this change indeed intended as a visible API change
to source code generating references to argument list values?  If so, can you
point me to a description of how I should >>> change our code?  Should
I bug someone else about this problem?  Should this API change be documented in
the 3.8.0 release notes?
>>>> 
>>>> Release notes is a good idea; somehow I never think of that. 
Hans, is it
>>>> too late?
>> 
>>> I wonder if it isn't it a bit to much detailed to put this in
the release notes? I mean we change the C++ API all the time...
>> 
>>> Here is the level of details we went with in 3.7:
http://llvm.org/releases/3.7.0/docs/ReleaseNotes.html#non-comprehensive-list-of-changes-in-this-release
>>> 
>>> 
>>> -- 
>>> Mehdi
>> 
>> I strongly recommend that C++ API changes, from one release to the
next, be documented in the release notes.  I venture to say that a large
fraction of out-of-tree LLVM users (like us) are exposed to API changes only at
release times.  I know of no other central place where they are documented.  Not
all API changes are discussed in llvm-dev. Even when they are, the API changes
are often buried in a lot of additional design detail not relevant to the source
code of the callers.
>> 
>> To maximize the release candidate testing time for us out-of-tree
users, I also recommend introducing any API change documentation as early as
possible - in RC1 if possible.
>> 
>> For the 3.7.0 release, it took me the better part of a month to track
down and remedy the changes to our sources that were necessary, and by then, it
was too late to reflect any of our findings in the final release.  We even had
to go back and re-implement some fixes that we initially just worked around.  We
just finished with those when 3.8.0rc2 came out.  The initial elapsed time to
get 3.7 up and running in our environment would have been only a couple of days,
if the API changes had been highlighted early.
>> 
>> In contrast, this change to formal Argument referencing appears to be
the only API change that affected us for the 3.8 release.  So I doubt that the
net burden on the committers is actually significant.
>> 
>> The text of the change documentation need not be large: Just an example
of the caller change necessary is usually sufficient, perhaps with a pointer to
any prior discussions that motivated the change, and reference to what headers
are affected.  As Duncan did here.
> 
> I think such a shift in the policy would require at least an dedicated RFC
on llvm-dev and a change in the developer guide.
This is a great reason to start tracking trunk and maybe even move
to in-tree development ;).  Realistically, C++ API changes do
happen all the time, and I think documenting them has to be on a
"best effort" basis.  We could certainly be better at this, but I
wouldn't want to move to a model where it was required.