Skip to content

Commit

Permalink
separate ASTVisitor.visit for ExpressionNode
Browse files Browse the repository at this point in the history
abstract classes should not have visitor functions, but rather only do
dynamic dispatch. This becomes more important later with dlang-community#409
  • Loading branch information
WebFreak001 committed Mar 23, 2023
1 parent 98bf0f4 commit edd03fa
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 42 deletions.
33 changes: 29 additions & 4 deletions src/dparse/ast.d
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,32 @@ enum DeclarationListStyle : ubyte
*/
abstract class ASTVisitor
{

/** */
deprecated("Don't use or override visit(ExpressionNode). For usage: dynamicDispatch(ExpressionNode) is equivalent; "
~ "for overriding: you should probably override more specific cases. If you need to override to inject some "
~ "before/after code for all cases, make sure to call `super.dynamicDispatch(n)` instead of `n.accept(this)`!")
void visit(const ExpressionNode n)
{
switch (typeMap[typeid(n)])
dynamicDispatch(n);
}

/**
* Looks at the runtime type of `n`, then calls the appropriate `visit`
* method at runtime.
*
* Rule of thumb: when the type is an abstract class, use `dynamicDispatch`,
* otherwise use `visit`.
*
* For templated calls:
* ---
* static if (__traits(isAbstractClass, typeof(node)))
* visitor.dynamicDispatch(node);
* else
* visitor.visit(node);
* ---
*/
void dynamicDispatch(const ExpressionNode n)
{
switch (typeMap.get(typeid(n), 0))
{
case 1: visit(cast(AddExpression) n); break;
case 2: visit(cast(AndAndExpression) n); break;
Expand Down Expand Up @@ -383,11 +404,15 @@ template visitIfNotNull(fields ...)
{
static if (typeof(fields[0]).stringof[$ - 2 .. $] == "[]")
{
static if (__traits(hasMember, typeof(fields[0][0]), "classinfo"))
static if (__traits(isAbstractClass, typeof(fields[0][0])))
immutable visitIfNotNull = "foreach (i; " ~ fields[0].stringof ~ ") if (i !is null) visitor.dynamicDispatch(i);\n";
else static if (__traits(hasMember, typeof(fields[0][0]), "classinfo"))
immutable visitIfNotNull = "foreach (i; " ~ fields[0].stringof ~ ") if (i !is null) visitor.visit(i);\n";
else
immutable visitIfNotNull = "foreach (i; " ~ fields[0].stringof ~ ") visitor.visit(i);\n";
}
else static if (__traits(isAbstractClass, typeof(fields[0])))
immutable visitIfNotNull = "if (" ~ fields[0].stringof ~ " !is null) visitor.dynamicDispatch(" ~ fields[0].stringof ~ ");\n";
else static if (__traits(hasMember, typeof(fields[0]), "classinfo"))
immutable visitIfNotNull = "if (" ~ fields[0].stringof ~ " !is null) visitor.visit(" ~ fields[0].stringof ~ ");\n";
else static if (is(Unqual!(typeof(fields[0])) == Token))
Expand Down
76 changes: 38 additions & 38 deletions src/dparse/astprinter.d
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<addExpression operator=\"", str(addExpression.operator), "\">");
output.writeln("<left>");
visit(addExpression.left);
dynamicDispatch(addExpression.left);
output.writeln("</left>");
if (addExpression.right !is null)
{
output.writeln("<right>");
visit(addExpression.right);
dynamicDispatch(addExpression.right);
output.writeln("</right>");
}
output.writeln("</addExpression>");
Expand Down Expand Up @@ -58,12 +58,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<andAndExpression>");
output.writeln("<left>");
visit(andAndExpression.left);
dynamicDispatch(andAndExpression.left);
output.writeln("</left>");
if (andAndExpression.right !is null)
{
output.writeln("<right>");
visit(andAndExpression.right);
dynamicDispatch(andAndExpression.right);
output.writeln("</right>");
}
output.writeln("</andAndExpression>");
Expand All @@ -73,12 +73,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<andExpression>");
output.writeln("<left>");
visit(andExpression.left);
dynamicDispatch(andExpression.left);
output.writeln("</left>");
if (andExpression.right !is null)
{
output.writeln("<right>");
visit(andExpression.right);
dynamicDispatch(andExpression.right);
output.writeln("</right>");
}
output.writeln("</andExpression>");
Expand Down Expand Up @@ -191,13 +191,13 @@ class XMLPrinter : ASTVisitor
if (caseRangeStatement.low !is null)
{
output.writeln("<low>");
visit(caseRangeStatement.low);
dynamicDispatch(caseRangeStatement.low);
output.writeln("</low>");
}
if (caseRangeStatement.high !is null)
{
output.writeln("<high>");
visit(caseRangeStatement.high);
dynamicDispatch(caseRangeStatement.high);
output.writeln("</high>");
}
if (caseRangeStatement.declarationsAndStatements !is null)
Expand Down Expand Up @@ -295,7 +295,7 @@ class XMLPrinter : ASTVisitor
if (deprecated_.assignExpression !is null)
{
output.writeln("<deprecated>");
visit(deprecated_.assignExpression);
dynamicDispatch(deprecated_.assignExpression);
output.writeln("</deprecated>");
}
else
Expand All @@ -320,7 +320,7 @@ class XMLPrinter : ASTVisitor
visit(enumMember.type);
output.write("<name>", enumMember.name.text, "</name>");
if (enumMember.assignExpression !is null)
visit(enumMember.assignExpression);
dynamicDispatch(enumMember.assignExpression);
output.writeln("</anonymousEnumMember>");
}

Expand All @@ -336,10 +336,10 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<equalExpression operator=\"", str(equalExpression.operator), "\">");
output.writeln("<left>");
visit(equalExpression.left);
dynamicDispatch(equalExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(equalExpression.right);
dynamicDispatch(equalExpression.right);
output.writeln("</right>");
output.writeln("</equalExpression>");
}
Expand Down Expand Up @@ -475,10 +475,10 @@ class XMLPrinter : ASTVisitor
else
output.writeln("<identityExpression operator=\"is\">");
output.writeln("<left>");
visit(identityExpression.left);
dynamicDispatch(identityExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(identityExpression.right);
dynamicDispatch(identityExpression.right);
output.writeln("</right>");
output.writeln("</identityExpression>");
}
Expand Down Expand Up @@ -541,10 +541,10 @@ class XMLPrinter : ASTVisitor
else
output.writeln("<inExpression operator=\"in\">");
output.writeln("<left>");
visit(inExpression.left);
dynamicDispatch(inExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(inExpression.right);
dynamicDispatch(inExpression.right);
output.writeln("</right>");
output.writeln("</inExpression>");
}
Expand Down Expand Up @@ -613,10 +613,10 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<keyValuePair>");
output.writeln("<key>");
visit(keyValuePair.key);
dynamicDispatch(keyValuePair.key);
output.writeln("</key>");
output.writeln("<value>");
visit(keyValuePair.value);
dynamicDispatch(keyValuePair.value);
output.writeln("</value>");
output.writeln("</keyValuePair>");
}
Expand Down Expand Up @@ -676,12 +676,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<mulExpression operator=\"", str(mulExpression.operator), "\">");
output.writeln("<left>");
visit(mulExpression.left);
dynamicDispatch(mulExpression.left);
output.writeln("</left>");
if (mulExpression.right !is null)
{
output.writeln("<right>");
visit(mulExpression.right);
dynamicDispatch(mulExpression.right);
output.writeln("</right>");
}
output.writeln("</mulExpression>");
Expand All @@ -691,12 +691,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<orOrExpression>");
output.writeln("<left>");
visit(orOrExpression.left);
dynamicDispatch(orOrExpression.left);
output.writeln("</left>");
if (orOrExpression.right !is null)
{
output.writeln("<right>");
visit(orOrExpression.right);
dynamicDispatch(orOrExpression.right);
output.writeln("</right>");
}
output.writeln("</orOrExpression>");
Expand Down Expand Up @@ -727,12 +727,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<powExpression>");
output.writeln("<left>");
visit(powExpression.left);
dynamicDispatch(powExpression.left);
output.writeln("</left>");
if (powExpression.right !is null)
{
output.writeln("<right>");
visit(powExpression.right);
dynamicDispatch(powExpression.right);
output.writeln("</right>");
}
output.writeln("</powExpression>");
Expand All @@ -743,10 +743,10 @@ class XMLPrinter : ASTVisitor
output.writeln("<relExpression operator=\"",
xmlAttributeEscape(str(relExpression.operator)), "\">");
output.writeln("<left>");
visit(relExpression.left);
dynamicDispatch(relExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(relExpression.right);
dynamicDispatch(relExpression.right);
output.writeln("</right>");
output.writeln("</relExpression>");
}
Expand All @@ -768,10 +768,10 @@ class XMLPrinter : ASTVisitor
output.writeln("<shiftExpression operator=\"",
xmlAttributeEscape(str(shiftExpression.operator)), "\">");
output.writeln("<left>");
visit(shiftExpression.left);
dynamicDispatch(shiftExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(shiftExpression.right);
dynamicDispatch(shiftExpression.right);
output.writeln("</right>");
output.writeln("</shiftExpression>");
}
Expand Down Expand Up @@ -804,7 +804,7 @@ class XMLPrinter : ASTVisitor
if (templateAliasParameter.colonExpression !is null)
{
output.writeln("<specialization>");
visit(templateAliasParameter.colonExpression);
dynamicDispatch(templateAliasParameter.colonExpression);
output.writeln("</specialization>");
}
else if (templateAliasParameter.colonType !is null)
Expand All @@ -817,7 +817,7 @@ class XMLPrinter : ASTVisitor
if (templateAliasParameter.assignExpression !is null)
{
output.writeln("<default>");
visit(templateAliasParameter.assignExpression);
dynamicDispatch(templateAliasParameter.assignExpression);
output.writeln("</default>");
}
else if (templateAliasParameter.assignType !is null)
Expand Down Expand Up @@ -962,14 +962,14 @@ class XMLPrinter : ASTVisitor
if (typeSuffix.high !is null)
{
output.writeln("<low>");
visit(typeSuffix.low);
dynamicDispatch(typeSuffix.low);
output.writeln("</low>");
output.writeln("<high>");
visit(typeSuffix.high);
dynamicDispatch(typeSuffix.high);
output.writeln("</high>");
}
else
visit(typeSuffix.low);
dynamicDispatch(typeSuffix.low);
output.writeln("</typeSuffix>");
}
}
Expand Down Expand Up @@ -1041,12 +1041,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<xorExpression>");
output.writeln("<left>");
visit(xorExpression.left);
dynamicDispatch(xorExpression.left);
output.writeln("</left>");
if (xorExpression.right !is null)
{
output.writeln("<right>");
visit(xorExpression.right);
dynamicDispatch(xorExpression.right);
output.writeln("</right>");
}
output.writeln("</xorExpression>");
Expand All @@ -1058,15 +1058,15 @@ class XMLPrinter : ASTVisitor
if (index.high)
{
output.writeln("<low>");
visit(index.low);
dynamicDispatch(index.low);
output.writeln("</low>");

output.writeln("<high>");
visit(index.high);
dynamicDispatch(index.high);
output.writeln("</high>");
}
else
visit(index.low);
dynamicDispatch(index.low);
output.writeln("</index>");
}

Expand Down

0 comments on commit edd03fa

Please sign in to comment.