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

inconsistent treatment of void in literals #35893

Open
tatumizer opened this Issue Feb 9, 2019 · 3 comments

Comments

Projects
None yet
3 participants
@tatumizer
Copy link

tatumizer commented Feb 9, 2019

Illustrated on example:

void bar() {}
void func(Map map) {}
void main() {
  var z=  {"x": bar()}; // OK
  print(z); // prints {x: null} -OK
  print({"x": bar()}); // prints {x: null} -OK
  func(z); // OK
  // had to comment out b/c error: The expression here has a type of "void"
  //func({"x": bar()}); // WHY error???
  func({"x": bar()} as Map); // "info: Unnecessary cast" but call succeeds in runtime
  // but why was it declared "unnecessary" if it didn't work without it?
}
@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Feb 11, 2019

This is an error.
As the specification is currently written, {"x": bar()} should be an error everywhere. A void-typed expression is a compile-time error except in a small list of cases. Those cases include assignment and parameters where the receiver has static type void, but it does not mention literals.

Both the analyzer and front-end miss this case. They seem to be working as if a void typed expression is allowed anywhere the context-type is void. Maybe that's what the specification should have been saying instead, but "context type" was not used in the specification at the time the list was written.

So, they accept var z = {"x": bar()}; because the inferred type of z is Map<String, void>.

The call func({x: bar()} fails because the context type of that map is Map<dynamic, dynamic>, and you can't use a void expression where a dynamic type is expected.

The func({"x": bar()} as Map) is unnecessary as a cast because the static type of {"x": bar()} is Map<String, void> which is assignable to Map<dynamic, dynamic>. It's not completely unnecessary because introducing it changes the context type for the map expression (there is no context type inside a an as cast, because the cast is there explicitly to convert between the expressions own type and the surrouding context type).

So, the tools are consistent and possibly smarter than the specification. It's still a bug.

@eernstg

This comment has been minimized.

Copy link
Member

eernstg commented Feb 11, 2019

It is true that the specification does not mention composite literals (lists and maps) where some type arguments are void, but we do have such cases in a number of tests that were created in order to support the implementation. For instance, void_type_usage_test.dart has the following:

...
Object testVoidParam(void x) {
  ...
  <void>[x];   //# param_literal_void_list_init: ok
  <Object>[x];   //# param_literal_list_init: compile-time error
  var m1 = <int, void>{4: x};   //# param_literal_map_value_init: ok
  var m2 = <void, int>{x : 4};   //# param_literal_map_key_init: ok
  var m3 = <dynamic, dynamic>{4: x};  //# param_literal_map_value_init2: compile-time error
  var m4 = <dynamic, dynamic>{x : 4};  //# param_literal_map_key_init2: compile-time error
  ...
}

Cases like this were added to this test file in 1a83245, clearly built on the expectation that elements in composite literals should be allowed to have type void when the corresponding type argument of the enclosing literal is void (inferred or specified). This might be a case where we should adjust the specification to express the intention that we have put into the test files (and that the implementations follow), rather than insisting on the current language in the spec.

@eernstg

This comment has been minimized.

Copy link
Member

eernstg commented Feb 11, 2019

Discussed this with @lrhn IRL, changing labels.

@eernstg eernstg self-assigned this Feb 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment