Refactor Library/CompilationUnit and how we define local Scope.

Refactor Library/CompilationUnit and how we define local Scope.

This also collapses members and constructors of a class into the same
map.

BUG=

Original URL: https://chromiumcodereview.appspot.com//10832203

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

[Anders Johnsen, 2012-08-14 11:20:23]

Same as earlier, passes co19 on bleeding.

[Lasse Reichstein Nielsen, 2012-08-14 11:34:31]

lgtm

[ahe, 2012-08-14 12:45:26]

Hi Anders,

Thank you for cleaning this up. There are still dragons in the compiler, but I
think you have slain more than you added :-)

Cheers,
Peter

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:206: CompilationUnitElement
getCompilationUnit() {
I'm confused about getCompilationUnit and asCompilationUnit. Let's look at this
in the next iteration.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:290: class
ScopeContainerElement extends ContainerElement {
Nit: Rename to ScopedContainerElement?

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:403: Uri get uri() =>
entryCompilationUnit.script.uri;
Looking at some of the other changes in this CL, this getter may have been
useful:

Script get script() => entryCompilationUnit.script;

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:1054: * Lookup super members
for the class. This will ignore constructors.
Indent.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:1129: bool get
hasConstructor() {
This is an O(n) operation but looks like a simple accessor. I think we should
make this a method and rename it.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:1131: for (var element in
localScope.getValues()) {
This should be iterating through a Link-list and use the C-style for-loop.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:1137: Link<Element> get
constructors() {
Same issue as above: doesn't feel like a getter as it returns a new list on each
call.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/elements/elements.dart:1140: for (Element member in
localMembers) {
C-style for-loop, please.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
File lib/compiler/implementation/scanner/scanner_task.dart (right):

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/compiler/implem...
lib/compiler/implementation/scanner/scanner_task.dart:9: void
scanLibrary(LibraryElement library) {
I think you need measure around this.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/dartdoc/mirrors...
File lib/dartdoc/mirrors/dart2js_mirror.dart (right):

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/dartdoc/mirrors...
lib/dartdoc/mirrors/dart2js_mirror.dart:239:
library.localMembers.forEach((element) {
I like this better than the old version.

https://chromiumcodereview.appspot.com/10830303/diff/5001/lib/dartdoc/mirrors...
lib/dartdoc/mirrors/dart2js_mirror.dart:489: String path = _library.uri.path;
Also better, I think.

https://chromiumcodereview.appspot.com/10830303/diff/5001/tests/compiler/dart...
File tests/compiler/dart2js/mock_compiler.dart (right):

https://chromiumcodereview.appspot.com/10830303/diff/5001/tests/compiler/dart...
tests/compiler/dart2js/mock_compiler.dart:199: for (var element in
imported.localMembers) {
This is a test, so I don't mind using for-in.

[ahe, 2012-08-14 12:45:46]

And one more thing... LGTM