D6412: Refactor and fix DU-Chain creation

Milian Wolff noreply at phabricator.kde.org
Wed Jun 28 08:15:18 UTC 2017

mwolff accepted this revision.
mwolff added a comment.

  overall certainly an improvement, some suggestions from my side


> declarationbuilder.cpp:587
> +        }
> +        if(retparamContext)
> +        {

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?)

> declarationbuilder.cpp:606
> +            DUChainWriteLocker lock;
> +            lastContext()->setType(DUContext::Other);
> +        }

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) {
      DUChainWriteLocker lock;
      bodyContext = lastContext();
      if (bodyContext) {

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

> declarationbuilder.h:71
> +     **/
> +    go::GoFunctionDeclaration* buildFunction(go::SignatureAst* node, go::BlockAst* block=0, go::IdentifierAst* name=0, const QByteArray& comment=QByteArray());

nitpicks here and below: please try to follow our coding style and add spaces around operators (such as the `=` here

also, you can use `= {}` to reduce some typing repetition here for the `comment`

> declarationbuilder.h:105
>      virtual go::GoFunctionDeclaration* declareFunction(go::IdentifierAst* id, const go::GoFunctionType::Ptr& type,
> -                                                       DUContext* paramContext, DUContext* retparamContext, const QByteArray& comment=QByteArray()) override;
> +                                                       DUContext* paramContext, DUContext* retparamContext, const QByteArray& comment=QByteArray(), DUContext* bodyContext = nullptr) override;

dito, see above

  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/b824e1ad/attachment-0001.html>

More information about the KDevelop-devel mailing list