Duncan P. N. Exon Smith via llvm-dev
2016-Feb-10 19:38 UTC
[llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since
> 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.
Mehdi Amini via llvm-dev
2016-Feb-10 19:47 UTC
[llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since
> On Feb 10, 2016, at 11:38 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > >> >> 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.Right now it is not even officially "encouraged". I don't know of any guideline on this (it there any?), and I'm not fond of the "stuff gets randomly in the release notes while other stuff does not without any good reason". On the topic, I'm not convinced adding minor changes to the C++ API to release notes is a good direction all. I'd rather keep the surface of the release notes limited to high-level changes: how to initialize the compiler, add passes, etc. Till recently, only the C API changes were documented "best effort" in the release notes (they are now required to be documented there). -- Mehdi
Harris, Kevin via llvm-dev
2016-Feb-10 19:56 UTC
[llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since
> From: dexonsmith at apple.com [mailto:dexonsmith at apple.com] > Sent: Wednesday, February 10, 2016 2:38 PM > To: Mehdi Amini > Cc: Harris, Kevin; LLVM Dev > Subject: Re: [llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since>>>>> 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.That sounds reasonable. I'm unfamiliar with the formalities, but I wouldn't mind leading the charge.>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.Tracking the trunk is a reasonable suggestion, and I've considered it. But that doesn't obviate the need for API changes to be highlighted and described, in order to understand how to modify the API callers. Sometimes, esp. when a change strongly increases the level of abstraction, it takes a long time to determine how to modify the caller, even when you've already pinpointed the change. Moving to in-tree development sounds desirable, but would unlikely be of interest to anyone else, and would require the participation of lawyers . . . :-) I'm mostly happy with a "best-effort" basis, as long as it doesn't degenerate into a "no-effort" basis, which is what happened with 3.7. -Kevin
Mehdi Amini via llvm-dev
2016-Feb-10 20:18 UTC
[llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since
> On Feb 10, 2016, at 11:56 AM, Harris, Kevin <Kevin.Harris at unisys.com> wrote: > >> From: dexonsmith at apple.com [mailto:dexonsmith at apple.com] >> Sent: Wednesday, February 10, 2016 2:38 PM >> To: Mehdi Amini >> Cc: Harris, Kevin; LLVM Dev >> Subject: Re: [llvm-dev] Question about an error we're now starting to get on LLVM 3.8.0rc2since > >>>>>> 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. > > That sounds reasonable. I'm unfamiliar with the formalities, but I wouldn't mind leading the charge.The "formalities" is pretty limited (I hope...): an email to llvm-dev (starting with something "[RFC]" is preferable) to allow everyone to chime in and reach a consensus. It is best if can try to explain in the RFC why such a change is a good thing, what should and should not be added, one (or multiple) examples might be welcome, and optionally a patch that introduce the guideline (even on a "best effort") in docs/DeveloperPolicy.rst (requirement for C API to be documented in the release notes are already in this file) Recent examples: http://lists.llvm.org/pipermail/llvm-dev/2016-February/094804.html http://lists.llvm.org/pipermail/llvm-dev/2016-February/094851.html http://lists.llvm.org/pipermail/llvm-dev/2016-February/094869.html http://lists.llvm.org/pipermail/llvm-dev/2016-February/095275.html Hope this helps, -- Mehdi> >> 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. > > Tracking the trunk is a reasonable suggestion, and I've considered it. But that doesn't obviate the need for API changes to be highlighted and described, in order to understand how to modify the API callers. Sometimes, esp. when a change strongly increases the level of abstraction, it takes a long time to determine how to modify the caller, even when you've already pinpointed the change. > > Moving to in-tree development sounds desirable, but would unlikely be of interest to anyone else, and would require the participation of lawyers . . . :-) > > I'm mostly happy with a "best-effort" basis, as long as it doesn't degenerate into a "no-effort" basis, which is what happened with 3.7. > -Kevin