[cfe-dev] RFC: What does this matcher mean to you?

[Vane, Edwin]

> -----Original Message-----
> From: Manuel Klimek [mailto:klimek at google.com]
> Sent: Thursday, June 06, 2013 10:58 AM
> To: Daniel Jasper
> Cc: Vane, Edwin; Gábor Kozár; Clang Dev List (cfe-dev at cs.uiuc.edu)
> Subject: Re: [cfe-dev] RFC: What does this matcher mean to you?
> 
> On Thu, Jun 6, 2013 at 4:47 PM, Daniel Jasper <djasper at google.com> wrote:
> 
> 
> 	I don't understand the confusion. To me, it feels kind of natural now.
> 
> 	Consider a matcher similar to what we have looked at above:
> 
> 	functionDecl(forEachDescendant(callExpr(callee(functionDecl().bind("f")
> ))), hasName("SomeFunction"));
> 
> 
> 	This is very similar to my second example case. The callback will be
> called once for each CallExpr in the function "SomeFunction" with the called
> function bound to "f". Now, if we change this to:
> 
> 
> 	functionDecl(forEachDescendant(callExpr(callee(functionDecl().bind("f")
> ))), equalsBoundNode("f"));
> 
> 
> 	This equalsBoundNode matcher will behave exactly like the hasName()-
> matcher. Thus, the callback will be called once for each CallExpr where a
> function calls itself recursively.
> 
> 
> 	Or am I missing something?
> 
> 
> Yes, there's a huge difference. (I agree with the proposed behavior tho)
> (1) is fixed with SomeFunction - that is, either all results from
> forEachDescendant apply, or none
> (2) means that the result set created by forEachDescendant is filtered down by
> equalsBoundNode

I think maybe this second matcher is starting to shine light on the filtering problem for me. In the second matcher, in the state it's currently implemented, the callback would be called N times for each of N CallExpr's as long as at least one of these CallExpr calls is a recursive call. Is that right? I can see why that's not helpful to the user. I guess you'd like the filtering thing to only have the callback called once and only for each recursiveCall right?

> 	On Thu, Jun 6, 2013 at 3:26 PM, Manuel Klimek <klimek at google.com>
> wrote:
> 
> 
> 		On Thu, Jun 6, 2013 at 3:19 PM, Vane, Edwin
> <edwin.vane at intel.com> wrote:
> 
> 
> 
> 
> 			> -----Original Message-----
> 			> From: Manuel Klimek [mailto:klimek at google.com]
> 
> 			> Sent: Wednesday, June 05, 2013 12:24 PM
> 			> To: Vane, Edwin
> 
> 			> Cc: Daniel Jasper; Gábor Kozár; Clang Dev List (cfe-
> dev at cs.uiuc.edu)
> 			> Subject: Re: [cfe-dev] RFC: What does this matcher
> mean to you?
> 
> 			>
> 			> On Wed, Jun 5, 2013 at 5:53 PM, Vane, Edwin
> <edwin.vane at intel.com> wrote:
> 			>
> 			>
> 			>
> 			>
> 			>       > -----Original Message-----
> 			>       > From: Daniel Jasper
> [mailto:djasper at google.com]
> 			>       > Sent: Tuesday, June 04, 2013 5:22 AM
> 			>       > To: Vane, Edwin
> 			>       > Cc: Manuel Klimek; Gábor Kozár; Clang Dev List
> (cfe-dev at cs.uiuc.edu)
> 			>       > Subject: Re: [cfe-dev] RFC: What does this
> matcher mean to you?
> 			>       >
> 			>
> 			>       > I think this question is basically not about
> equalsBoundNodes() at all.
> 			>       >
> 			>       > The questions that needs answering are:
> 			>       > - What should
> stmt(forEachDescendant(varDecl().bind("var")),
> 			>       > forEachDescendant(declRefExpr().bind("decl")))
> do?
> 			>       > - What should
> stmt(forEachDescendant(varDecl().bind("var")),
> 			>       > has(declRefExpr().bind("decl"))) do?
> 			>       >
> 			>       > I think the first should call the callback for each
> pair (VarDecl,
> 			> DeclRefExpr)
> 			>       > under each Stmt. And the second should call the
> callback once for
> 			> each VarDecl
> 			>       > under a Stmt that has a DeclRefExpr-child.
> 			>
> 			>
> 			>       I agree. This is in line with the current definition
> and behaviour of the
> 			> involved matchers. I interpret the 'filtering' behaviour
> as something new that, if
> 			> we want to implement it, needs a different matcher
> to expose. I think the
> 			> equalsBoundNode() matcher is independent of the
> filtering behaviour.
> 			>
> 			>
> 			>
> 			> I think with the proposed change to how we match,
> the equalsBoundNode
> 			> would naturally fit in as filtering matcher...
> 
> 
> 			What do you mean by proposed change to *how* we
> match? The equalsBoundNode() matcher was meant to be just another matcher.
> If we want to use it for filtering I still think we need a different matcher to
> expose that functionality. Otherwise we're changing the semantics of existing
> matchers.
> 
> 			So given we seem to agree on how Daniel's two
> matchers behave, how does this help us with equalsBoundNode()?
> 
> 
> 
> 		To me equalsBoundNode is not just another matcher - many
> details on how bound nodes are handled get exposed with equalsBoundNode,
> because the relevant information was not accessible at those points before.
> 
> 		For example, before equalsBoundNode, whether a matcher
> passed for failed did not depend on the set of bound nodes at that point - that's
> a very big change, for example, we need to take it into account when doing
> memoization, otherwise we'll create inconsistent results.
> 
> 		Also, I don't think that the proposed non-filtering behavior of
> equalsBoundNode is user friendly. I think that having it be a filtering matcher
> makes more sense, and that other filtering matchers are a different topic (as, in
> principle, all matchers are already filtering matchers on predicates of nodes,
> apart from the forEach matchers)
> 
> 
> 
> 			--
> 			Edwin Vane
> 			  Software Developer
> 			  Intel of Canada, Inc.
> 
> 
> 
> 
> 
>