Skip to content
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.

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

Allow typing expression with added imports and usings. #10537

Closed
back2dos opened this issue Dec 28, 2021 · 8 comments
Closed

Allow typing expression with added imports and usings. #10537

back2dos opened this issue Dec 28, 2021 · 8 comments

Comments

@back2dos
Copy link
Member

This is a bit of a follow up on #3252

While the actual feature discussed there might be a nice addition, what I'm really looking for is a way to type an expression in a modified context (i.e. with additional imports/usings). For that I propose adding a call like the following to haxe.macro.Context:

static public function typeWith(imports:Array<ImportExpr>, usings:Array<TypePath>, expr:Expr):Expr;

That way to define imports/usings is aligned with Context.defineModule for consistency.

The reason I'm proposing to return Expr is to avoid encoding a potentially big TypedExpr. Instead a placeholder (much like the one generated via Context.storeTypedExpr) would be returned, which still allows retrieving the typed AST via Context.typeExpr.

@Gama11
Copy link
Member

Gama11 commented Dec 28, 2021

Seems more natural to me to have expr be the first argument there.

@Antriel
Copy link
Contributor

Antriel commented Dec 29, 2021

I like this idea.
I've been implementing something akin to traits, and I keep going back and forth between various implementations. The nicer ones usually get blocked by certain things I wasn't aware of when starting (no reliable way to turn TypedExpr into Expr, no way to add usings/imports in a build macro).

This would possibly allow me to avoid defining a new type using genericBuild and instead just use normal build macro which seem to be faster and cause less compiler cache stability issues.

@Simn
Copy link
Member

Simn commented Dec 29, 2021

I agree that we need something like this, but I'm not sure about the exact API. If you want to type two different expressions in the same modified context, the compiler has to create that context both times...

What I would prefer from a usage perspective is something like this:

function addImports(imports:Array<ImportExpr>, usings:Array<TypePath>, ?ctx:ContextHandle):ContextHandle
function typeExpr(e:Expr, ?ctx:ContextHandle):TypedExpr

I didn't think this through yet though.

@back2dos
Copy link
Member Author

Such context handles can go stale, which is a marvelous footgun.

I'm wondering whether creating contexts is expensive enough to worry about, because if I'm reading the original PR right, it looks cheap enough. But if not, perhaps something like this would work?

function withImports<X>(imports:Array<ImportExpr>, usings:Array<TypePath>, code:()->X):X;

The return value is not really needed, but returning values rather than assigning them to captured locals usually plays better with null safety.

Used with storeExpr the above would be perfect.

@Simn
Copy link
Member

Simn commented Dec 29, 2021

Sorry if I wasn't clear, my concern isn't necessarily performance but the fact that this feature couldn't be combined with anything else. Your withImports idea addresses this to some extent, but what I worry about is that we might want a similar withSomething functionality at some point and then we can't combine the two.

@back2dos
Copy link
Member Author

but what I worry about is that we might want a similar withSomething functionality at some point and then we can't combine the two.

To my understanding, the passed function would be called synchronously and the original context would be restored after its execution.

One can thus compose via nested calls, i.e. it should be fine to do something like withImports(someImports, someUsings, () -> withOnTypeNotFound(customTemporaryResolver, () -> withoutInlining(() -> storeExpr(someExpr)))) (the concrete functions don't matter but I hope their names are indicative of the type of stuff that should be composable in this fashion - at least in principle).

@Simn
Copy link
Member

Simn commented Dec 29, 2021

Ah, I misread that a bit. In that case this seems like a good solution!

@kLabz
Copy link
Contributor

kLabz commented Feb 28, 2022

Closed by #10602 I guess?

@Simn Simn closed this as completed Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants