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

Dyno: resolve bug in de/serialize generation, and other fixes #26631

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

benharsh
Copy link
Member

This PR fixes a trivial off-by-one bug with the generation of de/serialize formals. While poking further at IO calls, I observed some other issues with resolution and made some fixes:

  • add a workaround for case when extern function return type is not resolved
  • Improve 'canPass' to handle passing borrowed class as an actual, which can occur in isSubtype
  • Improves compiler-handled casting to and from unmanaged/borrowed any-classes

This PR also adds some tests to lock in this behavior.

Testing:

  • test-dyno

Signed-off-by: Ben Harshbarger <ben.harshb@gmail.com>
… type

Signed-off-by: Ben Harshbarger <ben.harshb@gmail.com>
Signed-off-by: Ben Harshbarger <ben.harshb@gmail.com>
Signed-off-by: Ben Harshbarger <ben.harshb@gmail.com>
Signed-off-by: Ben Harshbarger <ben.harshb@gmail.com>
@@ -4036,10 +4036,14 @@ static bool resolveFnCallSpecial(Context* context,
// cast (borrowed class) : unmanaged
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"and unmanaged to borrowed"

result = QualifiedType(QualifiedType::CONST_VAR, VoidType::get(context));
return true;
} else {
result = QualifiedType(QualifiedType::CONST_VAR, ErroneousType::get(context));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're trying to move to using ErroneousType only when an error would be emitted. Since there's no error, I think UnknownType is what should be returned here, or an error issued.

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