Strange inference issues in const constructor default parameters

[nex3]

Consider the following code:

abstract class Equality<E> {}

class DefaultEquality<E> implements Equality<E> {
  const DefaultEquality();
}

class SetEquality<E> implements Equality<E> {
  const SetEquality([Equality<E> inner = const DefaultEquality()]);
}

This produces no analysis warnings, which is what I'd expect. However, if I add
the following method:

void main() {
  const SetEquality<String>();
}

I get the following:

[info] The object type 'DefaultEquality<E>' cannot be assigned to a parameter of type 'Equality<String>'

There are a few issues here. First, it's not clear what this message is trying
to communicate; as far as I can tell, the code is entirely valid. Second, the
fact that the info is only printed when the code is used rather than when it's
defined is a problem, because it means we can't reliably run strong-mode
analysis on our code to know whether it will be warning-clean for our users.

[jmesserly]

Great catch!

Just a guess, it sounds like a bug in the constant evaluator, since that's only triggered when you instantiate a constant. That could also explain why it's a "hint". Here's a repro that doesn't require strong mode:

abstract class Equality<E> {}
class DefaultEquality<E> implements Equality<E> {
  const DefaultEquality();
}
class SetEquality<E> implements Equality<E> {
  const SetEquality([Equality<E> inner = const DefaultEquality<E>()]);
}
main() {
  const SetEquality<String>();
}

$ dartanalyzer test.dart
Analyzing [test.dart]...
[error] The constant creation cannot use a type parameter (test.dart, line 8, col 64)
[info] The object type 'DefaultEquality<E>' cannot be assigned to a parameter of type 'Equality<String>' (test.dart, line 12, col 3)
1 error found.

Strong mode is losing the "constant creation cannot use a type parameter" error. I guess because we infer the <E>, probably ErrorVerifier only checks AST structure and not the actual DartType.

I'm not sure why spec mode decided to disallow the type parameter there.

[gabeschine]

Gentle ping.

We have fixed our 'collection' package to 1.4.0 to avoid analyzer errors that break our build using a newer version.

[jmesserly]

thanks for the ping @gabeschine . No promises but I'll try and get to this one sooner rather than later :)

[jmesserly]

CC @bwilkerson who might be able to help with this...

So I dug into this a bit. I think it can happen with fields too. Either way, strong mode infers a generic type argument for the default value/field initializer object. And it has to: otherwise it's a hole in the strong mode type system (or we'd have to reject these kinds of constants, not a good outcome either). But as Dart 1 didn't support type args there, Analyzer's constant evaluation engine didn't either for the same reason.

For default parameters and fields, Analyzer just gets the previously computed constant value that is cached on the element model. So there wouldn't be a way to apply the type argument at that point in the code (or at least, not that I can tell). We'd need the ability to evaluate it in context.

My guess is we need something similar to "constructor initializers" ... it looks like those AST nodes are exposed on the element model. There's also code to handle resynthesizing the AST for them needed. Then the AST is evaluated during the course of evaluating a constant constructor. If we had something like that for the default parameter value expressions & field initializers, we'd get the right value because we could compute them in the context of the current type argument. It would have some performance cost, but we could maybe save some work in ConstantVerifier (where the default value is currently computed).

I was debugging it with this test case (inferred_type_test.dart):

  void test_infersConstantGenericTypeArg() {
    // Regression test for https://github.com/dart-lang/sdk/issues/26141
    checkFile('''
abstract class Equality<E> {}
class DefaultEquality<E> implements Equality<E> {
  const DefaultEquality();
}
class SetEquality<E> implements Equality<E> {
  const SetEquality([Equality<E> inner = const DefaultEquality()]);
}
main() {
  const SetEquality<String>();
}
    ''');
  }

Error code is one of CONST_CONSTRUCTOR_PARAM_TYPE_MISMATCH or CONST_CONSTRUCTOR_FIELD_TYPE_MISMATCH. I found the place where things go wrong in evaluateConstructorCall (in the "Now evaluate the constructor declaration." section). So that's where, if the AST was available for the default value (& field initializers), we'd be able to compute it.

Does this sound like it's on the right track?

[bwilkerson]

@jmesserly I think the "non-strong-mode" example you gave (#26141 (comment)) is correctly reporting an error. According to section 16.12.2:

Let e be a constant object expression of the form const T.id(a1, . . . , an, xn+1 : an+1, . . . , xn+k : an+k) or the form const T(a1, . . . , an, xn+1 : an+1, . . . , xn+k : an+k). It is a compile-time error if T does not denote a class accessible in the current scope. It is a compile-time error if T is a deferred type (19.1). In particular, T may not be a type variable. If T is a parameterized type, it is a compile-time error if T includes a type variable among its type arguments.

Based on that, I think the error "The constant creation cannot use a type parameter" is correct. Later (#26141 (comment)), you wrote:

Either way, strong mode infers a generic type argument for the default value/field initializer object.

But isn't it a problem if strong mode is inferring a type argument for the const constructor invocation if that argument is a type parameter? It seems like inference shouldn't produce a result that cannot be expressed in the language.

That aside, I think the original code ought not to be producing a hint. It seems to me that the constructor ought to be treated as being equivalent to:

const SetEquality([Equality<E> inner = const DefaultEquality<dynamic>()]);

which removes the original hint, but produces what I think is a strong mode error on the default value in the constructor declaration:

ERROR: Type check failed: const DefaultEquality<dynamic>() (DefaultEquality<dynamic>) is not of type Equality<E> (test.dart:27)

tl;dr I'm confused about what the right semantics are here in strong mode, and want to understand what the right behavior of analyzer is before trying to figure out how to implement it.

[jmesserly]

tl;dr I'm confused about what the right semantics are here in strong mode, and want to understand what the right behavior of analyzer is before trying to figure out how to implement it.

oops, my bad, I should've written up chat with @leafpetersen. But yes in Strong Mode it is intended to infer the type there. Otherwise you couldn't use const generic objects in most cases, so they would effectively be turned off.

It seems like inference shouldn't produce a result that cannot be expressed in the language.

yeah definitely, that is a problem too :). Both explicit and inferred should be supported. We considered it a bit less serious of a problem if inference provides a way to express the desired behavior. (Also explicitly writing it hits the same problem as inference, so we'd have to fix this issue either way).

[jmesserly]

And also yeah, totally agree, this is only a Strong mode issue. Dart 1 type system can get away with the unsoundness of implicit-dynamic instantiation.

[jmesserly]

Getting back to this one... how would we add this capability to constant evaluation? My thought was to have default values & field initializers be evaluated in a similar way to the constructor initializers. I think that would get us to consistent behavior for all of them in strong mode.

I can also go ahead and add support for explicit <E> in strong mode, so folks don't have to rely on inference

[ianloic]

We're starting to hit this because our dependencies are starting to depend on more recent versions of the collection package...

[jmesserly]

I'm working on it now. Had been hoping for a bit of feedback on the approach, but in the meantime I'm forging ahead. It's quite an involved change now due to Analyzer summaries, but I'm making progress... (I think :) )

[ianloic]

Great! I think I worked out how to disable the errant errors in the meantime.

[jmesserly]

work in progress: https://codereview.chromium.org/2167263002

[jmesserly]

I'm making progress on that CL, btw, only thing left is to figure out redirecting factory constructors.

[jmesserly]

reverted due to a non-strong-mode failure, working on a quick fix there & re-landing

[jmesserly]

and the fix for that is rolled into https://codereview.chromium.org/2196483002/