Skip to content

dart:mirrors fails to find metadata on dependencies #33774

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
sestegra opened this issue Jul 7, 2018 · 7 comments
Closed

dart:mirrors fails to find metadata on dependencies #33774

sestegra opened this issue Jul 7, 2018 · 7 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@sestegra
Copy link
Contributor

sestegra commented Jul 7, 2018

Dart VM version: 2.0.0-dev.67.0 (Tue Jul 3 18:17:07 2018 +0200) on "macos_x64"

@Foo()
import 'dart:mirrors';

class Foo {
  const Foo();
}

@Foo()
class Bar {
  @Foo()
  String it;

  void doIt(@Foo() String param) {}
}

void main() {
  {
    print("### Annotation on dependencies");
    final mirror = currentMirrorSystem().isolate.rootLibrary;

    for (var d in mirror.libraryDependencies) {
      print(d.targetLibrary.simpleName);
      print(d.metadata);
    }
  }

  {
    print("### Annotation on class, field and parameters");
    final mirror = reflectClass(Bar);
    print(mirror.metadata);
    print((mirror.declarations[#it] as VariableMirror).metadata);
    for (var p in (mirror.declarations[#doIt] as MethodMirror).parameters) {
      print("${p.simpleName} -> ${p.metadata}");
    }
  }
}

Output with Dart 1.24.3

## Annotation on dependencies
Symbol("dart.mirrors")
[InstanceMirror on Instance of 'Foo']
Symbol("dart.core")
[]
### Annotation on class, field and parameters
[InstanceMirror on Instance of 'Foo']
[InstanceMirror on Instance of 'Foo']
Symbol("param") -> [InstanceMirror on Instance of 'Foo']

Output with Dart 2.0.0-dev.67.0

### Annotation on dependencies
Symbol("dart.mirrors")
[]
### Annotation on class, field and parameters
[InstanceMirror on Instance of 'Foo']
[InstanceMirror on Instance of 'Foo']
Symbol("param") -> []
@sestegra
Copy link
Contributor Author

sestegra commented Jul 7, 2018

Output with 6dfc4ef revision

### Annotation on dependencies
Symbol("dart.mirrors")
[]
### Annotation on class, field and parameters
[InstanceMirror on Instance of 'Foo']
[InstanceMirror on Instance of 'Foo']
Symbol("param") -> [InstanceMirror on Instance of 'Foo']

So only annotation on dependencies is still missing.

thosakwe added a commit to angel-dart-archive/framework that referenced this issue Jul 9, 2018
thosakwe added a commit to angel-dart-archive/framework that referenced this issue Jul 9, 2018
@sestegra sestegra changed the title dart:mirrors fails to find metadata on dependencies and method parameters dart:mirrors fails to find metadata on dependencies Jul 9, 2018
@sestegra
Copy link
Contributor Author

sestegra commented Jul 9, 2018

I found that dependencies annotations are not implemented.
I've started to investigate on the source code.
At least, the following patch should be applied.

diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.cc b/runtime/vm/compiler/frontend/kernel_translation_helper.cc
index ac65dec..5fac295 100644
--- a/runtime/vm/compiler/frontend/kernel_translation_helper.cc
+++ b/runtime/vm/compiler/frontend/kernel_translation_helper.cc
@@ -1304,7 +1304,10 @@ void LibraryDependencyHelper::ReadUntilExcluding(Field field) {
     }
       /* Falls through */
     case kAnnotations: {
-      helper_->SkipListOfExpressions();
+      annotation_count_ = helper_->ReadListLength();  // read list length.
+      for (intptr_t i = 0; i < annotation_count_; ++i) {
+        helper_->SkipExpression();  // read ith expression.
+      }
       if (++next_read_ == field) return;
     }
       /* Falls through */
diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.h b/runtime/vm/compiler/frontend/kernel_translation_helper.h
index 342a8de..ee81079 100644
--- a/runtime/vm/compiler/frontend/kernel_translation_helper.h
+++ b/runtime/vm/compiler/frontend/kernel_translation_helper.h
@@ -729,7 +729,7 @@ class LibraryDependencyHelper {
   };

   explicit LibraryDependencyHelper(KernelReaderHelper* helper)
-      : helper_(helper), next_read_(kFileOffset) {}
+      : annotation_count_(0), helper_(helper), next_read_(kFileOffset) {}

   void ReadUntilIncluding(Field field) {
     ReadUntilExcluding(static_cast<Field>(static_cast<int>(field) + 1));
@@ -740,6 +740,7 @@ class LibraryDependencyHelper {
   uint8_t flags_;
   StringIndex name_index_;
   NameIndex target_library_canonical_name_;
+  intptr_t annotation_count_;

  private:
   KernelReaderHelper* helper_;

@sestegra
Copy link
Contributor Author

sestegra commented Jul 9, 2018

I guess that void KernelLoader::LoadLibraryImportsAndExports(Library* library) method has to be updated as well (from runtime/vm/kernel_loader.cc file).

@sestegra
Copy link
Contributor Author

sestegra commented Jul 9, 2018

I'm close to the solution, but I'm only starting to understand C++ source.
This patch is crashing at "BuildAnnotations".

diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc
index 114ae9a..920740f 100644
--- a/runtime/vm/kernel_loader.cc
+++ b/runtime/vm/kernel_loader.cc
@@ -869,6 +869,7 @@ void KernelLoader::LoadLibraryImportsAndExports(Library* library) {
   const intptr_t deps_count = builder_.ReadListLength();
   for (intptr_t dep = 0; dep < deps_count; ++dep) {
     LibraryDependencyHelper dependency_helper(&builder_);
+    intptr_t library_dependency_kernel_offset = builder_.ReaderOffset();
     dependency_helper.ReadUntilExcluding(LibraryDependencyHelper::kCombinators);

     // Ignore the dependency if the target library is invalid.
@@ -937,6 +938,14 @@ void KernelLoader::LoadLibraryImportsAndExports(Library* library) {
         }
       }
     }
+
+    if (FLAG_enable_mirrors && (dependency_helper.annotation_count_ > 0)) {
+      AlternativeReadingScope alt(&builder_.reader_, library_dependency_kernel_offset);
+      LibraryDependencyHelper dependency_helper(&builder_);
+      dependency_helper.ReadUntilExcluding(LibraryDependencyHelper::kAnnotations);
+      Object& metadata = Object::ZoneHandle(Z, builder_.BuildAnnotations());
+      target_library.AddLibraryMetadata(metadata, builder_.ReadPosition());
+    }
   }
 }

@sestegra
Copy link
Contributor Author

sestegra commented Jul 9, 2018

@alexmarkov, @rmacnak-google
Could you give me small help ?
I know well Dart, but I'm only a beginner on C++ source code.

@a-siva a-siva added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jul 9, 2018
@sestegra
Copy link
Contributor Author

Thanks

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, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

4 participants