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

Adapt to the state when FunctionTypeAliasElement does not implement FunctionTypedElement. #2051

Conversation

scheglov
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Oct 30, 2019
Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

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

I think ModelElement.modelType needs a small additional adjustment along the same lines.

I am pretty certain ModelFunctionTypedef's construction isn't going to work right when the eventual change lands, and am surprised that the tests pass. We explicitly call ModelFunctionTypedef's constructor with a FunctionTypeAliasElement in the giant ModelElement.from dispatching constructor, and its constructor expects a FunctionTypedElement.

@scheglov
Copy link
Contributor Author

  1. It works in ModelElement.modelType because FunctionTypeAliasElement still implements TypeDefiningElement. It'd like it to stop doing this, but this is a bigger task than I'd like to do now (specifically there are many references in DDC, and I hope that DDC will die off sooner than I have to change it :-)). I still might do some tweaks there though, because I'd like to remove TypeDefiningElement.type (yes, type defining, but without a type :-)).

  2. Places where ModelFunctionTypedef is created, are inside if (e is GenericFunctionTypeElement), which just cannot be true both, at least now. It might have been the case in the past, but now FunctionTypeAliasElement does not implement GenericFunctionTypeElement. So, we hit the next block if (e is FunctionTypeAliasElement) and create Typedef instead, which sounds like a good name :-)

@jcollins-g
Copy link
Contributor

  1. It works in ModelElement.modelType because FunctionTypeAliasElement still implements TypeDefiningElement. It'd like it to stop doing this, but this is a bigger task than I'd like to do now (specifically there are many references in DDC, and I hope that DDC will die off sooner than I have to change it :-)). I still might do some tweaks there though, because I'd like to remove TypeDefiningElement.type (yes, type defining, but without a type :-)).

OK.

  1. Places where ModelFunctionTypedef is created, are inside if (e is GenericFunctionTypeElement), which just cannot be true both, at least now. It might have been the case in the past, but now FunctionTypeAliasElement does not implement GenericFunctionTypeElement. So, we hit the next block if (e is FunctionTypeAliasElement) and create Typedef instead, which sounds like a good name :-)

If you can drop a todo for me to clean this up I'd appreciate it. Otherwise, sounds good.

@scheglov scheglov merged commit 9528968 into dart-lang:master Oct 31, 2019
@scheglov scheglov deleted the FunctionTypeAliasElement-will-not-implement-FunctionTypedElement branch October 31, 2019 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants