Much simpler, thanks! 2015-01-15 0:28 GMT-02:00 Richard Smith <[email protected]>:
> Fixed in a simpler way in r226083. > > On Mon, Jan 5, 2015 at 4:29 PM, Francisco Lopes < > [email protected]> wrote: > >> ping >> >> 2014-12-04 7:09 GMT-02:00 Manuel Klimek <[email protected]>: >> >> Richard, according to the YCM owner, more people hit this. Is there a >>> problem with the patch? Anything else we need to do to drive this forward? >>> >>> Thanks, >>> /Manuel >>> >>> >>> On Wed Apr 30 2014 at 10:05:18 PM Francisco Lopes < >>> [email protected]> wrote: >>> >>>> 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 >>>> >>> >>> _______________________________________________ >>> 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
