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

On Wed, Feb 10, 2016 at 10:50 AM, Mehdi Amini via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>>>        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...
Sure, but I think anything that helps users move to the next version
is valuable. I realize we'll probably never have exhaustive lists of
API changes in the release notes, but every little helps, and if
Duncan wants to write a note for this one, I'll certainly take it.

Thanks,
Hans

If we're considering adding items to the release notes:

A couple of pain points I encountered when migrating my code from 3.5.1 to
3.7.1. I don't keep ToT, and thereby avoid the constant API churn, but then
I have a bit of work to do when upgrading from one release to another.

Bitcode reader/writer: I have a class that creates a Module*, and
optionally preloads it with bitcode read from a file.

In 3.5.1 I used:

  ErrorOr<Module *> parseBitcodeFile(MemoryBuffer *Buffer,
                                     LLVMContext &Context);

In 3.7.1 this became:

  ErrorOr<std::unique_ptr<Module>>
  parseBitcodeFile(MemoryBufferRef Buffer, LLVMContext &Context,
                   DiagnosticHandlerFunction DiagnosticHandler = nullptr);

I had a bit of difficulty figuring out why the unique_ptr<> was used. I
thought the purpose of a unique_ptr<> was to ensure that some (temporary)
object could never be leaked or dangled, by making it very hard to get at
the underlying raw pointer. Yet, everywhere else I needed to pass a
Module*, and there's nothing to stop me from dangling it.

I understand we're not out to create a C++11 tutorial, but major changes in
the memory ownership semantics ought to be documented.


Debug metadata generation: LOTS of changes, but the worst:

In 3.5.1 we have:

    DICompositeType createReplaceableForwardDecl(
        unsigned Tag, StringRef Name, DIDescriptor Scope, DIFile F,
        unsigned Line, unsigned RuntimeLang = 0, uint64_t SizeInBits = 0,
        uint64_t AlignInBits = 0, StringRef UniqueIdentifier = StringRef());

In 3.7.1 this was changed to:

    DICompositeType *createReplaceableCompositeType(
        unsigned Tag, StringRef Name, DIScope *Scope, DIFile *F, unsigned
Line,
        unsigned RuntimeLang = 0, uint64_t SizeInBits = 0,
        uint64_t AlignInBits = 0, unsigned Flags = DINode::FlagFwdDecl,
        StringRef UniqueIdentifier = "");

However, simply making the code change and adjusting the arguments leads to
an assert.  I also had to call replaceTemporary() at the point where the
replacement was available.  This was not required in 3.5.1. Prior to this
point I was unaware of the distinction between uniqued and non-uniqued
metadata, nor did I have any reason to care.



On Wed, Feb 10, 2016 at 1:57 PM, Hans Wennborg via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Wed, Feb 10, 2016 at 10:50 AM, Mehdi Amini via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >>>        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...
>
> Sure, but I think anything that helps users move to the next version
> is valuable. I realize we'll probably never have exhaustive lists of
> API changes in the release notes, but every little helps, and if
> Duncan wants to write a note for this one, I'll certainly take it.
>
> Thanks,
> Hans
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160210/ed9fbce5/attachment.html>

> On 2016-Feb-10, at 10:57, Hans Wennborg <hans at chromium.org> wrote:
> 
> On Wed, Feb 10, 2016 at 10:50 AM, Mehdi Amini via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>>>>       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...
> 
> Sure, but I think anything that helps users move to the next version
> is valuable. I realize we'll probably never have exhaustive lists of
> API changes in the release notes, but every little helps, and if
> Duncan wants to write a note for this one, I'll certainly take it.
> 
> Thanks,
> Hans
r260415

Hi David,

Have you started looking at 3.8 yet? Your email mentions issues with
3.7, but the ship for updated those release notes sailed when it was
released.

Cheers,
Hans

On Wed, Feb 10, 2016 at 11:23 AM, David Jones <djones at xtreme-eda.com>
wrote:
> If we're considering adding items to the release notes:
>
> A couple of pain points I encountered when migrating my code from 3.5.1 to
> 3.7.1. I don't keep ToT, and thereby avoid the constant API churn, but
then
> I have a bit of work to do when upgrading from one release to another.
>
> Bitcode reader/writer: I have a class that creates a Module*, and
optionally
> preloads it with bitcode read from a file.
>
> In 3.5.1 I used:
>
>   ErrorOr<Module *> parseBitcodeFile(MemoryBuffer *Buffer,
>                                      LLVMContext &Context);
>
> In 3.7.1 this became:
>
>   ErrorOr<std::unique_ptr<Module>>
>   parseBitcodeFile(MemoryBufferRef Buffer, LLVMContext &Context,
>                    DiagnosticHandlerFunction DiagnosticHandler = nullptr);
>
> I had a bit of difficulty figuring out why the unique_ptr<> was used.
I
> thought the purpose of a unique_ptr<> was to ensure that some
(temporary)
> object could never be leaked or dangled, by making it very hard to get at
> the underlying raw pointer. Yet, everywhere else I needed to pass a
Module*,
> and there's nothing to stop me from dangling it.
>
> I understand we're not out to create a C++11 tutorial, but major
changes in
> the memory ownership semantics ought to be documented.
>
>
> Debug metadata generation: LOTS of changes, but the worst:
>
> In 3.5.1 we have:
>
>     DICompositeType createReplaceableForwardDecl(
>         unsigned Tag, StringRef Name, DIDescriptor Scope, DIFile F,
>         unsigned Line, unsigned RuntimeLang = 0, uint64_t SizeInBits = 0,
>         uint64_t AlignInBits = 0, StringRef UniqueIdentifier =
StringRef());
>
> In 3.7.1 this was changed to:
>
>     DICompositeType *createReplaceableCompositeType(
>         unsigned Tag, StringRef Name, DIScope *Scope, DIFile *F, unsigned
> Line,
>         unsigned RuntimeLang = 0, uint64_t SizeInBits = 0,
>         uint64_t AlignInBits = 0, unsigned Flags = DINode::FlagFwdDecl,
>         StringRef UniqueIdentifier = "");
>
> However, simply making the code change and adjusting the arguments leads to
> an assert.  I also had to call replaceTemporary() at the point where the
> replacement was available.  This was not required in 3.5.1. Prior to this
> point I was unaware of the distinction between uniqued and non-uniqued
> metadata, nor did I have any reason to care.
>
>
>
> On Wed, Feb 10, 2016 at 1:57 PM, Hans Wennborg via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>>
>> On Wed, Feb 10, 2016 at 10:50 AM, Mehdi Amini via llvm-dev
>> <llvm-dev at lists.llvm.org> wrote:
>> >>>        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...
>>
>> Sure, but I think anything that helps users move to the next version
>> is valuable. I realize we'll probably never have exhaustive lists
of
>> API changes in the release notes, but every little helps, and if
>> Duncan wants to write a note for this one, I'll certainly take it.