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

Make FunctionType implement TypeReference #175

Merged
merged 4 commits into from Nov 27, 2017

Conversation

natebosch
Copy link
Member

Fixes #174

  • Add a test which fails before this change and passes after
  • Add tests for nested function types since that was untested previously
  • Make FuctionType implement TypeReference, add missing fields
    returning null and rebuild the built_value file.
  • Return this from the type getter.

Fixes #174

- Add a test which fails before this change and passes after
- Add tests for nested function types since that was untested previously
- Make `FuctionType` implement `TypeReference`, add missing fields
  returning null and rebuild the built_value file.
- Return `this` from the `type` getter.
@natebosch
Copy link
Member Author

Hmm, I don't think we can do this. We can't implement both Built<FunctionType, FunctionTypeBuilder> and some other built_value class.

I'm not sure what the right way forward is.

@matanlurey
Copy link
Contributor

What about just loosening whatever is requesting a TypeReference to accept a Reference?

@natebosch
Copy link
Member Author

Changing type to a Reference seems to work.

I think, though, that it's a breaking change. Implementations of SpecVisitor will be broken by the change in signature of visitTypeParameters, and the change of the type field is also technically breaking.

@matanlurey
Copy link
Contributor

Realistically the only implementation of SpecVisitor is DartEmitter today.

... so I'm OK with saying the original implementation was broken, and custom implementations of the visitor/emitter will need a few lines updated to keep working.

@natebosch
Copy link
Member Author

Sounds good. I added a note to the changelog about the breaking change.

@natebosch natebosch merged commit 0ff897d into master Nov 27, 2017
@natebosch natebosch deleted the natebosch_function-type-parameter branch November 27, 2017 00:00
@natebosch
Copy link
Member Author

@matanlurey - if you have a chance to publish soon I'd appreciate it 😄

@matanlurey
Copy link
Contributor

@natebosch Definitely today, I'm back home in about an hour.

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.

None yet

3 participants