Skip to content

unexpected token 'Function' #28610

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

Closed
zoechi opened this issue Feb 2, 2017 · 11 comments
Closed

unexpected token 'Function' #28610

zoechi opened this issue Feb 2, 2017 · 11 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@zoechi
Copy link
Contributor

zoechi commented Feb 2, 2017

'package:code_builder/dart/core.dart': error: line 114 pos 26: unexpected token 'Function'
  final ReferenceBuilder Function = _ref('Function');
                         ^
Dart VM version: 1.23.0-edge.b27f5f910078d8a53cbfdb33663a50fa96812e5f (Thu Feb  2 00:04:28 2017) on "linux_x64"
@zoechi
Copy link
Contributor Author

zoechi commented Feb 2, 2017

https://github.com/dart-lang/code_builder/blob/master/lib/dart/core.dart#L114

Is this a bug? Is Function used as a type becoming invalid? Is there a workaround how to prepare for the future?

@eernstg eernstg added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Feb 2, 2017
@eernstg
Copy link
Member

eernstg commented Feb 2, 2017

Function has always been the core library type that marked a type as being a function type. After a large amount of discussion we decided on a syntax for specifying specific function types using that same identifier, e.g., a function that receives an int and returns an int can be written as int Function(int). The intuition is that you just declare a function header, then replace the name by Function, and (optionally) drop the names of parameters (except named ones, if any), so this offers a way to write function types in many locations and in a reasonably readable format, in addition to the ability to use the same syntax in typedefs (where you previously couldn't declare a generic method type).

For more details about this feature, please check out #27527.

We are currently working with this feature and the breakage that it causes, in particular because it will be similar to a built-in identifier (i.e., it cannot be the name of a type, except for the one in core). The parser is allowed to disambiguate declarations based on having seen Function in a specific context, but I think that shouldn't cause the error message that you have seen (so it may indeed be a bug).

However, you won't be able to use Function as a user-defined identifier in many contexts. The rule of thumb would be to not declare anything to have the name Function. Hope you can adjust your code (or code generators!) to follow that rule without too much hassle.

@zoechi
Copy link
Contributor Author

zoechi commented Feb 2, 2017

@eernstg thanks a lot for your explanation.
The code is in the code_builder package in the dart-lang repo (see link above).

I also created dart-lang/tools#904 recently.
Perhaps you can post a comment, if you think the code should be changed.
If the code is supposed to work, and just caused by a bug in the bleeding edge VM, then there is nothing to do of course.

I'm totally fine with bugs caused by using bleeding edge builds not being fixed.
My intention is just to point out issues ahead of time if possible, so they can be fixed before a new release is published and stuff breaks in stable.

@eernstg
Copy link
Member

eernstg commented Feb 3, 2017

The parser is allowed to disambiguate and commit to seeing an inline function type (as opposed to a typedef'd function type) when it has seen type 'Function' ('(' | '<'), but there is no '(' and no '<' following 'Function' here. So the vm parser should not make any attempts to parse any part of final ReferenceBuilder Function = _ref('Function'); as a function type, and hence it is a bug in the vm parser (at commit b27f5f9). Thanks for catching this so early!

@zoechi
Copy link
Contributor Author

zoechi commented Feb 3, 2017

@eernstg Thanks for clarification. Wasn't entirely sure from your previous comment.

@crelier
Copy link
Contributor

crelier commented Feb 3, 2017

Agreed, this is my bug in the VM parser. I first had it right and then tried to improve error messages, totally forgetting that locals and members are allowed to be named "Function". A fix is under review: https://codereview.chromium.org/2673893002/

Thanks for the report.

Note however that it will be tricky to use such locals and members, especially to call a closure named "Function". I would recommend using a different name, e.g. "Function_".

@a-siva
Copy link
Contributor

a-siva commented Feb 3, 2017

We should probably cherry pick this fix to 1.22

@whesse
Copy link
Contributor

whesse commented Feb 6, 2017

This issue is not in 1.22, because the function syntax only starts being accepted in 1.23, I think.
Do we need to cherry-pick all the relevant CLs, or none of them?

@crelier
Copy link
Contributor

crelier commented Feb 6, 2017 via email

@eernstg
Copy link
Member

eernstg commented Feb 6, 2017

Right, milestone 1.23 was indicated for #27527 when this comment was written, confirming what Bill said.

@a-siva
Copy link
Contributor

a-siva commented Feb 6, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

5 participants