[PATCH] D14265: DI: Reverse direction of subprogram -> function edge.

[Peter Collingbourne via llvm-commits]

On Thu, Nov 05, 2015 at 11:52:03AM -0800, Duncan P. N. Exon Smith wrote:
> I have a couple of minor comments below; otherwise this LGTM, as long
> as you can somehow prevent the DIBuilder API changes from inserting
> bugs in frontends (more below).
> 
> > On 2015-Nov-03, at 13:17, Peter Collingbourne <peter at pcc.me.uk> wrote:
> > 
> > pcc added a comment.
> > 
> > Adrian wrote:
> > 
> >> Could you include your upgrade script so people can upgrade their out-of-tree testcases?
> > 
> > 
> > F1054572: add-subprogram-attachments.sh <http://reviews.llvm.org/F1054572>
> > Attached. This is basically the same as Duncan's upgrade script attached to PR23367 with a quick hack to remove function references from subprograms.
> 
> Great.  Linking to that from the PR (and mentioning where to find it in
> your eventual commit message) will help out-of-tree people a lot.

Done.

> I think this reads a little more clearly as:
> --
> if (DISubprogram *SP = FunctionsWithSPs.lookup(F))
>   F->setSubprogram(SP->second);
> --
> (assuming that or something like it compiles).

Done.

> C++ APIs are awkward to update when they have defaults.  How can we
> protect against callers that were explicitly passing `nullptr` for the
> `Function*` argument?
> 
> Maybe for now you should add overloads such as:
> --
> void foo(DIFile *File, unsigned LineNo, DISubroutineType *Ty,
>          bool isLocalToUnit, bool isDefinition, unsigned ScopeLine,
>          unsigned Flags = 0, bool isOptimized = false,
>          MDNode *TParam = nullptr, MDNode *Decl = nullptr);
> void foo(DIFile *File, unsigned LineNo, DISubroutineType *Ty,
>          bool isLocalToUnit, bool isDefinition, unsigned ScopeLine,
>          unsigned Flags = 0, bool isOptimized, std::nullptr_t,
>          MDNode *TParam, MDNode *Decl = nullptr);
> --
> ??
> 
> Alternatively, changing the `Decl` argument to a `DISubprogram*` might
> be enough to fix this...

I decided to go with this approach (similarly for TParam) as it seems like
the right end state in any case.

Thanks,
-- 
Peter