Add proper API for creating private symbols wrt a library.

Add proper API for creating private symbols wrt a library.

BUG=http://dartbug.com/13355
BUG=http://dartbug.com/12358
R=ahe@google.com, asiva@google.com, gbracha@google.com

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

[rmacnak, 2013-09-26 00:02:55]

https://codereview.chromium.org/24631003/diff/1/sdk/lib/mirrors/mirrors.dart
File sdk/lib/mirrors/mirrors.dart (right):

https://codereview.chromium.org/24631003/diff/1/sdk/lib/mirrors/mirrors.dart#...
sdk/lib/mirrors/mirrors.dart:117: external static Symbol getSymbol(String name,
[LibraryMirror lib]);
createSymbol? newSymbol?

[rmacnak, 2013-09-27 01:38:27]

https://chromiumcodereview.appspot.com/24631003/diff/8001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/24631003/diff/8001/runtime/vm/object.c...
runtime/vm/object.cc:221: RawString* String::StripAccessorPrefix(const String&
name) {
Yuck. This should at least be refactored so it can shared with
IdentifierPrettyName.

https://chromiumcodereview.appspot.com/24631003/diff/8001/tests/lib/lib.status
File tests/lib/lib.status (right):

https://chromiumcodereview.appspot.com/24631003/diff/8001/tests/lib/lib.statu...
tests/lib/lib.status:14: mirrors/intercepted_class_test: RuntimeError # Issue
13644
Not sure if there is anything to split out for partial coverage.

https://chromiumcodereview.appspot.com/24631003/diff/8001/tests/lib/mirrors/c...
File tests/lib/mirrors/closures_test.dart (left):

https://chromiumcodereview.appspot.com/24631003/diff/8001/tests/lib/mirrors/c...
tests/lib/mirrors/closures_test.dart:21: String rest = ' in s(_HashMap)'; //
Might become Map instead.
Testing the function's owner is bogus because the implementation is free to
define it in a superclass.

https://chromiumcodereview.appspot.com/24631003/diff/8001/tests/lib/mirrors/i...
File tests/lib/mirrors/intercepted_class_test.dart (left):

https://chromiumcodereview.appspot.com/24631003/diff/8001/tests/lib/mirrors/i...
tests/lib/mirrors/intercepted_class_test.dart:14: expect('s($name)',
cls.simpleName);
Testing the class name is bogus because the implementation is only required that
the class implement num/bool/String/etc

[siva, 2013-09-30 23:47:12]

LGTM.

Please wait for review from Peter for the API and dart2js parts.

https://chromiumcodereview.appspot.com/24631003/diff/17001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/24631003/diff/17001/runtime/vm/object....
runtime/vm/object.cc:351: String::Handle(String::SubString(name, at_pos, len -
at_pos));
const String& pre_at = .....;
const String* post_at = ......;

[rmacnak, 2013-10-01 00:47:11]

On 2013/09/30 23:47:12, siva wrote:
> LGTM.
> 
> Please wait for review from Peter for the API and dart2js parts.
> 
>
https://chromiumcodereview.appspot.com/24631003/diff/17001/runtime/vm/object.cc
> File runtime/vm/object.cc (right):
> 
>
https://chromiumcodereview.appspot.com/24631003/diff/17001/runtime/vm/object....
> runtime/vm/object.cc:351: String::Handle(String::SubString(name, at_pos, len -
> at_pos));
> const String& pre_at = .....;
> const String* post_at = ......;

Done.

[rmacnak, 2013-10-04 23:03:44]

On 2013/10/01 00:47:11, Ryan Macnak wrote:
> On 2013/09/30 23:47:12, siva wrote:
> > LGTM.
> > 
> > Please wait for review from Peter for the API and dart2js parts.
> > 
> >
>
https://chromiumcodereview.appspot.com/24631003/diff/17001/runtime/vm/object.cc
> > File runtime/vm/object.cc (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/24631003/diff/17001/runtime/vm/object....
> > runtime/vm/object.cc:351: String::Handle(String::SubString(name, at_pos, len
-
> > at_pos));
> > const String& pre_at = .....;
> > const String* post_at = ......;
> 
> Done.

Fixing http://dartbug.com/12141 also depends on this change.

[gbracha, 2013-10-05 01:43:24]

lgtm

[ahe, 2013-10-11 10:27:14]

LGTM!

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/_internal/...
File sdk/lib/_internal/lib/mirrors_patch.dart (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/_internal/...
sdk/lib/_internal/lib/mirrors_patch.dart:11: patch static Symbol
getSymbol(String name, [LibraryMirror lib]) {
Add newline before method.

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/mirrors/mi...
File sdk/lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/mirrors/mi...
sdk/lib/mirrors/mirrors.dart:110: * returned is with respect to [lib].
What happens if name is a private identifier and lib is null?

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/mirrors/mi...
sdk/lib/mirrors/mirrors.dart:117: external static Symbol getSymbol(String name,
[LibraryMirror lib]);
Change from "lib" to "library" (don't abbreviate).

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/lib.status
File tests/lib/lib.status (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/lib.stat...
tests/lib/lib.status:16: mirrors/intercepted_object_test: RuntimeError # Issue
13644
This seems to decrease test-coverage of dart2js. Can't you modify some other
tests?

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/mirrors/...
File tests/lib/mirrors/invoke_import_test.dart (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/mirrors/...
tests/lib/mirrors/invoke_import_test.dart:1: // Copyright (c) 2013, the Dart
project authors.  Please see the AUTHORS file
Do we need something like this for exports as well?

[rmacnak, 2013-10-11 20:39:33]

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/_internal/...
File sdk/lib/_internal/lib/mirrors_patch.dart (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/_internal/...
sdk/lib/_internal/lib/mirrors_patch.dart:11: patch static Symbol
getSymbol(String name, [LibraryMirror lib]) {
On 2013/10/11 10:27:15, ahe wrote:
> Add newline before method.

Done.

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/mirrors/mi...
File sdk/lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/mirrors/mi...
sdk/lib/mirrors/mirrors.dart:110: * returned is with respect to [lib].
On 2013/10/11 10:27:15, ahe wrote:
> What happens if name is a private identifier and lib is null?

Specified ArgumentError.

https://chromiumcodereview.appspot.com/24631003/diff/25001/sdk/lib/mirrors/mi...
sdk/lib/mirrors/mirrors.dart:117: external static Symbol getSymbol(String name,
[LibraryMirror lib]);
On 2013/10/11 10:27:15, ahe wrote:
> Change from "lib" to "library" (don't abbreviate).

Done.

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/lib.status
File tests/lib/lib.status (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/lib.stat...
tests/lib/lib.status:16: mirrors/intercepted_object_test: RuntimeError # Issue
13644
On 2013/10/11 10:27:15, ahe wrote:
> This seems to decrease test-coverage of dart2js. Can't you modify some other
> tests?

Refactored.

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/mirrors/...
File tests/lib/mirrors/invoke_import_test.dart (right):

https://chromiumcodereview.appspot.com/24631003/diff/25001/tests/lib/mirrors/...
tests/lib/mirrors/invoke_import_test.dart:1: // Copyright (c) 2013, the Dart
project authors.  Please see the AUTHORS file
On 2013/10/11 10:27:15, ahe wrote:
> Do we need something like this for exports as well?

Added to my list of things to test.

[rmacnak, 2013-10-11 20:56:28]

Committed patchset #10 manually as r28551 (presubmit successful).