Re: [cdt-dev] use of hasMultipleArgs() in TemplateIdStrategy

[Nathan Ridge <zeratul976@xxxxxxxxxxx>]

After taking a closer look at how the use of hasMultipleArgs() fixed bug 363609, I was able to come up with a relatively simple solution.

The key realization was that TemplateIdStrategy.setNextAlternative() was being called in two different scenarios: when parsing the previous alternative was successful, and when it was not. 

The hasMultipleArgs() optimization seems to be valid in the case when the previous parse was successful, but not when it wasn't (if the template-id with multiple args isn't part of a successful parse tree, it's useless). By limiting the optimization to the valid case, I was able to both fix bug 445177, and not regress bug 363609.

Patch is on gerrit. I also took the opportunity to add some explanatory comments to TemplateIdStrategy, while its workings are fresh in my mind.

Regards,
Nate
________________________________
> Date: Thu, 5 Feb 2015 11:12:41 -0800 
> From: eclipse.sprigogin@xxxxxxxxx 
> To: cdt-dev@xxxxxxxxxxx 
> CC: markus.schorn@xxxxxxxxxxxxx 
> Subject: Re: [cdt-dev] use of hasMultipleArgs() in TemplateIdStrategy 
>  
> The intent behind the hasMultipleArgs() condition is to prevent  
> combinatorial explosion. It looks like we need a smarter way to limit  
> the number of parsing variants we are considering. One way to do that  
> is to introduce a limiting counter similar to the ones used to protect  
> against infinite recursion. This is probably easier said than done. I  
> have to admit that I haven't investigated how hard it is to do in the  
> existing code. 
>  
> My 2c 
> -sergey 
>  
> On Tue, Dec 16, 2014 at 5:12 PM, Nathan Ridge  
> <zeratul976@xxxxxxxxxxx<mailto:zeratul976@xxxxxxxxxxx>> wrote: 
> Hi, 
>  
> I'm investigating bug 445177, where CDT fails to parse the following  
> declaration: 
>  
> a<b<c,b<c>::*member; 
>  
> The only valid parse here is 
>  
> a<(b<c),(b<c)>::*member, 
>  
> i.e. the first '<' starts a template-parameter-list while other two '<'  
> are less-than operators. 
>  
> However, CDT does not consider this parse. The reason is that in  
> TemplateIdStrategy, we skip over the alternative that would interpret  
> the first '<' as opening a template-parameter-list but the second '<'  
> as less-than. 
>  
> This is because in setNextAlternative(), the code that would consider  
> the second '<' as less-than is conditioned on  
> '!hasMultipleArgs(names[--nameLen])', where 'names[--nameLen]' in this  
> case is the first 'b', and 'hasMultipleArgs()' checks whether the name  
> is part of a template-id with multiple template arguments. 
>  
> As it happens, the check fails, because while considering the previous  
> alternative, the first 'b' was parsed as part of the template-id  
> 'b<c,(b<c)>', which has two arguments. 
>  
> I see that the use of hasMultipleArgs() was added in bug 363609 for  
> performance reasons. However, is there a correctness argument for  
> conditioning on hasMultipleArgs() here? The above example would lead me  
> to believe 'no', but perhaps I'm missing something. 
>  
> cc'ing Markus, who originally wrote this code. 
>  
> Thanks, 
> Nate 
>  
> _______________________________________________ 
> cdt-dev mailing list 
> cdt-dev@xxxxxxxxxxx<mailto:cdt-dev@xxxxxxxxxxx> 
> To change your delivery options, retrieve your password, or unsubscribe  
> from this list, visit 
> https://dev.eclipse.org/mailman/listinfo/cdt-dev 
>  
>  
> _______________________________________________ cdt-dev mailing list  
> cdt-dev@xxxxxxxxxxx To change your delivery options, retrieve your  
> password, or unsubscribe from this list, visit  
> https://dev.eclipse.org/mailman/listinfo/cdt-dev