Implement MirrorUsed.targets for libraries

Implement MirrorUsed.targets for libraries.

Caveat: non-library targets are "replaced" by their library.

R=johnniwinther@google.com

Committed: https://code.google.com/p/dart/source/detail?r=25611

[Johnni Winther, 2013-07-30 11:15:09]

lgtm

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:215: void
registerMirrorUsage(Set<String> symbols,
Please document.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:219: bool
isNeededForReflection(Element element) => false;
Please document. For instance, is [element] only needed for reflection?

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:1203: void
reportHere(Spannable node, String debugMessage) {
Document that this is for debugging only.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart
(right):

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:331:
final Set<Element> metaTargetsUsed = new Set<Element>();
Isn't this a list of the annotated elements and not of the annotations
themselves?

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart (right):

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:67: * compiler
will emit a hint (that is, a warning that is not really a warning).
'not really a warning' -> 'not defined by the language specification'?

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:171: if
(overriddenUsage.override.contains(library)) {
I don't think this will find cycles of length > 2. For instance

library a;
@MirrorsUsed(override='b');
import 'dart:mirrors';

library b;
@MirrorsUsed(override='c');
import 'dart:mirrors';

library c;
@MirrorsUsed(override='a');
import 'dart:mirrors';

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:190:
usageMap.forEach((LibraryElement library, List<MirrorUsage> usages) {
What if we have

library a;
@MirrorsUsed(override='b',targets='Foo');
import 'dart:mirrors';

library b;
@MirrorsUsed(override='c',targets='Bar');
import 'dart:mirrors';

library c;
@MirrorsUsed(targets='Baz');
import 'dart:mirrors';

Will the effect not be that 'c' uses 'Bar' and 'a' and 'b' uses 'Foo' meaning
that 'a' does not really override 'b'?

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:263: } else if
(b.symbols == null && b.targets == null && b.metaTargets == null) {
Long line.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:402: if (string
== libraryName || string.startsWith('$libraryName.')) {
Add a TODO to ensure resolution to the longest match.

[ahe, 2013-07-30 12:05:45]

Thank you, Johnni.

As discussed, I'll submit this now so people can start playing with it, and
follow up with more CLs.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:215: void
registerMirrorUsage(Set<String> symbols,
On 2013/07/30 11:15:09, Johnni Winther wrote:
> Please document.

Done.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:219: bool
isNeededForReflection(Element element) => false;
On 2013/07/30 11:15:09, Johnni Winther wrote:
> Please document. For instance, is [element] only needed for reflection?

Done.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:1203: void
reportHere(Spannable node, String debugMessage) {
On 2013/07/30 11:15:09, Johnni Winther wrote:
> Document that this is for debugging only.

Done.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart
(right):

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:331:
final Set<Element> metaTargetsUsed = new Set<Element>();
On 2013/07/30 11:15:09, Johnni Winther wrote:
> Isn't this a list of the annotated elements and not of the annotations
> themselves?

No. This is a list of classes that can be used as annotations. I think this
works best for the enqueuer.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart (right):

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:67: * compiler
will emit a hint (that is, a warning that is not really a warning).
On 2013/07/30 11:15:09, Johnni Winther wrote:
> 'not really a warning' -> 'not defined by the language specification'?

Done.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:171: if
(overriddenUsage.override.contains(library)) {
On 2013/07/30 11:15:09, Johnni Winther wrote:
> I don't think this will find cycles of length > 2. For instance
> 
> library a;
> @MirrorsUsed(override='b');
> import 'dart:mirrors';
> 
> library b;
> @MirrorsUsed(override='c');
> import 'dart:mirrors';
> 
> library c;
> @MirrorsUsed(override='a');
> import 'dart:mirrors';

It seems like we need something else. I have removed this method.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:190:
usageMap.forEach((LibraryElement library, List<MirrorUsage> usages) {
On 2013/07/30 11:15:09, Johnni Winther wrote:
> What if we have
> 
> library a;
> @MirrorsUsed(override='b',targets='Foo');
> import 'dart:mirrors';
> 
> library b;
> @MirrorsUsed(override='c',targets='Bar');
> import 'dart:mirrors';
> 
> library c;
> @MirrorsUsed(targets='Baz');
> import 'dart:mirrors';
> 
> Will the effect not be that 'c' uses 'Bar' and 'a' and 'b' uses 'Foo' meaning
> that 'a' does not really override 'b'?

I think we're settling on a simpler solution.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:263: } else if
(b.symbols == null && b.targets == null && b.metaTargets == null) {
On 2013/07/30 11:15:09, Johnni Winther wrote:
> Long line.

Done.

https://codereview.chromium.org/21110003/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/mirrors_used.dart:402: if (string
== libraryName || string.startsWith('$libraryName.')) {
On 2013/07/30 11:15:09, Johnni Winther wrote:
> Add a TODO to ensure resolution to the longest match.

Actually, I'm already rewriting this in another CL, so I prefer not even adding
a TODO to avoid conflicts :-)

[ahe, 2013-07-30 12:07:25]

Committed patchset #4 manually as r25611 (presubmit successful).