Skip to content

Compilation time improvements for PolymodBaseClassMacro #523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AbnormalPoof merged 6 commits into from
Oct 5, 2026
Merged

Compilation time improvements for PolymodBaseClassMacro #523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0dcb9ee
Move the script lookup of generated function overrides into PolymodSc…
EliteMasterEric Oct 2, 2026
6e9dbe8
Stop removing inline call sites from every class, since generated ove…
EliteMasterEric Oct 2, 2026
3ffe996
Skip classes before retrieving their fields, and return null instead …
EliteMasterEric Oct 2, 2026
129d3ac
Check for the _asc field with root class metadata, instead of retriev…
EliteMasterEric Oct 2, 2026
8607879
Generate scriptInit as a call to PolymodScriptBridge.instantiate to r…
EliteMasterEric Oct 2, 2026
802ec5a
fix: Don't attempt to make classes with inline constructors scriptable
NotHyper-474 Oct 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions polymod/hscript/PolymodScriptBridge.hx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,21 @@ class PolymodScriptBridge
return null;
}

/**
* Finds the script class which overrides the given function, unless `scriptCallSuper` is skipping it.
* Called at the start of every function generated by `PolymodBaseClassMacro`.
* @param asc The `_asc` field of the instance.
* @param skipAscFrom The `_skipAscFrom` field of the instance.
* @param funcName The name of the function.
* @return The script class to call the function on, or `null` to run the original function.
*/
public static function findOverride(asc:Dynamic, skipAscFrom:Null<Array<String>>, funcName:String):Dynamic
{
if (skipAscFrom != null && skipAscFrom.contains(funcName)) return null;

return findScript(asc, funcName);
}

public static function callOn(scriptCls:Dynamic, funcName:String, funcArgs:Array<Dynamic>):Dynamic
{
if (scriptCls == null) return null;
Expand Down
206 changes: 62 additions & 144 deletions polymod/hscript/_internal/PolymodBaseClassMacro.hx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import Type as HaxeType;

using haxe.macro.Tools;
using StringTools;
using Lambda;

/**
* `PolymodBaseClassMacro` is responsible for adding fields to all classes to make them extendable.
Expand All @@ -30,6 +31,12 @@ class PolymodBaseClassMacro
*/
public static final CPPIA_EXTENDABLE_META:String = ':hscriptExtendable';

/**
* A metadata added to the root classes which received the `_asc` field.
* This lets subclasses check for the field without retrieving the fields of their superclasses.
*/
public static final ASC_ROOT_META:String = ':hscriptAscRoot';

/**
* A list of package name prefixes for which this macro should not be executed.
*/
Expand All @@ -42,35 +49,26 @@ class PolymodBaseClassMacro

public static function buildBaseClass():Array<Field>
{
var cls:ClassType = null;
var pos:Position = Context.currentPos();
var localClass:Null<Ref<ClassType>> = Context.getLocalClass();

try
{
cls = Context.getLocalClass().get();
}
catch (e)
{
// Building wasn't called from a class; skip.
return null;
}
// Building wasn't called from a class; skip.
if (localClass == null) return null;

var cls:ClassType = localClass.get();
var pos:Position = Context.currentPos();

// Make sure the macro doesn't run twice on the class.
if (cls.meta.has(PROCESS_FINISHED_META)) return null;
cls.meta.add(PROCESS_FINISHED_META, [], pos);

// Remove `inline` from function calls, since functions pointing towards `_asc` don't have a final return.
var fields:Array<Field> = Context.getBuildFields().copy();
removeInlinedFunctionCalls(fields);

// Omit being able to extend some classes.
if (cls.isInterface || cls.isAbstract || cls.isExtern || cls.isFinal) return fields;
if (cls.isInterface || cls.isAbstract || cls.isExtern || cls.isFinal) return null;

// Exclude classes that start with an underscore, as they indicate classes that were created through compilation from parameters.
if (cls.name.startsWith('_') || (cls.pack.length > 0 && cls.pack[cls.pack.length - 1].startsWith('_'))) return fields;
if (cls.name.startsWith('_') || (cls.pack.length > 0 && cls.pack[cls.pack.length - 1].startsWith('_'))) return null;

// Disallow generic classes, as they mess with this macro.
if (!HaxeType.enumEq(cls.kind, KNormal)) return fields;
if (!HaxeType.enumEq(cls.kind, KNormal)) return null;

// If a class has type parameters but without any constraints (default values), it wouldn't be possible to extend them on runtime.
for (param in cls.params)
Expand All @@ -81,30 +79,37 @@ class PolymodBaseClassMacro
switch (classType.kind)
{
case KTypeParameter(c) if (c.length == 0):
return fields;
return null;
default:
}
default:
}
}

// Core api classes require type which can't be specified on runtime.
if (cls.meta.has(':coreApi')) return fields;
if (cls.meta.has(':coreApi')) return null;

// Classes specified to act as anonymous structures shouldn't be extended.
if (cls.meta.has(':structInit')) return fields;
if (cls.meta.has(':structInit')) return null;

// :nativeGen makes the class get treated as an extern, so it shouldn't be extended.
if (cls.meta.has(':nativeGen')) return fields;
if (cls.meta.has(':nativeGen')) return null;

var fullClsName:String = formatClassString(cls);
// Disallow extending certain classes.
for (filter in PACKAGE_FILTERS)
{
if (fullClsName.indexOf(filter) == 0) return fields;
if (fullClsName.indexOf(filter) == 0) return null;
}

if (Context.defined('cppia') && !isHostClass(fullClsName) && !cls.meta.has(CPPIA_EXTENDABLE_META)) return fields;
if (Context.defined('cppia') && !isHostClass(fullClsName) && !cls.meta.has(CPPIA_EXTENDABLE_META)) return null;

// Fields are only retrieved once the class is known to be extendable.
// Skipped classes return `null` (no changes), which is much cheaper than resubmitting their unchanged fields.
var fields:Array<Field> = Context.getBuildFields();

var constructorField:Null<Field> = fields.find((fld) -> fld.name == 'new');
if (constructorField != null && constructorField.access.contains(AInline)) return null;

// Check if a class already has one of the fields needed for the scripts before attempting to build fields.
// We only need to check (and add) instance fields if a class doesn't extend anything, considering extending classes inherit them.
Expand Down Expand Up @@ -137,21 +142,22 @@ class PolymodBaseClassMacro
'PolymodBaseClassMacro: Couldn\'t build hscript fields for the class $fullClsName since it already has the static field ${fld.name}.',
pos
);
return fields;
return null;
}
else if (cls.superClass == null && neededInstFields.contains(fld.name))
{
Context.info(
'PolymodBaseClassMacro: Couldn\'t build hscript fields for the class $fullClsName since it already has the instance field ${fld.name}.',
pos
);
return fields;
return null;
}
}

// Build the hscript needed fields.
fields = fields.concat(buildBaseClassInstanceFields(cls));
fields = fields.concat(buildBaseClassStaticFields(cls));
if (cls.superClass == null) cls.meta.add(ASC_ROOT_META, [], pos);

// Override the functions now since the `_asc` field was generated.
overrideBaseFunctions(cls, fields, neededInstFields);
Expand All @@ -173,7 +179,7 @@ class PolymodBaseClassMacro
if (Context.defined('display')) return;

// Don't override anything if the class (or the superclass) doesn't have the _asc field.
if (cls.findField('_asc') == null && ![for (f in fields) f.name].contains('_asc')) return;
if (!hasAscField(cls)) return;

for (i in 0...fields.length)
{
Expand Down Expand Up @@ -238,50 +244,17 @@ class PolymodBaseClassMacro
doesReturnVoid = (f.ret.toString() == 'Void');
}

var oldPos:haxe.macro.Expr.Position = f.expr.pos;
f.expr = useBridge() ? (macro
{
var skipAscFrom:Null<Array<String>> = _skipAscFrom;
if (_asc != null && (skipAscFrom == null || !skipAscFrom.contains($v{fields[i].name})))
{
var scriptCls:Dynamic = polymod.hscript.PolymodScriptBridge.findScript(_asc, $v{fields[i].name});
if (scriptCls != null) $
{
doesReturnVoid ? (macro
{
polymod.hscript.PolymodScriptBridge.callOn(scriptCls, $v{fields[i].name}, [$a{argExprs}]);
return;
}) : (macro return cast polymod.hscript.PolymodScriptBridge.callOn(scriptCls, $v{fields[i].name}, [$a{argExprs}]))
}
}
var callExpr:Expr = macro polymod.hscript.PolymodScriptBridge.callOn(scriptCls, $v{fields[i].name}, [$a{argExprs}]);
var scriptCallExpr:Expr = doesReturnVoid ? callExpr : (macro return cast $callExpr);

// Fallback, call the original function.
${f.expr}
}) : (macro
// This is generated for every function of every class, so keep it small; the script lookup happens in the bridge.
// The original body goes in an `else` branch with no `return` inside a loop, so the function can still be inlined at call sites.
var oldPos:haxe.macro.Expr.Position = f.expr.pos;
f.expr = macro
{
var skipAscFrom:Null<Array<String>> = _skipAscFrom;
if (_asc != null && (skipAscFrom == null || !skipAscFrom.contains($v{fields[i].name})))
{
var cls:Dynamic = _asc;
while (cls != null && cls is polymod.hscript._internal.PolymodScriptClass)
{
var scriptCls = (cls : polymod.hscript._internal.PolymodScriptClass);
if (scriptCls.hasScriptFunction($v{fields[i].name})) $
{
doesReturnVoid ? (macro
{
scriptCls.callFunction($v{fields[i].name}, [$a{argExprs}]);
return;
}) : (macro return cast scriptCls.callFunction($v{fields[i].name}, [$a{argExprs}]))
}

cls = cls.superClass;
}
}

// Fallback, call the original function.
${f.expr}
});
var scriptCls:Dynamic = (_asc == null) ? null : polymod.hscript.PolymodScriptBridge.findOverride(_asc, _skipAscFrom, $v{fields[i].name});
if (scriptCls != null) $scriptCallExpr else ${f.expr}
};

f.expr.pos = oldPos;

Expand All @@ -291,6 +264,21 @@ class PolymodBaseClassMacro
}
}

/**
* Checks whether the class has the `_asc` field, either generated for it or inherited from a superclass.
* `_asc` is only generated on root classes, which get marked with `ASC_ROOT_META`.
* Retrieving the superclass fields instead (like `findField` does) forces them to be typed during this build, which is slow.
* @param cls The class to check.
*/
static function hasAscField(cls:ClassType):Bool
{
var rootCls:ClassType = cls;
while (rootCls.superClass != null)
rootCls = rootCls.superClass.t.get();

return rootCls.meta.has(ASC_ROOT_META);
}

/**
* Builds the hscript instance fields for the class. Doesn't do anything if a class extends something.
* @param cls The class to build the instance fields for.
Expand Down Expand Up @@ -584,50 +572,12 @@ class PolymodBaseClassMacro
}
],
ret: macro :Null<$complexType>,
expr: useBridge() ? (macro
{
// The whole of this lives in the bridge, so a script names neither the class
// reference nor the error reporter.
return cast polymod.hscript.PolymodScriptBridge.instantiate(clsName, $v{underlyingClass}, (cast args) ?? []);
}) : (macro
// The whole of this lives in the bridge, so a script names neither the class reference nor the error reporter.
// This also keeps the code generated for every class small.
expr: macro
{
var clsRef = polymod.hscript._internal.PolymodStaticClassReference.tryBuild(clsName);

if (clsRef == null)
{
polymod.Polymod.error(
SCRIPT_RUNTIME_EXCEPTION,
'Could not construct instance of scripted class (${clsName} extends ' + $v{underlyingClass} + ')\nUnknown error building class reference',
SCRIPT_RUNTIME
);
return null;
}

try
{
var result = clsRef.instantiate((cast args) ?? []);
if (result == null)
{
polymod.Polymod.error(
SCRIPT_RUNTIME_EXCEPTION,
'Could not construct instance of scripted class (${clsName} extends ' + $v{underlyingClass} + '):\nUnknown error instantiating class',
SCRIPT_RUNTIME
);
return null;
}

return result;
}
catch (error)
{
polymod.Polymod.error(
SCRIPT_RUNTIME_EXCEPTION,
'Could not construct instance of scripted class (${clsName} extends ' + $v{underlyingClass} + '):\n${error}',
SCRIPT_RUNTIME
);
return null;
}
}),
return cast polymod.hscript.PolymodScriptBridge.instantiate(clsName, $v{underlyingClass}, cast args);
},
}),
}

Expand Down Expand Up @@ -793,11 +743,6 @@ class PolymodBaseClassMacro
];
}

/**
* Goes over the function expressions and removes `inline` from them. This is because inlining a function call requires said function
* to only have a single return, however due to how script extending works, this is no longer a guarantee.
* @param fields The fields whose function expressions to check and modify.
*/
/**
* The classes the host binary already carries, read from the `dll_import` file hxcpp is given.
*/
Expand Down Expand Up @@ -827,33 +772,6 @@ class PolymodBaseClassMacro
return hostClasses.exists(clsName);
}

static function removeInlinedFunctionCalls(fields:Array<Field>):Void
{
// This is a heavy operation and isn't needed to be done during code completion.
if (Context.defined('display')) return;

function removeInlines(expr:Expr):Expr
{
return switch (expr.expr)
{
case EMeta(s, e) if (s.name == ':inline'):
e;
default:
expr.map(removeInlines);
}
}

for (fld in fields)
{
switch (fld.kind)
{
case FFun(f) if (f.expr != null):
f.expr = f.expr.map(removeInlines);
default: // Do nothing.
}
}
}

/**
* `TypeTools.toComplexType` doesn't properly convert parameters into their underlying types, which results in a compilation error.
* We need the complex types as a return type for the `Class.scriptInit` function, and as such require underlying types.
Expand Down
Loading