D6412: Refactor and fix DU-Chain creation

Mikhail Ivchenko noreply at phabricator.kde.org
Wed Jun 28 09:47:14 UTC 2017

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

  Thank you for your review!


> 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.

  R59 KDevelop Go



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>

More information about the KDevelop-devel mailing list