As result, function body context is added as internal context and args / return vars contexts are added as imported to internal.
Details
- Reviewers
brauch apol mwolff - Commits
- R59:a61ed96f2688: Refactor and fix DU-Chain creation
Tests pass. Wiped cache and started KDevelop - standard libs are parsed without faults.
Diff Detail
- Repository
- R59 KDevelop Go
- Branch
- fixing
- Lint
No Linters Available - Unit
No Unit Test Coverage
overall certainly an improvement, some suggestions from my side
duchain/builders/declarationbuilder.cpp | ||
---|---|---|
587 ↗ | (On Diff #15932) | 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?) |
606 ↗ | (On Diff #15932) | 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 |
duchain/builders/declarationbuilder.h | ||
71 ↗ | (On Diff #15932) | 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 |
105 ↗ | (On Diff #15932) | dito, see above |
Thank you for your review!
duchain/builders/declarationbuilder.cpp | ||
---|---|---|
587 ↗ | (On Diff #15932) | I think that it's handful because in Go language "named result parameters " exists: https://golang.org/doc/effective_go.html#named-results. |
606 ↗ | (On Diff #15932) | You were right, there is no need for calling setType since visitBlock already opens context with correct type. |
thanks for the explanation - I have zero clue about go, but looking at that snippet I can understand why you want another context there.
Cool stuff!