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

Option in TFA to annotate inferred method return types, local variable types #55668

Closed
mkustermann opened this issue May 8, 2024 · 4 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. triaged Issue has been triaged by sub team type-enhancement A request for a change that isn't a bug

Comments

@mkustermann
Copy link
Member

It seems TFA annotates

  • parameter variables of procedures with their incoming argument type (vm.inferred-arg-type.metadata)
  • fields with the inferred type (vm.inferred-type.metadata)
  • captured local variables with inferred type (vm.inferred-type.metadata)
  • procedures with unboxing info (vm.unboxing-info.metadata)
  • ... (e.g. result of calls)

This is very much tailored to the VM (e.g. calling convention selection only needs unboxing information because in VM we only have to decide whether parameters/returns are tagged or unboxed primitives).

To make TFA work better for other backends it would be nice to have an option that allows annotating

  • all VariableDeclarations with inferred types (not just captured variables)
  • all procedures with inferred result type (not just unboxing information)

/cc @alexmarkov

@mkustermann mkustermann added the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label May 8, 2024
@a-siva a-siva added the type-enhancement A request for a change that isn't a bug label May 8, 2024
@a-siva
Copy link
Contributor

a-siva commented May 8, 2024

@mkustermann how critical is this in terms of priority ?

@a-siva a-siva added the triaged Issue has been triaged by sub team label May 8, 2024
@mkustermann
Copy link
Member Author

@mkustermann how critical is this in terms of priority ?

It's an optimization opportunity - so similar to other optimizations.

If it's easy to do I'd prefer we just do it now. If it requires significant work, then it's fine to do in future.

@mkustermann
Copy link
Member Author

I'll have a look at annotating the procedure return values as this is the most important atm.

@mkustermann mkustermann self-assigned this May 14, 2024
copybara-service bot pushed a commit that referenced this issue May 17, 2024
* Make TFA annotate members with inferred return types
  => We introduce new `vm.inferred-return-type.metadata`
* Make dart2wasm use inferred return types for wasm signatures

Issue #55668

TEST=Updated expectation files.

Change-Id: If2478255c2bb2633348f475b157f28581481578f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366540
Reviewed-by: Ömer Ağacan <omersa@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
@mkustermann mkustermann removed their assignment May 21, 2024
@mkustermann
Copy link
Member Author

I've implemented part of this (TFA annotating inferred return types) and we have a somewhat workaround for local variables not having the inferred type, so I'll close this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. triaged Issue has been triaged by sub team type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

2 participants