Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(17)

Issue 99373002: Mark native functions when they are created. (Closed)

Created:
7 years ago by Florian Schneider
Modified:
7 years ago
Reviewers:
srdjan
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Mark native functions when they are created. Until now the parser marked native functions as native when parsing. This may be too late for some functions. E.g. the native typed list constructor may not be invoked because the intrinsic code is executed instead (unless for example new-space allocation fails). This causes missing type information when optimizing functions using those recognized factory functions like Uint8List._new. R=srdjan@google.com Committed: https://code.google.com/p/dart/source/detail?r=30846

Patch Set 1 #

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -31 lines) Patch
M runtime/vm/class_finalizer.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M runtime/vm/code_descriptors_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/code_patcher_arm_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/code_patcher_ia32_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/code_patcher_mips_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/code_patcher_x64_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/compiler.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M runtime/vm/megamorphic_cache_table.cc View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/object.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/object.cc View 8 chunks +7 lines, -1 line 0 comments Download
M runtime/vm/object_test.cc View 5 chunks +10 lines, -8 lines 0 comments Download
M runtime/vm/parser.cc View 17 chunks +20 lines, -3 lines 0 comments Download
M runtime/vm/resolver.cc View 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/runtime_entry_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/stub_code_arm_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/stub_code_ia32_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/stub_code_mips_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/stub_code_x64_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/vm/symbols.h View 2 chunks +4 lines, -4 lines 0 comments Download
M runtime/vm/unit_test.cc View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Florian Schneider
7 years ago (2013-12-02 15:41:38 UTC) #1
srdjan
LGTM. The Function::New has become almost unreadable as it has too many arguments. I like ...
7 years ago (2013-12-02 18:32:34 UTC) #2
Florian Schneider
Committed patchset #2 manually as r30846 (presubmit successful).
7 years ago (2013-12-04 10:32:56 UTC) #3
Florian Schneider
7 years ago (2013-12-04 10:35:18 UTC) #4
Message was sent while issue was closed.
On 2013/12/02 18:32:34, srdjan wrote:
> LGTM. 
> 
> The Function::New has become almost unreadable as it has too many arguments. I
> like having to set native flag at construction time, not explicitly via a
call.
> Any idea how to make the function factories simpler?

It is pretty complicated. The alternative (not passing a constructor argument)
is not ideal either. It is too easy to forget to set the flags correctly after
construction.

Maybe multiple factory methods like NewNativeFunction, NewStaticFunction, etc.
would be better? What do you think?

Powered by Google App Engine
This is Rietveld 408576698