PING. Does my previous statements make any sense? I'm not so into this patch anymore, but locally at my machine, it solves the issue without side-effects. I believe some measure regarding this issue could be taken, given all the information I've given.
Regards. 2014-04-16 13:41 GMT-03:00 Francisco Lopes < [email protected]>: > Reposting, since last one looks like didn't formatted well: > > 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. > > > Please check this for the reason of such fix: > http://llvm.org/bugs/show_bug.cgi?id=13699#c9 > > - 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. > > (The current codebase has an issue doing this as can be checked in the > issue tracker, and here I'm going for > the easier fix by changing the code completion source solely, which is > what's affecting me. I'm not sure whether > this friend lookup thing problem requires a major change in other parts of > the codebase, which it may, so > I opted for the minor fix that would solve this for code completion at > last.) > > - The Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type check may be > spurious in the original call of > 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 > direct call to AddResult, such check would be a valid one. The canonical > decl would be still marked as target > of friend decl, as I'm understanding, and I must check the visibility > because even though the non canonical friend > declaration may be visible, the canonical may be not, but the code would > still be valid. Like this one for example: > > namespace boost { > template<typename T> struct shared_ptr { > template<typename U> friend struct unique_ptr; // 1 > }; > } > > int main() { > boost::shared_ptr<int> p1; > boost::| // 1 is visible, 2 not, so only boost::shared_ptr must show up > } > > namespace boost { > template<typename T> struct unique_ptr { // 2 canonical > }; > } > > > > So the friend namespace check must exist, because of the current issue, > its intent is for applying such fix when it's the > case to do so, solely in lookups that involve friends. > > And the visibility check exists because of the previous explanation. > > I must get the canonical decl target of friending, so to suggest a > completion option with canonical parameters, and not the > ones of the friend declaration. > > So,... in this process of thought, I still can't remove checks of the > patch. I hope to have explained it better. > Of course, this process of thought may be completely equivocated. > > Regards, > Francisco > > > 2014-04-16 13:28 GMT-03:00 Francisco Lopes < > [email protected]>: > > 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. >> >> Please check this for the reason of such fix: >> http://llvm.org/bugs/show_bug.cgi?id=13699#c9 >> >> - 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. >> >> (The current codebase has an issue doing this as can be checked in the >> issue tracker, and here I'm going for >> the easier fix by changing the code completion source solely, which is >> what's affecting me. I'm not sure whether >> this friend lookup thing problem requires a major change in other parts >> of the codebase, which it may, so >> I opted for the minor fix that would solve this for code completion at >> last.) >> >> - The Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type check may be >> spurious in the original call of >> 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 >> direct call to AddResult, such check would be a valid one. The canonical >> decl would be still marked as target >> of friend decl, as I'm understanding, and I must check the visibility >> because even though the non canonical friend >> declaration may be visible, the canonical may be not, but the code would >> still be valid. Like this one for example: >> >> namespace boost { >> template<typename T> struct shared_ptr { >> template<typename U> friend struct unique_ptr; // 1 >> }; >> } >> >> int main() { >> boost::shared_ptr<int> p1; >> boost::| // 1 is visible, 2 not, so only boost::shared_ptr must show >> up >> } >> >> namespace boost { >> template<typename T> struct unique_ptr { // 2 canonical >> }; >> } >> >> >> >> So the friend namespace check must exist, because of the current issue, >> its intent is for applying such fix when it's the >> case to do so, solely in lookups that involve friends. >> >> And the visibility check exists because of the previous explanation. >> >> I must get the canonical decl target of friending, so to suggest a >> completion option with canonical parameters, and not the >> ones of the friend declaration. >> >> So,... in this process of thought, I still can't remove checks of the >> patch. I hope to have explained it better. >> Of course, this process of thought may be completely equivocated. >> >> Regards, >> Francisco >> >> >> >> 2014-04-14 22:53 GMT-03:00 Richard Smith <[email protected]>: >> >> 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. >>> >>> On Mon, Apr 14, 2014 at 1:15 PM, Francisco Lopes < >>> [email protected]> wrote: >>> >>>> 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. >>>> >>>> >>>> 2014-04-11 17:10 GMT-03:00 Richard Smith <[email protected]>: >>>> >>>> On Fri, Mar 14, 2014 at 6:59 AM, Francisco Lopes < >>>>> [email protected]> wrote: >>>>> >>>>>> hi, thanks for the comment but, for example, as I want to add the >>>>>> call to getCanonicalDecl >>>>>> for this situation of friends solely, don't I need to check whether >>>>>> it's in friend name space too? >>>>>> >>>>>> I'm not sure whether you meant to replace the two first checks, or >>>>>> just the second. >>>>>> >>>>> >>>>> 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? >>>>> >>>>> Regards. >>>>>> >>>>>> >>>>>> >>>>>> 2014-03-13 20:59 GMT-03:00 Richard Smith <[email protected]>: >>>>>> >>>>>> Your visibility check seems more complex than necessary. I think this >>>>>>> should do what you want: >>>>>>> >>>>>>> if >>>>>>> (ND->getMostRecentDecl()->isInIdentifierNamespace(Decl::IDNS_Ordinary | >>>>>>> Decl::IDNS_Tag)) >>>>>>> // visible >>>>>>> >>>>>>> >>>>>>> On Wed, Mar 12, 2014 at 2:12 PM, Francisco Lopes < >>>>>>> [email protected]> wrote: >>>>>>> >>>>>>>> Ping >>>>>>>> >>>>>>>> >>>>>>>> 2014-03-07 14:47 GMT-03:00 Francisco Lopes < >>>>>>>> [email protected]>: >>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> attached is a patch that tries to fix libclang bug >>>>>>>>> 13699<http://llvm.org/bugs/show_bug.cgi?id=13699> >>>>>>>>> . >>>>>>>>> Please review. >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Francisco Lopes >>>>>>>>> >>>>>>>>> PS: >>>>>>>>> I have requested commit access in late 2012 but never made a test >>>>>>>>> commit or anything. >>>>>>>>> >>>>>>>>> At the time I have received from Chris Lattner: >>>>>>>>> "I'm sorry for the delay, I've been fighting mailing list issues. >>>>>>>>> Commit after approval access is granted. Please try a test >>>>>>>>> commit!" >>>>>>>>> >>>>>>>>> I'm not sure whether it's still valid. >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> _______________________________________________ >>>>>>>> cfe-commits mailing list >>>>>>>> [email protected] >>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
