Skip to content

Metadata does not accept runtimeType literals #11857

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
rmacnak-google opened this issue Jul 16, 2013 · 15 comments
Closed

Metadata does not accept runtimeType literals #11857

rmacnak-google opened this issue Jul 16, 2013 · 15 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-as-intended Closed as the reported issue is expected behavior

Comments

@rmacnak-google
Copy link
Contributor

Runtime type literals are const and should be accepted.

import 'dart:mirrors';
@object
class C {}
main(){
 reflectClass(C).metadata; //Fails
}

import 'dart:mirrors';
const foo = Object;
@foo
class C {}
main(){
 reflectClass(C).metadata; //Works
}

@DartBot
Copy link

DartBot commented Jul 16, 2013

This comment was originally written by @mhausner


I suspect the VM does not treat a class name as a compile-time constant expression.


Added Triaged label.

@gbracha
Copy link
Contributor

gbracha commented Jul 16, 2013

I believe Ryan verified that it does. It just doesn't accept it as metadata.

@rmacnak-google
Copy link
Contributor Author

It does, otherwise in the second example
const foo = Object;
would not work.

@DartBot
Copy link

DartBot commented Jul 17, 2013

This comment was originally written by @seaneagan


Would it be more useful to have @­Object be shorthand for @­Object() ?

For example we could add messages to deprecated annotations:

class Deprecated {

  String _message();

  const Deprecated([this.message]);
}

And then I can omit the parentheses when I don't want to include a message as in Java (C# ?):

@deprecated
oldApi();

and when I do want a message:

@deprecated('Use [newApi] instead.');
oldApi();

@gbracha
Copy link
Contributor

gbracha commented Jul 17, 2013

One problem with the proposal in comment 4 is that an identifier used in metadata means something different than in the rest of Dart. Object now means new Object().

Besides, we have real uses for using class names as metadata.

@DartBot
Copy link

DartBot commented Jul 18, 2013

This comment was originally written by @seaneagan


@object() already means "const Object()" whereas in the rest of dart,
Object() means a function application.

Also, I can see it being a common source of errors, using @­Object when one
means @­Object() (due to familiarity with Java or C#).

I have to think those would be much less common, any examples? I'm worried
that with the current syntax people will design annotations like:

class SomeAnnotation {}

just so they can refer to it as @­SomeAnnotation, but then if they ever want
to make it parameterizable, it would be a breaking change, whereas if they
were able to use the same @­SomeAnnotation syntax with:

class SomeAnnotation {
  const SomeAnnotation();
}

then making it parameterizable would be a backwards compatible change.

If there are cases to use a runtimeType as a metadata value, maybe
assigning it to a variable and using that is sufficient:

const object = Object;

@object
...

@iposva-google
Copy link
Contributor

Set owner to @rmacnak-google.
Added Accepted label.

@DartBot
Copy link

DartBot commented Oct 2, 2013

This comment was originally written by @seaneagan


What are the use cases for this?

There are plenty of use cases and demand for treating @­Foo as @­Foo(), as elaborated in issue #2.

@DartBot
Copy link

DartBot commented Oct 3, 2013

This comment was originally written by [email protected]


This point was also discussed on the Dart misc list : https://groups.google.com/a/dartlang.org/d/msg/misc/bZ5kVTOnUN8/ZlQeTWpMnMwJ

@rmacnak-google
Copy link
Contributor Author

This issue causes mirrors/immutable_collections_test to fail on Dartium.

@DartBot
Copy link

DartBot commented Oct 25, 2013

This comment was originally written by @seaneagan


The analyzer also does not accept @­Foo (where Foo is a class), and even has a specific error message for it:

"Annotation creation must have arguments"

Maybe the best thing to do for 1.0 is to just disallow @­Foo, and figure out later whether to fix issue #2 or this issue?

@DartBot
Copy link

DartBot commented Oct 25, 2013

This comment was originally written by [email protected]


If you do that, you will have an annotation nightmare to fix post 1.0 ...

I understand 1.0 is approaching, but please have a look to current annotations status in the different frameworks (Google and thirdparty), nobody is happy with current implementation.

@rmacnak-google
Copy link
Contributor Author

Subsumed by Issue #14647.


Added AsDesigned label.

@DartBot
Copy link

DartBot commented Apr 23, 2014

This comment was originally written by [email protected]


As a Dart GDE, I am one of the strongest Dart fan, but I still think that annotations in Dart have a big problem and lead to ugly code.

Look at what AngularDart did with their recent support of injectable classes. We must use @­inject or @­Injectable().

Being able to use @­Injectable or @­Inject would have been so much better ...

@DartBot
Copy link

DartBot commented Apr 23, 2014

This comment was originally written by @seaneagan


Sebastian, please star/comment issue #13582.

@rmacnak-google rmacnak-google added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-as-intended Closed as the reported issue is expected behavior labels Apr 23, 2014
@rmacnak-google rmacnak-google self-assigned this Apr 23, 2014
This issue was closed.
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. closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

5 participants