<table><tr><td style="">ematirov marked 5 inline comments as done.<br />ematirov added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D6412" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Thank you for your review!</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6412#inline-26461" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mwolff</span> wrote in <span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:587</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">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?)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think that it's handful because in Go language "named result parameters " exists: <a href="https://golang.org/doc/effective_go.html#named-results" class="remarkup-link" target="_blank" rel="noreferrer">https://golang.org/doc/effective_go.html#named-results</a>.<br />
We need to provide declarations for them because they are declared inside function context and have zero values by default. See <a href="https://play.golang.org/p/Azoc_LmHkG" class="remarkup-link" target="_blank" rel="noreferrer">this snippet</a> 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: <a href="https://phabricator.kde.org/F3794508" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">F3794508: named return args example.png</a></p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D6412#inline-26462" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mwolff</span> wrote in <span style="color: #4b4d51; font-weight: bold;">declarationbuilder.cpp:606</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">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:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">if (block) {
visitBlock(block);
DUChainWriteLocker lock;
bodyContext = lastContext();
if (bodyContext) {
bodyContext->setType(DUContext::Other);
}
}</pre></div>
<p style="padding: 0; margin: 8px;">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</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">You were right, there is no need for calling setType since visitBlock already opens context with correct type.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R59 KDevelop Go</div></div></div><br /><div><strong>BRANCH</strong><div><div>fixing</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D6412" rel="noreferrer">https://phabricator.kde.org/D6412</a></div></div><br /><div><strong>To: </strong>ematirov, brauch, apol, mwolff<br /><strong>Cc: </strong>mwolff, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger<br /></div>