<div dir="ltr">Reposting, since last one looks like didn't formatted well:<br><div><br>I'll put in words what I was trying to do in this fix... sorry because it has been so long that I'm reviewing it again.<br>
<br>Please check this for the reason of such fix: <a href="http://llvm.org/bugs/show_bug.cgi?id=13699#c9">http://llvm.org/bugs/show_bug.cgi?id=13699#c9</a><br><br>- I'm checking whether it's a friend because I still want to get the friend's target as a result if it's ok to do so.<br>
<br>(The current codebase has an issue doing this as can be checked in the issue tracker, and here I'm going for<br>the easier fix by changing the code completion source solely, which is what's affecting me. I'm not sure whether<br>
this friend lookup thing problem requires a major change in other parts of the codebase, which it may, so<br>I opted for the minor fix that would solve this for code completion at last.)<br><br>- The Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type check may be spurious in the original call of<br>
AddResult, but as can be seen, I'm gonna do a recursive call to it, and my intent was to so that, at this second<br>direct call to AddResult, such check would be a valid one. The canonical decl would be still marked as target<br>
of friend decl, as I'm understanding, and I must check the visibility because even though the non canonical friend<br>declaration may be visible, the canonical may be not, but the code would still be valid. Like this one for example:<br>
<br>namespace boost {<br> template<typename T> struct shared_ptr {<br> template<typename U> friend struct unique_ptr; // 1<br> };<br>}<br><br>int main() {<br> boost::shared_ptr<int> p1;<br>
boost::| // 1 is visible, 2 not, so only boost::shared_ptr must show up<br>}<br><br>namespace boost {<br> template<typename T> struct unique_ptr { // 2 canonical<br> };<br>}<br><br><br><br>So the friend namespace check must exist, because of the current issue, its intent is for applying such fix when it's the<br>
case to do so, solely in lookups that involve friends.<br><br>And the visibility check exists because of the previous explanation.<br><br>I must get the canonical decl target of friending, so to suggest a completion option with canonical parameters, and not the<br>
ones of the friend declaration.<br><br>So,... in this process of thought, I still can't remove checks of the patch. I hope to have explained it better.<br>Of course, this process of thought may be completely equivocated.<br>
<br>Regards,<br>Francisco<br><div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-16 13:28 GMT-03:00 Francisco Lopes <span dir="ltr"><<a href="mailto:francisco.mailing.lists@oblita.com" target="_blank">francisco.mailing.lists@oblita.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div>I'll put in words what I was trying to do in this fix. Sorry because it has been so long that I'm reviewing it again.<br>
<br></div><div><div class="h5">Please check this for the reason of such fix: <a href="http://llvm.org/bugs/show_bug.cgi?id=13699#c9" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=13699#c9</a><br>
<br></div></div></div><div><div class="h5">- I'm checking whether it's a friend because I still want to get the friend's target as a result if it's ok to do so.<br><br>(The current codebase has an issue doing this as can be checked in the issue tracker, and here I'm going for<br>
</div></div></div><div><div class="h5"><div>the easier fix by changing the code completion source solely, which is what's affecting me. I'm not sure whether<br></div><div>this friend lookup thing problem requires a major change in other parts of the codebase, which it may, so<br>
</div><div>I opted for the minor fix that would solve this for code completion at last.)<br><br></div><div>- The Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type check may be spurious in the original call of<br>AddResult, but as can be seen, I'm gonna do a recursive call to it, and my intent was to so that, at this second<br>
</div><div>direct call to AddResult, such check would be a valid one. The canonical decl would be still marked as target<br></div><div>of friend decl, as I'm understanding, and I must check the visibility because even though the non canonical friend<br>
</div><div>declaration may be visible, the canonical may be not, but the code would still be valid. Like this one for example:<br><br>namespace boost {<br> template<typename T> struct shared_ptr {<br> template<typename U> friend struct unique_ptr; // 1<br>
};<br>}<br><br>int main() {<br> boost::shared_ptr<int> p1;<br></div><div> boost::| // 1 is visible, 2 not, so only boost::shared_ptr must show up<br></div><div>}<br><br>namespace boost {<br> template<typename T> struct unique_ptr { // 2 canonical<br>
};<br>}<br><br><br><br></div><div>So the friend namespace check must exist, because of the current issue, its intent is for applying such fix when it's the<br>case to do so, solely in lookups that involve friends.<br>
<br></div><div>And the visibility check exists because of the previous explanation.<br><br></div><div>I must get the canonical decl target of friending, so to suggest a completion option with canonical parameters, and not the<br>
ones of the friend declaration.<br><br></div><div>So,... in this process of thought, I still can't remove checks of the patch. I hope to have explained it better.<br></div><div>Of course, this process of thought may be completely equivocated.<br>
<br></div><div>Regards,<br></div>Francisco<br><br></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-14 22:53 GMT-03:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<div>
<div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra">Hmm, I've looked into this a bit more, and it's unclear to me why we have an IDNS check here at all. If LookupVisibleDecls is working properly, it should only return declarations that are actually visible anyway.</div>
<div><div>
<div class="gmail_extra">
<br><div class="gmail_quote">On Mon, Apr 14, 2014 at 1:15 PM, Francisco Lopes <span dir="ltr"><<a href="mailto:francisco.mailing.lists@oblita.com" target="_blank">francisco.mailing.lists@oblita.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div>Hi Richard, as can be checked in the bug history, there's a side effect if I don't call getCanonicalDecl. Also, this patch fixes an issue that's specially related to template friends and lookup. Please take a look at the history, since it looks like following your advices, I would end up with regressions.<br>
</div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-11 17:10 GMT-03:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<div>
<div><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>On Fri, Mar 14, 2014 at 6:59 AM, Francisco Lopes <span dir="ltr"><<a href="mailto:francisco.mailing.lists@oblita.com" target="_blank">francisco.mailing.lists@oblita.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div>hi, thanks for the comment but, for example, as I want to add the call to getCanonicalDecl<br>
for this situation of friends solely, don't I need to check whether it's in friend name space too?<br>
<br></div>I'm not sure whether you meant to replace the two first checks, or just the second.<br></div></div></blockquote><div><br></div></div><div>I meant to replace all the checks. I don't see why you would want to call getCanonicalDecl here, or why you'd care whether the name has ever been declared as a friend. All you should check is, is the name visible now?</div>
<div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div></div>Regards.<br><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">
2014-03-13 20:59 GMT-03:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<div><div><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Your visibility check seems more complex than necessary. I think this should do what you want:<div>
<br>
</div>
<div>if (ND->getMostRecentDecl()->isInIdentifierNamespace(Decl::IDNS_Ordinary | Decl::IDNS_Tag))</div>
<div> // visible</div></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div>On Wed, Mar 12, 2014 at 2:12 PM, Francisco Lopes <span dir="ltr"><<a href="mailto:francisco.mailing.lists@oblita.com" target="_blank">francisco.mailing.lists@oblita.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div dir="ltr">Ping<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">
<div>2014-03-07 14:47 GMT-03:00 Francisco Lopes <span dir="ltr"><<a href="mailto:francisco.mailing.lists@oblita.com" target="_blank">francisco.mailing.lists@oblita.com</a>></span>:<br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><div><div><div><div><div>Hi,<div><div><br>attached is a patch that tries to fix <a href="http://llvm.org/bugs/show_bug.cgi?id=13699" target="_blank">libclang bug 13699</a>.<br>
</div></div></div>Please review.<span><font color="#888888"><br><br></font></span></div><span><font color="#888888">--<br></font></span></div><span><font color="#888888">Francisco Lopes<br>
<br></font></span></div><div><div>PS:<br>I have requested commit access in late 2012 but never made a test commit or anything.<br><br></div></div></div><div><div>At the time I have received from Chris Lattner:<br>
"I'm sorry for the delay, I've been fighting mailing list issues.<br>
Commit
after approval access is granted. Please try a test commit!"<br><br></div></div></div><div><div>I'm not sure whether it's still valid.<br></div></div></div>
</blockquote></div><br></div>
<br></div></div><div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></div></blockquote></div><br></div>
</blockquote></div></div></div><br></div>
</blockquote></div></div><br></div></div>
</blockquote></div></div></div><br></div>
</blockquote></div><br></div></div></div></div>
</blockquote></div></div></div><br></div>
</blockquote></div><br></div></div></div>