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

VM should support new optional parameters syntax #4290

Closed
gbracha opened this issue Jul 31, 2012 · 6 comments
Closed

VM should support new optional parameters syntax #4290

gbracha opened this issue Jul 31, 2012 · 6 comments
Assignees
Labels
Milestone

Comments

@gbracha
Copy link
Contributor

@gbracha gbracha commented Jul 31, 2012

Per issue #4288

@crelier
Copy link
Member

@crelier crelier commented Aug 30, 2012

The plan is the following:

  1. Implement the argument definition test ?v, which returns true if v was passed as argument. This can be implemented independently in both in the vm (https://chromiumcodereview.appspot.com/10915022/) and in dart2js with the current flavor of optional parameters. It would be nice to have this feature working before transitioning the library to the new optional parameter syntax, i.e. before step 3 below.

  2. Implement the new optional named parameter syntax in the vm and in dart2js, that is the flavor using { }. However, we should temporarily accept a named argument passed as a positional optional. This guarantees that the current libraries and client code still using the old [ ] flavor will work without change.
    The temporary lax behavior is controlled by a flag --reject_named_argument_as_positional with false as default. Feel free to suggest a better flag name.

  3. Once the new named parameter syntax and the temporary flag are supported in both the vm and dart2js, we can advertise them and ask people to start changing programs and libraries with the help of the flag to find errors. Note that errors will be reported without help from the flag when they switch from the [ ] to the { } flavor. The flag only helps to discover errors with the [ ] flavor. At these stage, library writers can decide which flavor is best for a particular api, and they can use the argument definition test ?v in the implementation of the api.

  4. The default value of the flag is flipped.

  5. The flag is removed and the implementation is simplified to only support the new version.


Set owner to @crelier.
Added Started label.

@sethladd
Copy link
Member

@sethladd sethladd commented Sep 3, 2012

FYI the spec says that the optional named parameter syntax is: "Optional elements of a production are suffixed by a question mark like so: anElephant? "

and not ?v as the comment above hints at.

@crelier
Copy link
Member

@crelier crelier commented Sep 4, 2012

Hi Seth,

"Optional elements of production" have nothing to do with optional parameters. They refer to the metasyntax (i.e. notation in the spec itself) used to specify Dart syntax.

The ?v notation refers to the argument definition test (feature request issue #4265). Here is the relevant section:

An argument definition test is an expression that tests whether a formal parameter is bound to an object explicitly passed to a method or function.

argumentDefinitionTest:
   ‘?’ identifier
    ;

An argument definition test e of the form ?v evaluates to true iff the currently executing invocation of the function that declares v explicitly provided an argument for the formal parameter v; otherwise e evaluates to false.

It is a compile time error if v does not denote a formal parameter. The static type of an argument definition test is bool.

@sethladd
Copy link
Member

@sethladd sethladd commented Sep 4, 2012

Thanks Regis, that clears it up. :)

@crelier
Copy link
Member

@crelier crelier commented Sep 6, 2012

FYI, step 2) for the vm is under review: https://chromiumcodereview.appspot.com/10910119/

@crelier
Copy link
Member

@crelier crelier commented Sep 7, 2012

Fixed at r12004.
Note that issue #4977 has been filed to track steps 4 and 5, i.e. removal of the --reject_named_argument_as_positional flag.


Added Fixed label.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.