Skip to content
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

Type inference should always infer the return type of operator []= as void #31779

Closed
stereotype441 opened this issue Jan 5, 2018 · 7 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. language-strong-mode-polish P2 A bug or feature request we're likely to work on

Comments

@stereotype441
Copy link
Member

For this code:

class C {
  dynamic operator[]=(dynamic index, dynamic value) {}
}
abstract class I {
  void operator[]=(dynamic index, dynamic value) {}
}
class D extends C implements I {
  operator[]=(dynamic index, dynamic value) {}
}

The front end reports the following error:

Error: Can't infer the type of '[]=': overridden members must all have the same type.
Specify the type explicitly.
  operator[]=(dynamic index, dynamic value) {}
          ^

As discussed in #31756, it would be better to simply infer a return type of void, since returning a value from operator []= is meaningless anyhow. Although this is a small corner case, addressing it seems like it would make the language more consistent, since it would make the type inference rule for the return type of operator []= the same as the type inference rule for the return type of a setter.

@leafpetersen and @lrhn do you agree? This would require changes to the front end, the analyzer, and the language spec. I'm happy to do the front end and analyzer changes.

@stereotype441 stereotype441 added analyzer-strong-mode area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). language-strong-mode-polish labels Jan 5, 2018
@stereotype441 stereotype441 self-assigned this Jan 5, 2018
@kmillikin kmillikin added the P2 A bug or feature request we're likely to work on label Jan 5, 2018
@eernstg
Copy link
Member

eernstg commented Jan 5, 2018

Lasse and I agreed when we talked about this earlier today: Yes, the inferred return type should be void, just like it is for setters. Here is a CL for the change that Lasse and Leaf may sign off to get it done.

@eernstg
Copy link
Member

eernstg commented Jan 8, 2018

The CL has now been landed.

whesse pushed a commit that referenced this issue Jan 9, 2018
… void.

R=brianwilkerson@google.com, paulberry@google.com

Bug: #31779
Change-Id: I1c396cce3f148585218e02ff4e41bc2f85cc924a
     #31638
Reviewed-on: https://dart-review.googlesource.com/32687
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@leafpetersen
Copy link
Member

SGTM.

@kmillikin
Copy link

This issue affects package yaml when compiling with the common front end.

@stereotype441
Copy link
Member Author

The analyzer was fixed in bb52d1a. This CL should fix the front end: https://dart-review.googlesource.com/#/c/sdk/+/35005

@stereotype441 stereotype441 removed analyzer-strong-mode area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Jan 17, 2018
@whesse whesse closed this as completed in 52c036c Jan 17, 2018
@mraleph
Copy link
Member

mraleph commented Jan 28, 2018

This still does not work if operator []= contains throw:

   operator []=(int index, value) {
     throw new UnsupportedError("Cannot modify an unmodifiable List.");
   }

(code from package yaml).

@mraleph mraleph reopened this Jan 28, 2018
@stereotype441
Copy link
Member Author

@mraleph It turns out that your case wasn't broken because of the throw; it was broken because when type inference is required for a parameter, it was triggering the old (broken) type inference logic for the return type. Should be fixed by https://dart-review.googlesource.com/c/sdk/+/37300.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. language-strong-mode-polish P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

5 participants