Skip to content

Confusing reference to inheritance in error message about member conflict #34274

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

Closed
eernstg opened this issue Aug 27, 2018 · 2 comments
Closed
Labels
improve-diagnostics Related to the quality of diagnostic messages legacy-area-front-end Legacy: Use area-dart-model instead. P2 A bug or feature request we're likely to work on

Comments

@eernstg
Copy link
Member

eernstg commented Aug 27, 2018

Consider the following test (excerpt from co19 test Language/Classes/Instance_Methods/same_name_setter_t01.dart):

class C {
  foo() {}
  set foo(var a) {}
}

main() {
  C c=new C();
  c.foo();
//    c.foo=1;
}

With dart2js (I'm using area-front-end because I assume that the behavior is predominantly determined in the front end), we get the following response:

Language/Classes/Instance_Methods/same_name_setter_t01.dart:16:3:
Error: Can't declare a member that conflicts with an inherited one.
  foo() {}
  ^
Language/Classes/Instance_Methods/same_name_setter_t01.dart:17:7:
Info: This is the inherited member.
  set foo(var a) {}
      ^
Error: Compilation failed.

It is correct to flag the given situation as an error, but it is confusing to claim that there is any involvement of inheritance: The two conflicting members are both declared in C, and the only superclass relationship is the one to Object, and Object has no member whose basename is foo.

So I believe that the given error message should avoid mentioning inheritance in this situation.

PS: This comment of #21201 says 'This is indeed fixed with the unified frontend', and this issue basically says "Not quite!", in the sense that the error is flagged, but the error message is misleading. It might be useful to check out what was done for #21201 when fixing this issue, because it should be concerned with the same parts of the implementation.

@eernstg eernstg added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) legacy-area-front-end Legacy: Use area-dart-model instead. P3 A lower priority bug or feature request labels Aug 27, 2018
@eernstg
Copy link
Member Author

eernstg commented Aug 27, 2018

Marking this as a p3-low issue, because it's only a confusing error message, not an outright incorrect behavior.

@askeksa-google askeksa-google removed the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Aug 28, 2018
@kmillikin kmillikin added P2 A bug or feature request we're likely to work on and removed P3 A lower priority bug or feature request labels Sep 6, 2018
@askeksa-google askeksa-google added the improve-diagnostics Related to the quality of diagnostic messages label Oct 29, 2018
@askeksa-google
Copy link

It now says:

34274.dart:3:7: Error: 'foo' is already declared in this scope.
  set foo(var a) {}
      ^^^
34274.dart:2:3: Context: Previous declaration of 'foo'.
  foo() {}
  ^^^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improve-diagnostics Related to the quality of diagnostic messages legacy-area-front-end Legacy: Use area-dart-model instead. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

3 participants