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
