Add an imported into list for libraries so that it is possible to lookup

Add an imported into list for libraries so that it is possible to lookup
the local scope at the imported site in order to check if a variable is already
defined.

Added a check for duplicate definitions during class finialization which
checks for duplicate definitions.
Committed: https://code.google.com/p/dart/source/detail?r=593

[hausner, 2011-10-19 23:56:27]

LGTM w/comment.

http://codereview.chromium.org/8342046/diff/2004/vm/raw_object.h
File vm/raw_object.h (right):

http://codereview.chromium.org/8342046/diff/2004/vm/raw_object.h#newcode451
vm/raw_object.h:451: RawArray* imported_into_;      // List of libraries where
this is imported.
It doesn't fit on one line, but I think it's worthwhile to point out that the
list contains the importers that import _without_ a prefix.

[Ivan Posva, 2011-10-20 08:46:04]

DBCs and a question:

main.dart:
#import("library1.dart");
#import("library2.dart", prefix: "foo");

library1.dart:
#library("library1.dart");
var foo = 1;

library2.dart:
#library("library2.dart");
var foo2;

Shouldn't that be an error? Is this tested?

-Ivan

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc
File vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc#new...
vm/dart_api_impl_test.cc:1436: "#import(\"library1.dart\");\n"
This would become a little bit more readable if it was:
"#import('library1.dart');\n"

Also the \n is not strictly needed either.

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc#new...
vm/dart_api_impl_test.cc:1471: assert(!Dart_IsValidResult(result));
Could you check that there is a complaint about "foo" in the error string?

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc#new...
vm/dart_api_impl_test.cc:1582: "#library(\"library2.dart\");\n"
library2.dart?

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc
File vm/object.cc (right):

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc#newcode3632
vm/object.cc:3632: // First check if name is found in the local scope of the
library into
Please factor this out as it is essentially LookupObjectFiltered(name, *this)
where the second parameter is used to filter which imported library is skipped.
LookupObject then calls LookupObjectFiltered(name, Library::Handle()).

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc#newcode3701
vm/object.cc:3701: if (obj.IsNull()) {
I don't think the iterator is ever handing out null objects.

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc#newcode3712
vm/object.cc:3712: } else {
Please add a comment explaining why you can skip library prefixes.

[siva, 2011-10-20 23:08:47]

Yes the test case you had should be an error and I have
added it to the test cases that I have for this in the
other changelist (Library4NegativeTest.dart).

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc
File vm/dart_api_impl_test.cc (right):

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc#new...
vm/dart_api_impl_test.cc:1436: "#import(\"library1.dart\");\n"
On 2011/10/20 08:46:04, Ivan Posva wrote:
> This would become a little bit more readable if it was:
> "#import('library1.dart');\n"
> 
> Also the \n is not strictly needed either.

Done.

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc#new...
vm/dart_api_impl_test.cc:1471: assert(!Dart_IsValidResult(result));
On 2011/10/20 08:46:04, Ivan Posva wrote:
> Could you check that there is a complaint about "foo" in the error string?

Done.

http://codereview.chromium.org/8342046/diff/2004/vm/dart_api_impl_test.cc#new...
vm/dart_api_impl_test.cc:1582: "#library(\"library2.dart\");\n"
Changed to libraryB.dart
On 2011/10/20 08:46:04, Ivan Posva wrote:
> library2.dart?

Done.

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc
File vm/object.cc (right):

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc#newcode3632
vm/object.cc:3632: // First check if name is found in the local scope of the
library into
On 2011/10/20 08:46:04, Ivan Posva wrote:
> Please factor this out as it is essentially LookupObjectFiltered(name, *this)
> where the second parameter is used to filter which imported library is
skipped.
> LookupObject then calls LookupObjectFiltered(name, Library::Handle()).

Done.

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc#newcode3701
vm/object.cc:3701: if (obj.IsNull()) {
True, I changed this to an assertion.

On 2011/10/20 08:46:04, Ivan Posva wrote:
> I don't think the iterator is ever handing out null objects.

http://codereview.chromium.org/8342046/diff/2004/vm/object.cc#newcode3712
vm/object.cc:3712: } else {
Added comment stating that library prefixes in this library are not visible in
the importing scope.

On 2011/10/20 08:46:04, Ivan Posva wrote:
> Please add a comment explaining why you can skip library prefixes.

http://codereview.chromium.org/8342046/diff/2004/vm/raw_object.h
File vm/raw_object.h (right):

http://codereview.chromium.org/8342046/diff/2004/vm/raw_object.h#newcode451
vm/raw_object.h:451: RawArray* imported_into_;      // List of libraries where
this is imported.
On 2011/10/19 23:56:27, hausner wrote:
> It doesn't fit on one line, but I think it's worthwhile to point out that the
> list contains the importers that import _without_ a prefix.

Done.