Check parameter types when evaluating constants.

Check parameter types when evaluating constants.

R=brianwilkerson@google.com

Committed: https://code.google.com/p/dart/source/detail?r=40978

[Paul Berry, 2014-10-07 00:15:51]

paulberry@google.com changed reviewers:
+ brianwilkerson@google.com, scheglov@google.com

[Brian Wilkerson, 2014-10-07 16:30:49]

I really don't want to add modes to the command-line analyzer. The other changes
look good, but let's discuss the commsnd-line changes if you think we really
need them.

https://codereview.chromium.org/633803002/diff/1/pkg/analyzer/lib/src/generat...
File pkg/analyzer/lib/src/generated/constant.dart (right):

https://codereview.chromium.org/633803002/diff/1/pkg/analyzer/lib/src/generat...
pkg/analyzer/lib/src/generated/constant.dart:729: if (!argumentValue.isNull &&
!argumentValue.type.isSubtypeOf(parameter.type)) {
We should perform this test before we access the default value, otherwise an
invalid default value will (I think) cause an error to be reported at every call
site.

[Paul Berry, 2014-10-07 17:03:09]

On 2014/10/07 16:30:49, Brian Wilkerson wrote:
> I really don't want to add modes to the command-line analyzer. The other
changes
> look good, but let's discuss the commsnd-line changes if you think we really
> need them.

Ok, let's discuss this morning.

[Paul Berry, 2014-10-07 17:03:37]

https://codereview.chromium.org/633803002/diff/1/pkg/analyzer/lib/src/generat...
File pkg/analyzer/lib/src/generated/constant.dart (right):

https://codereview.chromium.org/633803002/diff/1/pkg/analyzer/lib/src/generat...
pkg/analyzer/lib/src/generated/constant.dart:729: if (!argumentValue.isNull &&
!argumentValue.type.isSubtypeOf(parameter.type)) {
On 2014/10/07 16:30:48, Brian Wilkerson wrote:
> We should perform this test before we access the default value, otherwise an
> invalid default value will (I think) cause an error to be reported at every
call
> site.

The difficulty with that is that it doesn't have correct semantics according to
the spec.  For better or worse, the spec defines this as correct code (no
errors, no warnings):

  class A {
    const A();
  }
  class B extends A {
    const B();
  }
  class C {
    const C([B b = const A()]); // *
  }
  main() {
    new C(const B()); // **
  }

But if the line marked "// **" is replaced with "new C()", then that's a checked
mode compile time error.

I would be ok with adding a hint at the line marked "// *", though, to let the
user know that a checked mode compile time error will occur if the default isn't
overridden.

[Brian Wilkerson, 2014-10-07 19:01:57]

LGTM

[Paul Berry, 2014-10-07 20:51:35]

Committed patchset #1 (id:1) manually as 40978 (presubmit successful).