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

need super-mixins flag in Fasta to alter warnings/errors around super calls #31994

Closed
mraleph opened this issue Jan 29, 2018 · 6 comments
Closed
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. P0 A serious issue requiring immediate resolution

Comments

@mraleph
Copy link
Member

mraleph commented Jan 29, 2018

When compiling Flutter we get a slew of errors like this:

compiler message: file:///usr/local/google/home/vegorov/src/flutter-clean/flutter/packages/flutter/lib/src/rendering/binding.dart:295:11: Error: Superclass has no method named 'hitTest'.
compiler message:     super.hitTest(result, position); // ignore: abstract_super_member_reference
compiler message:  

The error comes from this code:

/// An object that can hit-test pointers.
abstract class HitTestable { // ignore: one_member_abstracts
  // This class is intended to be used as an interface with the implements
  // keyword, and should not be extended directly.
  factory HitTestable._() => null;

  /// Check whether the given position hits this object.
  ///
  /// If this given position hits this object, consider adding a [HitTestEntry]
  /// to the given hit test result.
  void hitTest(HitTestResult result, Offset position);
}

/// The glue between the render tree and the Flutter engine.
abstract class RendererBinding extends BindingBase with ServicesBinding, SchedulerBinding, HitTestable {
  // This class is intended to be used as a mixin, and should not be
  // extended directly.
  factory RendererBinding._() => null;

  @override
  void hitTest(HitTestResult result, Offset position) {
    // ...
    // This super call is safe since it will be bound to a mixed-in declaration.
    super.hitTest(result, position); // ignore: abstract_super_member_reference
  }

}

as you can see the error is valid because Dart language conflates the notion of a class and a mixin together. You can also see that the error is suppressed in the analyzer.

To make Flutter builds error free we need a way to perform a similar suppression in CFE, while retaining correctness.

/cc @kmillikin @peter-ahe-google @leafpetersen @lrhn

@mraleph mraleph added P1 A high priority bug; for example, a single project is unusable or has many test failures area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels Jan 29, 2018
@mraleph mraleph added this to the 2.0-alpha1 milestone Jan 29, 2018
@lrhn
Copy link
Member

lrhn commented Jan 29, 2018

This code should only be accepted with the "super-mixins" flag enabled. In that case, it should be accepted and an invalid super-invocation is a dynamic error (like in Dart 1).

Without that flag, any method that callas a super-method must have an actual implementation of said method in its superclass.

@mraleph
Copy link
Member Author

mraleph commented Jan 29, 2018

Then we need super-mixins flag in the front-end

@mraleph mraleph changed the title need a way to suppress an error about "non-existent" super method need a super-mixins flag in Fasta to alter warnings/errors around super calls Jan 29, 2018
@mraleph mraleph changed the title need a super-mixins flag in Fasta to alter warnings/errors around super calls need super-mixins flag in Fasta to alter warnings/errors around super calls Jan 29, 2018
@a-siva
Copy link
Contributor

a-siva commented Jan 29, 2018

Please see step zero in the document linked from #31542 for more details on plan agreed upon by the language team.

@a-siva a-siva added P0 A serious issue requiring immediate resolution and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jan 30, 2018
@kmillikin kmillikin added this to In Progress in Dart Front End Jan 31, 2018
@mraleph
Copy link
Member Author

mraleph commented Jan 31, 2018

Update on this: I have discussed this with @askeksa-google - instead of setting up new flags and plumbing them through FE we are just going to just suppress these errors in pkg/vm integration layer.

If there are no objections to it - I will implement this tomorrow.

@mraleph mraleph assigned mraleph and unassigned askeksa-google Jan 31, 2018
@peter-ahe-google
Copy link
Contributor

This is already supported:

  • Don't use CompilerOptions.onError and CompilerOptions.reportMessages.
  • Instead use CompilerOptions.onProblem

@mraleph mraleph reopened this Feb 2, 2018
@mraleph mraleph added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. and removed 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). labels Feb 2, 2018
@mraleph
Copy link
Member Author

mraleph commented Feb 2, 2018

Keeping this open to track the implementation

@kmillikin kmillikin moved this from In Progress to Done in Dart Front End Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. P0 A serious issue requiring immediate resolution
Projects
Development

No branches or pull requests

5 participants