<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119178/">https://git.reviewboard.kde.org/r/119178/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 8th, 2014, 1:52 p.m. UTC, <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">this sounds wrong. Where is the lookup for "dont_use_me" done, that should be adapted to only look in the context of array{}, which should be of Class type and not propagate up into other contexts.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is already the case. When I put debug statements in DeclarationBuilder and even UseBuilder, everything is correct. When I ask ExpressionVisitor to find the declaration corresponding to "array.dont_use_me", the automatic declaration of "array.dont_use_me" on line 3 is returned. In fact, every call to newUse() is correct :</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">newUse(line 3 col 1 to 5, var array)</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">newUse(line 3 col 7 to 17, automatic declaration of dont_use_me, the declaration is on line 3 col 7 to 17)</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"var dont_use_me" is never passed to any newUse call, so another mechanism is responsible for changing the right "dont_use_me" to the one I don't want, after newUse has been called. By forcing the declarations to be direct, I think that I force KDevelop to keep my declarations and not to try to re-resolve using their names.</p></pre>
<br />
<p>- Denis</p>
<br />
<p>On July 8th, 2014, 1:19 p.m. UTC, Denis Steckelmacher wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Denis Steckelmacher.</div>
<p style="color: grey;"><i>Updated July 8, 2014, 1:19 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdev-qmljs
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch calls "setAlwaysForceDirect" on every declaration created by the QMLJS plugin. I don't know what are all the implications, but it solves a nasty bug:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">var dont_use_me;<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
var array = {};<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" />
array.dont_use_me = "fail";</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In this snippet, without this patch, "array.dont_use_me" is considered to be an use of "var dont_use_me". I've verified that UseBuilder does not create an use for "var dont_use_me" on line 3. It still creates an use for "array.dont_use_me" that is automatically declared on line 3, though.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think that line 3 is still considered to be an use of line 1 because the two incrimined declarations (the variable and the object member) have the same name. Moreover, "array" has an internal context that has no name, so the scope of the two declarations is the same. Using only their name, there is no way to differentiate them, hence the unwanted use. By forcing every declaration to be direct, this problem disappears.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would like some advice here because the fact that a declaration is direct or not seems to have deep implications in KDevelop. Is it correct to have every declaration direct? Why is this not the case by default?</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The whole testsuite still passes and a new unit test is added (it looks like the example given above). The unit test fails without this patch and passes when this patch is applied. The testsuite is not slowed down nor sped up by this patch.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>duchain/declarationbuilder.h <span style="color: grey">(1ab77a1)</span></li>
<li>duchain/declarationbuilder.cpp <span style="color: grey">(2ec2f49)</span></li>
<li>tests/files/arrays.js <span style="color: grey">(e9a8efa)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119178/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>