Skip to content

language: disallow type literals in annotations #14647

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
kasperl opened this issue Oct 31, 2013 · 7 comments
Closed

language: disallow type literals in annotations #14647

kasperl opened this issue Oct 31, 2013 · 7 comments
Assignees
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). P0 A serious issue requiring immediate resolution
Milestone

Comments

@kasperl
Copy link

kasperl commented Oct 31, 2013

Right now, the spec explicitly allows:

   class Foo { ... }
   
   @­Foo
   bar() { ... }

We should disallow that so we have time to figure out what it is supposed to mean. We should change the wording in section 11 to:


Metadata consists of a series of annotations, each of which begin with the
character @­, followed by a constant expression that starts with an identi fier. It is a compile time error if the expression is not one of the following:

� - A reference to a compile-time constant variable.
�� - A call to a constant constructor (without the leading 'const').

@lrhn
Copy link
Member

lrhn commented Oct 31, 2013

Agree completely.

The current way it is working is error prone.

Users are expecting @­Deprecated to mean something that it doesn't, but it doesn't trigger any warnings or errors.

@gbracha
Copy link
Contributor

gbracha commented Oct 31, 2013

I disagree of course. This all feels rather Javanese. But I'm happy to accommodate.


Added Accepted label.

@gbracha
Copy link
Contributor

gbracha commented Oct 31, 2013

Would it be acceptable to make this a warning?

@kasperl
Copy link
Author

kasperl commented Oct 31, 2013

I'd say that this has to be a compile-time error (not a static warning) for two reasons:

First (and most importantly), only an error allows us to decide on the semantics later. If we make it a warning, we cannot really change what it means at runtime later without breaking stuff.

Second, it is much more consistent if everything that isn't explicitly allowed in an annotation leads to a compile-time error.

@gbracha
Copy link
Contributor

gbracha commented Oct 31, 2013

Ok. However, see my comments on 14652. These things won't be detected by the VM unless someone accesses the metadata.

@lrhn
Copy link
Member

lrhn commented Nov 1, 2013

Many errors won't be detected by the VM unless the code is run, so that should be fine.

The only potentially surprising part is that the "compile time" of an annotation isn't until it's accessed through mirrors, but as long as that's stated somewhere in the spec, I'm fine with it. The analyzer will report the error, which is what developers running on the VM should be using anyway.

@gbracha
Copy link
Contributor

gbracha commented Nov 1, 2013

Fixed in 0.8 spec (revision 29775).


Added Done label.

@kasperl kasperl added Type-Defect P0 A serious issue requiring immediate resolution area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Nov 1, 2013
@kasperl kasperl added this to the M8 milestone Nov 1, 2013
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). P0 A serious issue requiring immediate resolution
Projects
None yet
Development

No branches or pull requests

4 participants