Review Request 118483: Ensure that declarations returned by Utils::getDeclaration are visible in the context passed as parameter

[Sven Brauch]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118483/#review59022
-----------------------------------------------------------


Hmm. You can turn off declarations from being in the symbol table (setInSymbolTable(false)). I'm not sure if this is the right solution here because I don't know by heart if class members should be in the symbol table (I'll have to look that up) but I guess you could try it.

- Sven Brauch


On June 2, 2014, 8:36 p.m., Denis Steckelmacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118483/
> -----------------------------------------------------------
> 
> (Updated June 2, 2014, 8:36 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-qmljs
> 
> 
> Description
> -------
> 
> This is a workaround for a behavior that I cannot completely explain. Here is a snippet of Javascript code that exhibits the behavior:
> 
> ----
> var a = {key: "value"};
> 
> key = 2;
> ----
> 
> Without my patch, "key = 2" is considered to be an use of a.key. However, the declaration of "key" is in a context that spans the space inside the brackets. Some debugging has even shown that "key" is not listed in DUContext::getDeclarations(), called on the context in which "key = 2" appears (the top-level context):
> 
> ----
> looking for "key":
>     <struct> a
>  -> found "string key"
> ----
> 
> By looking at the code and running GDB, I've seen that when the context in which a declaration is being looked for is a TopDUContext, then the symbol table is used instead of the normal code path. The problem is that "key" appears in the symbol table (I think this is expected), but I don't want it to be "visible" from the top-level context.
> 
> This patch is therefore a mean to avoid using TopDUContext::getDeclarations. First, getLocalDeclarations is tried (this finds declaration in the current context and its parents, but not its imported parent contexts, if I've correctly understood how the thing works), then, and only if the context has imported parent contexts, getDeclarations is used.
> 
> This is quite a dirty solution. Is there a mean to prevent TopDUContext from short-circuiting the normal declaration filtering process and from returning declarations that should normally not be visible?
> 
> 
> Diffs
> -----
> 
>   duchain/helper.cpp 27f5dd4 
> 
> Diff: https://git.reviewboard.kde.org/r/118483/diff/
> 
> 
> Testing
> -------
> 
> The whole testsuite passes, but the patch seems a little fragile: the top-level context may also have imported parent contexts (if there are import statements, or in the future when Javascript require() statements will be supported), and so the second if may well be entered even if context is a TopDUContext. The patch therefore does not fix the bug in all cases.
> 
> 
> Thanks,
> 
> Denis Steckelmacher
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140602/ba7b062b/attachment-0001.html>