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

Annotations are not inherited from mixins with --preview-dart-2 #33099

Closed
scheglov opened this issue May 11, 2018 · 3 comments
Closed

Annotations are not inherited from mixins with --preview-dart-2 #33099

scheglov opened this issue May 11, 2018 · 3 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-kernel
Milestone

Comments

@scheglov
Copy link
Contributor

This code prints true without --preview-dart-2 and prints false with it.
@bwilkerson

import 'dart:mirrors';

const _FailingTest failingTest = const _FailingTest();

class _FailingTest {
  const _FailingTest();
}

class MyTest {
  @failingTest
  void foo() {}
}

class MyTest2 extends Object with MyTest {}

main() {
  ClassMirror classMirror = reflectClass(MyTest2);
  classMirror.instanceMembers
      .forEach((Symbol symbol, MethodMirror memberMirror) {
    if (memberMirror.simpleName == #foo) {
      print(memberMirror);
      print(_hasFailingTestAnnotation(memberMirror));
    }
  });
}

bool _hasFailingTestAnnotation(MethodMirror method) {
  var r = _hasAnnotationInstance(method, failingTest);
  print('[_hasFailingTestAnnotation] $method $r');
  return r;
}

bool _hasAnnotationInstance(DeclarationMirror declaration, instance) =>
    declaration.metadata.any((InstanceMirror annotation) {
      print('annotation: ${annotation.reflectee}');
      return identical(annotation.reflectee, instance);
    });
@scheglov scheglov added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. library-mirrors area-kernel area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels May 11, 2018
dart-bot pushed a commit that referenced this issue May 11, 2018
This works around #33099

R=brianwilkerson@google.com

Change-Id: Idfada538b30fe6d8cce2d23112787522e5d19521
Reviewed-on: https://dart-review.googlesource.com/54705
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@a-siva a-siva removed the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label May 16, 2018
@a-siva a-siva added this to the Dart2Stable milestone May 16, 2018
@kmillikin kmillikin reopened this May 28, 2018
@kmillikin
Copy link

This was reverted because it broke some Flutter tests. I am investigating.

@kmillikin
Copy link

This same code was used to generate noSuchMethod forwarders, and so the change to always copy annotations caused nSM forwarders to have the same annotations as the method they represented (they should not, according to the language team). This revealed a bug: the nSM forwarders would be marked external if the method they represented was (but they are not external). This was relatively benign until they had a corresponding @ExternalName annotation, which caused a crash.

We will fix the bugs in nSM forwarders and then reland the fix for this issue.

@kmillikin kmillikin removed area-front-end Use area-front-end for front end / CFE / kernel format related issues. library-mirrors labels May 29, 2018
@dgrove dgrove changed the title Annotations are not inherited from mixins with --prevew-dart-2 Annotations are not inherited from mixins with --preview-dart-2 May 29, 2018
@kmillikin kmillikin added this to In Progress in Dart Front End May 30, 2018
@kmillikin
Copy link

@stefantsov will implement removing annotations (including parameter annotations) from NSM forwarders (and clearing the external bit). He'll land a fix for this issue at the same time.

dart-bot pushed a commit that referenced this issue May 31, 2018
This reverts commit 242bc34.

R=brianwilkerson@google.com

Bug: #33099
Change-Id: I8a1b24926072fe64e3ab73d38a1fa73695237fc4
Reviewed-on: https://dart-review.googlesource.com/57701
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@kmillikin kmillikin moved this from In Progress to Done in Dart Front End Jun 1, 2018
@kmillikin kmillikin added area-front-end Use area-front-end for front end / CFE / kernel format related issues. front-end-kernel and removed area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels Sep 19, 2018
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. front-end-kernel
Projects
Development

No branches or pull requests

4 participants