Remove definitions from Kernel canonical names.

Remove definitions from Kernel canonical names.

Before: Canonical names contained pointers to the corresponding Kernel
tree which assumed that the whole tree was in memory whenever the
canonical names were.

Now: Canonical names do not contain these pointers.  They were only
really used to perform name-based lookup in the VM's heap so the
canonical name itself is enough.

If we later find that we need to get from a canonical name to its
Kernel tree we can add an offset in the binary (for instance) to the
canonical name or in a separate mapping on the side.

BUG=
R=asgerf@google.com, jensj@google.com, vegorov@google.com

Committed: https://github.com/dart-lang/sdk/commit/ed77783cd32d55fdad61bf9bc749030847ba9384
Committed: https://github.com/dart-lang/sdk/commit/d562f7ced52c28cd417660128b6bafe44534a422

[Kevin Millikin (Google), 2017-03-29 11:47:45]

kmillikin@google.com changed reviewers:
+ asgerf@google.com, vegorov@google.com

[Kevin Millikin (Google), 2017-03-29 11:48:52]

https://codereview.chromium.org/2781893004/diff/20001/pkg/kernel/lib/canonica...
File pkg/kernel/lib/canonical_name.dart (right):

https://codereview.chromium.org/2781893004/diff/20001/pkg/kernel/lib/canonica...
pkg/kernel/lib/canonical_name.dart:51: ///         Qualified name
The VM needs to distinguish factories from other (static) methods because the
names are decorated differently.

[Kevin Millikin (Google), 2017-03-29 11:50:30]

https://codereview.chromium.org/2781893004/diff/20001/runtime/vm/kernel_reade...
File runtime/vm/kernel_reader.cc (right):

https://codereview.chromium.org/2781893004/diff/20001/runtime/vm/kernel_reade...
runtime/vm/kernel_reader.cc:138: dart::Library& library =
LookupLibrary(main->Enclosing());
Maybe 'EnclosingName' because otherwise this reads like a verb?

[asgerf, 2017-03-29 11:50:34]

lgtm

[Vyacheslav Egorov (Google), 2017-03-29 13:19:52]

lgtm

https://codereview.chromium.org/2781893004/diff/20001/runtime/vm/kernel.cc
File runtime/vm/kernel.cc (right):

https://codereview.chromium.org/2781893004/diff/20001/runtime/vm/kernel.cc#ne...
runtime/vm/kernel.cc:92: return memcmp(parent->name()->buffer(), "@fields",
sizeof("@fields") - 1) ==
GCC and clang seem to constant fold strlen("zzzzz") which might be a nicer way
to write it. 

Also consider making a macro:

#define CompareName(ptr, literal) (memcmp(buf, literal, strlen(literal)) == 0)

because this pattern occurs multiple times down the file.

[Kevin Millikin (Google), 2017-03-29 13:43:42]

https://codereview.chromium.org/2781893004/diff/20001/runtime/vm/kernel.cc
File runtime/vm/kernel.cc (right):

https://codereview.chromium.org/2781893004/diff/20001/runtime/vm/kernel.cc#ne...
runtime/vm/kernel.cc:92: return memcmp(parent->name()->buffer(), "@fields",
sizeof("@fields") - 1) ==
On 2017/03/29 13:19:51, Vyacheslav Egorov (Google) wrote:
> GCC and clang seem to constant fold strlen("zzzzz") which might be a nicer way
> to write it. 
> 
> Also consider making a macro:
> 
> #define CompareName(ptr, literal) (memcmp(buf, literal, strlen(literal)) == 0)
> 
> because this pattern occurs multiple times down the file.

Done.

[Kevin Millikin (Google), 2017-03-29 14:03:10]

Description was changed from

==========
Remove definitions from Kernel canonical names.

Before: Canonical names contained pointers to the corresponding Kernel
tree which assumed that the whole tree was in memory whenever the
canonical names were.

Now: Canonical names do not contain these pointers.  They were only
really used to perform name-based lookup in the VM's heap so the
canonical name itself is enough.

If we later find that we need to get from a canonical name to its
Kernel tree we can add an offset in the binary (for instance) to the
canonical name or in a separate mapping on the side.

BUG=
==========

to

==========
Remove definitions from Kernel canonical names.

Before: Canonical names contained pointers to the corresponding Kernel
tree which assumed that the whole tree was in memory whenever the
canonical names were.

Now: Canonical names do not contain these pointers.  They were only
really used to perform name-based lookup in the VM's heap so the
canonical name itself is enough.

If we later find that we need to get from a canonical name to its
Kernel tree we can add an offset in the binary (for instance) to the
canonical name or in a separate mapping on the side.

BUG=
R=asgerf@google.com, vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ed77783cd32d55fdad61bf9bc749030847ba9384
==========

[Kevin Millikin (Google), 2017-03-29 14:03:12]

Committed patchset #3 (id:40001) manually as
ed77783cd32d55fdad61bf9bc749030847ba9384 (presubmit successful).

[Kevin Millikin (Google), 2017-03-30 07:17:13]

Description was changed from

==========
Remove definitions from Kernel canonical names.

Before: Canonical names contained pointers to the corresponding Kernel
tree which assumed that the whole tree was in memory whenever the
canonical names were.

Now: Canonical names do not contain these pointers.  They were only
really used to perform name-based lookup in the VM's heap so the
canonical name itself is enough.

If we later find that we need to get from a canonical name to its
Kernel tree we can add an offset in the binary (for instance) to the
canonical name or in a separate mapping on the side.

BUG=
R=asgerf@google.com, vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ed77783cd32d55fdad61bf9bc749030847ba9384
==========

to

==========
Remove definitions from Kernel canonical names.

Before: Canonical names contained pointers to the corresponding Kernel
tree which assumed that the whole tree was in memory whenever the
canonical names were.

Now: Canonical names do not contain these pointers.  They were only
really used to perform name-based lookup in the VM's heap so the
canonical name itself is enough.

If we later find that we need to get from a canonical name to its
Kernel tree we can add an offset in the binary (for instance) to the
canonical name or in a separate mapping on the side.

BUG=
R=asgerf@google.com, vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ed77783cd32d55fdad61bf9bc749030847ba9384
==========

[Kevin Millikin (Google), 2017-03-30 07:28:14]

kmillikin@google.com changed reviewers:
+ jensj@google.com

[Kevin Millikin (Google), 2017-03-30 07:28:15]

This was reverted because of test failures.

There is nothing really wrong with this change.  It turned some tests that were
failing at runtime by doing the wrong thing into ones that were crashing by
refusing to do the wrong thing.

I've updated the test expectations and filed
https://github.com/dart-lang/sdk/issues/29201 to track the issue.

Please take a quick look.

[jensj, 2017-03-30 07:30:32]

lgtm

[Kevin Millikin (Google), 2017-03-30 07:31:40]

Description was changed from

==========
Remove definitions from Kernel canonical names.

Before: Canonical names contained pointers to the corresponding Kernel
tree which assumed that the whole tree was in memory whenever the
canonical names were.

Now: Canonical names do not contain these pointers.  They were only
really used to perform name-based lookup in the VM's heap so the
canonical name itself is enough.

If we later find that we need to get from a canonical name to its
Kernel tree we can add an offset in the binary (for instance) to the
canonical name or in a separate mapping on the side.

BUG=
R=asgerf@google.com, vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ed77783cd32d55fdad61bf9bc749030847ba9384
==========

to

==========
Remove definitions from Kernel canonical names.

Before: Canonical names contained pointers to the corresponding Kernel
tree which assumed that the whole tree was in memory whenever the
canonical names were.

Now: Canonical names do not contain these pointers.  They were only
really used to perform name-based lookup in the VM's heap so the
canonical name itself is enough.

If we later find that we need to get from a canonical name to its
Kernel tree we can add an offset in the binary (for instance) to the
canonical name or in a separate mapping on the side.

BUG=
R=asgerf@google.com, jensj@google.com, vegorov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ed77783cd32d55fdad61bf9bc749030847ba9384
Committed:
https://github.com/dart-lang/sdk/commit/d562f7ced52c28cd417660128b6bafe44534a422
==========

[Kevin Millikin (Google), 2017-03-30 07:31:42]

Committed patchset #4 (id:60001) manually as
d562f7ced52c28cd417660128b6bafe44534a422 (presubmit successful).