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

[Extension type] DDC implementation #49735

Open
Tracked by #52684
itsjustkevin opened this issue Aug 19, 2022 · 4 comments
Open
Tracked by #52684

[Extension type] DDC implementation #49735

itsjustkevin opened this issue Aug 19, 2022 · 4 comments
Assignees
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P1 A high priority bug; for example, a single project is unusable or has many test failures web-dev-compiler

Comments

@itsjustkevin
Copy link
Contributor

No description provided.

@a-siva a-siva added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Aug 19, 2022
@sigmundch sigmundch assigned nshahan and annagrin and unassigned sigmundch Aug 22, 2022
@annagrin annagrin added the P2 A bug or feature request we're likely to work on label Oct 26, 2022
@eernstg
Copy link
Member

eernstg commented Mar 6, 2023

Please note that this feature has been renamed: It is now the 'inline class' feature.

@itsjustkevin itsjustkevin changed the title [Views] DDC implementation [Inline Class] DDC implementation Mar 6, 2023
@itsjustkevin itsjustkevin changed the title [Inline Class] DDC implementation [Extension type] DDC implementation Jul 25, 2023
@nshahan
Copy link
Contributor

nshahan commented Aug 18, 2023

Open question: Is it useful for DDC to see ExtensionType nodes in the AST or can they be erased sometime before compilation begins? Are they necessary for the production of JS interop errors?

@sigmundch
Copy link
Member

I believe it is useful. @srujzs can share more, but I believe the _js_interop_checker and transformation that runs from DDC and dart2js needs to see the extension classes to properly do its work. That said, during code emission performing the erasure should be simple in that the CFE already provides the erasure semantics as properties on the extension type (e.g. DDC can read the internal .erasedType and emit that instead.)

@annagrin - may also have some insights from the debugging perspective. There may be some metadata that we may need to include in the future to integrate with devtools.

@srujzs
Copy link
Contributor

srujzs commented Aug 21, 2023

Nick and I chatted offline shortly after - we indeed do need to see ExtensionTypes in the AST to make some of our interop checks work. Our takeaway was that there is a small tax, however, from handling these types in every visitor (and not handling it is not necessarily an error). In DDC's case, there are some explicit conditionals checking for types too (and emitting an "invalid" type if not handled), so this lead to some hidden errors. Still, this can be handled by just calling typeErasure and more generally, these issues can be fixed by improving how we visit types as well.

I think the open question then is whether there's a way to have the JS interop checks still see nodes related to extension types, but have DDC use a lowering that removes them from the AST.

@nshahan nshahan added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Nov 14, 2023
copybara-service bot pushed a commit that referenced this issue Nov 14, 2023
Some SDK only inline helper methods expect specific types passed to
them. These expectations can't be enforced by the type system and
everything is currently working properly. Extension methods may cause 
confusion in the future so this change adds a bit more context to
the error messages just in case.

Issue: #49735
Change-Id: Idc620993d1a240fa5aaaccd4519433b04f0ba9ad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/335942
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
copybara-service bot pushed a commit that referenced this issue Nov 14, 2023
extension types. This ensures the same optimizations are applied to
extension types and the representation type they are erased to.

Issue: #49735
Change-Id: I630fc8b68e6e86b81ec85495286719ca39f89d14
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/335943
Reviewed-by: Mark Zhou <markzipan@google.com>
copybara-service bot pushed a commit that referenced this issue Nov 14, 2023
A better API is now available from the kernel nodes directly.

Issue: #49735
Change-Id: I1b4b36e44292cadc3f9d6c495ae93ae20e0db970
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/335944
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
copybara-service bot pushed a commit that referenced this issue Nov 17, 2023
Erase extension types when deciding to dispatch directly or to a helper
method for Object members.

Issue: #49735
Change-Id: I31081bf4ec64a0f667c8d40b08b94773e229ebe1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/335601
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
copybara-service bot pushed a commit that referenced this issue Nov 21, 2023
Erases extension types to their representation type for field
signatures. This is the best representation we have at runtime for
the field. Note this isn't necessarily the runtime type of the
field value.

Issue: #49735
Change-Id: Ibe064f4fd3829a858fc9fd920e2e91175d9ae0c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/336823
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
copybara-service bot pushed a commit that referenced this issue Dec 18, 2023
Ensures the same optimizations to expressions statically typed as
extension types.

Issue: #49735
Change-Id: Id9b944d3f716faf38bb4770eb9b90ccc708056ce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342104
Reviewed-by: Mark Zhou <markzipan@google.com>
copybara-service bot pushed a commit that referenced this issue Dec 18, 2023
Ensures the same optimizations to expressions statically typed as
extension types.

Issue: #49735
Change-Id: I981859562bdd81cf34bd5ed4160a270e6fdff9f3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342105
Reviewed-by: Mark Zhou <markzipan@google.com>
copybara-service bot pushed a commit that referenced this issue Dec 18, 2023
Ensures the same optimizations to expressions statically typed as
extension types.

Issue: #49735
Change-Id: If85f17203cb4c07c06da0e2e1712fea15c39c444
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342106
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
copybara-service bot pushed a commit that referenced this issue Dec 28, 2023
Either erased the extension type or documented why it seems erasure is 
not needed.

Issue: #49735
Change-Id: Ic6c2fcaebaf10570691b95a383b8ad2e40d2061a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/343720
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. P1 A high priority bug; for example, a single project is unusable or has many test failures web-dev-compiler
Projects
None yet
Development

No branches or pull requests

8 participants