Community
Participate
Working Groups
I am trying to use CDT and its visitor infrastructure to parse a C++ project. It seems that the visitor methods are too few and too "high level" For example the same visit(...) method is called whether it is a type declaration/class/declaration/function declaration: visit(IASTDeclSpecifier) This forces the user of the visitor to do a lot of additional checks (instanceof) in these too generic methods. A possible cause would be that too few accept(Visitor) methods have been implemented in the different ASTNode subclasses ... ? I would be willing to help improve this part (if my diagnosis is correct) as it is really painful to use the visitor as it is nicolas
It's not clear what is being proposed in comment #0. ASTVisitor.visit methods are distinguished by the type of the argument. Is the proposal is to somehow change that?
thanks for looking into it. The issue is that currently, the visitor does not seem to be able to receive visit(CPPASTNamedTypeSpecifier) or visit(IASTFunctionDeclarator) for example. I was wondering why? The problem is that it forces one to put instanceof the visit method to check what subtype of the visit method one is actually dealing with. A possible solution/proposal was to put more accept(aVisitor) in all classes AST classes so that the visitor could receive such calls.
(In reply to Nicolas Anquetil from comment #2) The current minimalistic approach used by ASTVisitor has a significant advantage that it is always easy to anticipate what method will be called for any AST node. Specializing visit and leave methods by of the parameter would introduce additional confusion and complexity. Consider, for example, visit(IASTStatement). This method would have to be replaced by more than 20 separate methods. It wouldn't be immediately obvious what method is called for a compound statement since there would be three candidates to choose from. Using leaf nodes in the node type hierarchy for visit methods may be convenient for code that needs to distinguish between all different statement types but very inconvenient for code that does not.
Yes I thought about this point. My proposal would be to add in the default visitor (ASTVisitor I guess, or maybe ASTGenericVisitor?) specialized visit methods that would "point it back" to the generic method. Thus, considering your example, one could define in the default visitor: public int visit(IASTWhileStatement node) { return this.visit( (IASTStatement)node); } That way code inheriting from the default visitor would not see any change, but code needing more specialized behaviour could override visit(IASTWhileStatement node) ... nicolas
(In reply to Nicolas Anquetil from comment #4) Somebody who writes a specialized visitor would still have to be acutely aware of node type hierarchy, e.g. ICPPASTCompoundStatement -> IASTCompoundStatement -> IASTStatement. Moreover, new branches of the type hierarchy may be introduced in future and break previously valid assumptions. You may say that the same is true for the chained "if" statements with instanceof checks, but at least ASTVisitor is not responsible for that.
I am afraid I must disagree with you: "Somebody who writes a specialized visitor would still have to be acutely aware of node type hierarchy" 1- Anybody writing a visitor must already know more or less what the type hierarchy is. 2- And same thing goes for the current situation because one needs to know to what super type a given node is "mapped" to know where to put the right instanceof moreover, the example you give (ICPPASTCompoundStatement -> IASTCompoundStatement -> IASTStatement) show that anyway, the hierarchy is quite natural and intuitive. So it does not seem it would be a real issue. "new branches of the type hierarchy may be introduced in future and break previously valid assumptions". 1- one may assume that not so many new AST nodes will appear in C in the future (even though I agree it can happen) 2- but as you say, if that happens, anyway both solution will have to deal with the problem. Anyway, I think it defeats the very purpose of the Visitor pattern not to have all the visiting methods available (at least for all the available Java interfaces).
(In reply to Nicolas Anquetil from comment #4) > My proposal would be to add in the default visitor (ASTVisitor I guess, or > maybe ASTGenericVisitor?) specialized visit methods that would "point it > back" to the generic method. Interestingly, we have a class called ASTGenericVisitor, which is similar to what you describe, just a lot more generic: it "points back" all of the existing visit() methods to a genericVisit(IASTNode) method. Anyways, I would expect that we would consider patches that enhance our AST visitors subject to the following constraints: - They do not break the behaviour of public API classes like ASTVisitor. - They do not make existing uses of AST visitors inside CDT code significantly more verbose or complex.
