NamedParameter[6,8]NegativeTests passes for the wrong reasons on dart2js.

[DartBot]

This issue was originally filed by [email protected]

---

These 4 tests are all expecting compile-time errors due to invalid uses of named parameters. However, they also have an Expect.equals(true, false) in them which will cause them to pass (as negative tests) even if the correct compile-time checks are not implemented.

Running these tests directly on each implementation has shown (to the best of my understanding):
VM: Handles these correctly and will pass tests after they are fixed.
DartC: Does not perform the correct checks - and passes these tests only because they are broken.
Frog: Current performs exactly like dartc - but I will submit a CL later today so that Frog will match the VM.

I've marked status as high because having accurate and precise tests is of very high-value at this stage of the project.

[DartBot]

This comment was originally written by [email protected]

---

Added Area-Compiler, Triaged labels.

[kasperl]

I think we should rewrite the tests as a single multi-test like FauxverrideTest. That way we'll know that the test fails for the right reasons (on the right lines) and that the test pass if all problematic lines are removed.

[scheglov]

Set owner to @scheglov.
Added Accepted label.

[scheglov]

Hm...
I don't see in spec that method of interfaces are not allowed to have optional parameters with default values. Can you point on item in spec?

[DartBot]

This comment was originally written by [email protected]

---

8 Interfaces

[...]

It is a compile-time error if any default values are specified in the signature of an interface method, getter, setter or constructor.

[scheglov]

DartC is fixed to pass these tests without Expect.equals(true, false).

I change area to Area-Frog, once it will be fixed, probably should be changed to Area-Test.

---

Removed Area-Compiler label.
Added Area-Frog label.

[dgrove]

Added this to the FrogEditor milestone.

[dgrove]

Removed the owner.

[dgrove]

as of r1908, all of these tests pass on dartc, frog, and the VM with the Expect.equals(true, false) manually removed.

Moving to Area-Test so that the tests can be fixed.

---

Removed this from the FrogEditor milestone.
Removed Area-Frog label.
Added Area-Test label.

[whesse]

If we remove the "Expect.equals(true, false);" line, and run the tests on dart2js, the tests will fail (meaning they run with no errors, since they are negative tests.

So someone should commit the change that fixes the tests, update the status file, and file bugs on dart2js that the syntax errors are not found.

---

Removed Area-Test label.
Added Area-Dart2JS label.
Changed the title to: "NamedParameter[6,8]NegativeTests passes for the wrong reasons on dart2js.".

[whesse]

Added this to the M1 milestone.

[peter-ahe-google]

Yes, someone should change the test. The test is broken.

---

Removed Area-Dart2JS label.
Added Area-Test label.

[DartBot]

This comment was originally written by @aam

---

I have looked at this defect and made small changes to dart2js parser(parser.dart, partial_parser.dart) so that parsing of formal parameters is dependent on whether it is invoked from parsing of named function alias or not. That is very similar to how it is done in dart(C++) compiler.

However, with the way how dart2js parser is written it scans typedefs without going into deep analysis of the formal parameters first, parses them more carefully if typedefs are used in the code.

So with my changes code below fails compilation with "parameter must not specify a default value"

===
typedef void Callback([String msg = ""]);

main() {
  Callback cb1 = null;
}
===

but the code below(original named_parameters6) compiles without errors because Callback is not used.
===
typedef void Callback([String msg = ""]);

main() {
}
===

Is this "relaxed" mode of parsing expected for dart2js parser?

Same thing with named_parameters8_negative_test: you need to add InstallCallback invocation so that parser takes closer look at formal parameters.
===
main() {
  C c = new C();
+++ c.InstallCallback(null);
}
===

If this kind of errors need to be flagged even if types/methods are not used, then this verification needs to be moved to scanner(partial_parser).

[DartBot]

This comment was originally written by @aam

---

Here is codereview link for the patch for your review, please:
https://chromiumcodereview.appspot.com/10800027.

This fixes two tests referenced above as well as fixes named optional named params issue in dart2js parser.

[peter-ahe-google]

I'll work with Alexander on getting this submitted.

---

Added Started label.

[peter-ahe-google]

Set owner to @peter-ahe-google.

[peter-ahe-google]

Set owner to [email protected].

[DartBot]

This comment was originally written by @aam

---

Committed as http://code.google.com/p/dart/source/detail?r=11573

---

Added Fixed label.