D6412: Refactor and fix DU-Chain creation

[Mikhail Ivchenko]

ematirov marked 5 inline comments as done.
ematirov added a comment.


  Thank you for your review!

INLINE COMMENTS

> mwolff wrote in declarationbuilder.cpp:587
> a context for the return parameter? is that really required? just asking out of curiosity, afaik the other language plugins don't do that (or?)

I think that it's handful because in Go language "named result parameters " exists: https://golang.org/doc/effective_go.html#named-results.
We need to provide declarations for them because they are declared inside function context and have zero values by default.  See this snippet <https://play.golang.org/p/Azoc_LmHkG> for example of usage.  Aside from that, having them in separate context is useful for showing them as part of function signature. Since there could be more than one return value in Go it provides a handful documentation: F3794508: named return args example.png <https://phabricator.kde.org/F3794508>

> mwolff wrote in declarationbuilder.cpp:606
> are you sure that this always creates a context? i.e. even if the user typed somewhat broken code into the editor? I suggest you better rewrite this like the below, to be on the safe side:
> 
>   if (block) {
>       visitBlock(block);
>       DUChainWriteLocker lock;
>       bodyContext = lastContext();
>       if (bodyContext) {
>           bodyContext->setType(DUContext::Other);
>       }
>   }
> 
> better yet, could you set this type directly wherever you are creating the context itself? from a cursory glance, I don't even see where this is done

You were right, there is no need for calling setType since visitBlock already opens context with correct type.

REPOSITORY
  R59 KDevelop Go

BRANCH
  fixing

REVISION DETAIL
  https://phabricator.kde.org/D6412

To: ematirov, brauch, apol, mwolff
Cc: mwolff, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170628/9658db90/attachment.html>