D6412: Refactor and fix DU-Chain creation

[Milian Wolff]

mwolff accepted this revision.
mwolff added a comment.


  overall certainly an improvement, some suggestions from my side

INLINE COMMENTS

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

> 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

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