Sdk: Map literal type arguments sometimes aren't inferred with --no-implicit-dynamic

[nex3]

If I analyze the following code with --no-implicit-dynamic:

void fn(Map<dynamic, dynamic> map) {}

void main() {
  fn({1: "foo"});
}

I get this error:

  error • Missing type arguments for map literal at test.dart:4:6 • strong_mode_implicit_dynamic_map_literal

I'd expect the map's type arguments to be inferred from the values I used, or maybe to be inferred as dynamic since I'm passing it to fn(); either way, I don't expect an error.

[Porl91]

Ran into this recently. It seems to be related to the arguments being the dynamic type.

[bwilkerson]

@stereotype441 Will this be covered by the inference back-porting that you're working on?

[stereotype441]

@bwilkerson it's not directly covered by my inference back-porting work, but since it's an inference bug, I'm happy to take a look. I've added it to my list and I'll try to get to it in the next week.

[stereotype441]

On second thought, isn't this the intended behavior? The idea of --no-implicit-dynamic (as I understand it) is to produce an error if type inference infers dynamic somewhere, as a way of hinting to the user that they may want to consider specifying a more precise type. That's precisely what's happening here, so I don't think it's a bug at all. (Note that if I remove the --no-implicit-dynamic option, or change the map literal to <dynamic, dynamic>{1: "foo"}, or change fn's argument type to Map<int, String>, the error goes away). @bwilkerson can you confirm that this is working as intended?

[bwilkerson]

As far as I know, you're right about the intent for --no-implicit-dynamic, so it appears to be working as defined.

But then the question becomes whether it's useful to warn users in all cases, such as when we're inferring dynamic because of an explicit dynamic. Perhaps the behavior of the flag needs to be refined.

[matanlurey]

I think we need to revisit this flag (and related ones, like --no-implicit-casts) later in 2018 (after Dart 2) and have some unified guidelines. The current implementation is helpful, but as we can see here it's not ideal for many use cases.

/cc @leafpetersen @vsmenon @jmesserly @bwilkerson

Would you be supportive of opening some meta issues ("post Dart2: Revisit --no-implicit-casts & --no-implicit-dynamic"), and then closing all issues related to this flag today as _WAI_? We should make it clear these flags are best effort, and not currently "production" supported as we push towards Dart 2/strong-mode.

[lrhn]

I agree with Brian (propagating an explicit dynamic doesn't feel the same as inferring a dynamic from lack of information), and I would not actually think that this was an "implicit dynamic" if I wrote the code myself.

[leafpetersen]

Yes, this isn't an inference bug. I think the general consensus is that the behavior of this flag should change as suggested above, but this isn't on the critical path right now.

[eernstg]

tl;dr Keep flagging this implicit dynamism; dynamic may be erased here later on rd;lt

There's an inherent ambiguity in a declaration like void fn(Map<dynamic, dynamic> map) {}: The first thing to note is that this is a map from a top type to a top type, and by generic covariance this means that clients are allowed to pass any Map to fn. When we wish to know which arguments are appropriate this is all that matters, all top types work the same.

Zooming in on the details, we can see that the top type is dynamic (rather than Object or void), and that matters in two ways:

• For the client, i.e., at a call site, the type annotation on the formal parameter map emerges (if statically known) as the type-from-context which may affect type inference on the actual argument.

• For the body of fn, the use of dynamic in the type annotation on map has implications for the treatment of expressions containing map.

It's not at all obvious to me that the perceived needs of the client and the concrete needs felt by the developer who is writing the body will coincide. For instance, it may well be convenient for the developer who writes the body to be able to call arbitrary methods on the values retrieved from the Map, so the developer would use dynamic rather than another top type, but then all the clients may actually be negatively affected by this implementation-driven choice, simply because their actual arguments will be "somewhat dynamic" for no good reason. I tend to agree with Paul that this _is_ an implicit dynamic which should be flagged by --no-implicit-dynamic.

However, we can use the variance of each occurrence of a top type to determine whether it's significant for the client respectively for an associated body which top type it is.

I've suggested that we should erase all top types to Object when they occur in contravariant locations of a callee. For instance, a declaration like fn would then look like void fn(Map<Object, Object> map) for clients; the "spuriously dynamic" problem which is the topic of this issue would then disappear.

In a more general setting, covariant locations are significant for the client, and contravariant locations are significant for an associated body. We've already seen that the choice among top types in the simplest contravariant location (the type of a formal parameter) is obviously significant for the body.

For covariant locations, (1) it matters a lot for the client whether the return type of a function which is being invoked is dynamic or another top type (and it doesn't matter for the body, so there's no conflict of interests), and (2) a similar logic applies to covariant locations which are more deeply nested:

void foo(void f(dynamic x)) {...}

main() {
  foo((x) => x.bar(42));
}

In this example, the single occurrence of dynamic is covariant, and it matters for the client because it becomes the type annotation of the formal parameter x of the function literal passed to foo in the body of main.

So my recommendation would be to keep flagging this kind of "implicit dynamic" for now, and then later on use variance based top type erasure to help developers route these effects only along the paths where they make sense.

[srawlins]

Bump. We're playing with --no-implicit-dynamic, and hitting this in several places, e.g. Intl.plural:

• Map<String, dynamic> examples (--no-implicit-dynamic complains when, e.g. 'widgets': 123} is passed as an argument)
• List args (--no-implicit-dynamic complains when, e.g. [a, b] is passed as an argument) - for this one I would expect --no-implicit-dynamic to continue to complain until the signature is changed to List<dynamic>.

[stereotype441]

Judging from the comments on Feb 4-5, it sounds like the language team doesn't have a consensus on what the expected behavior should be. @leafpetersen, @eernstg, @lrhn, can you make a recommendation on how we should proceed?

[leafpetersen]

I think my take is that --no-implicit-dynamic isn't really what we want, and that what I wrote up in my "simple strict static checking" doc is closer to what we want.

[srawlins]

That makes sense. I agree. We're trying to figure out of we can pursue using --no-implicit-dynamic until (simple) strict static checks can be written. This issue ("implicit dynamic" complaints where inference gives dynamic anyway) may leave --no-implicit-dynamic unhelpful, as it requires too much explicit dynamic.

It's an effort question then, re: --no-implicit-dynamic and strict static checks.

[leafpetersen]

I think that the changes that you would probably want to make to --no-implicit-dynamic to avoid the issue that you are running into are more or less the equivalent of implementing my proposed --strict-inference from the simple static checking doc (not the initial doc, which requires more work).

[imod]

any plans to fix this?