FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Remove no longer used half of RTTI by dcodeIO · Pull Request #2555 · AssemblyScript/assemblyscript · GitHub

Remove no longer used half of RTTI - #2555

Merged
dcodeIO merged 3 commits into
mainfrom
condense-typeinfo
Nov 10, 2022
Merged

Remove no longer used half of RTTI#2555
dcodeIO merged 3 commits into
mainfrom
condense-typeinfo

Conversation

dcodeIO commented Nov 10, 2022

Copy link
Copy Markdown
Member

The compiler no longer depends on embedded RTTI, in particular base class ids (see #2548, where instanceof helpers are now compiler-generated). There are still two uses of RTTI outside of the compiler itself, though: One is to determine pointer-free objects during GC, and the other is the loader, that both rely on type flags. The pointer-free use case is important enough to at least keep the flags, so this PR slashes RTTI in half, removing the base class id alongside the now unused programmatic __instanceof helper.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

dcodeIO marked this pull request as ready for review November 10, 2022 19:45
dcodeIO merged commit a565d73 into main Nov 10, 2022

jtenner commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

This is going to break ASON. Why is __instanceof being removed? There doesn't seem like a logical reason for this to occur.

dcodeIO commented Nov 12, 2022

Copy link
Copy Markdown
Member Author

It's a change resulting from #2548, where remaining open issues with interfaces have been tackled as well that required a more sophisticated approach to implement instanceof to also support interfaces. It's now compiler generated. In turn, the old mechanism to perform these checks became unused, and as such redundant. Can you explain the use case a little? Perhaps there are ways to support it otherwise.

jtenner commented Nov 12, 2022
edited
Loading

Copy link
Copy Markdown
Contributor
abstract class BaseClass {}

class MyClass extends BaseClass {}

ASON.deserialize<BaseClass>(buffer); // no longer can validate a serialized MyClass

This breaks the entirety of the as-lunatic package and I would have to ask end users of ASON to do this:

let result = ASON.deserialize<BaseClass>(); // oh no it's an Array
assert(result instanceof BaseClass); // not ergonomic and backwards uncompatible

also support interfaces

Okay. I understand. Sorry for not understanding. I just feel like I have no workarounds for this.

dcodeIO commented Nov 12, 2022

Copy link
Copy Markdown
Member Author

Can ASON.deserialize<T> perhaps do:

function deserialize<T>(...) {
  let ret = ...;
  if (ret instanceof T) ... // AS allows this
}

jtenner commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

The assert would panic if it isn't an instanceof T... let me try that.

dcodeIO commented Nov 12, 2022

Copy link
Copy Markdown
Member Author

I guess one question there is what is used as the type of ret so this doesn't become precomputed, hmm.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL