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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 11667012: Convert all symbols accessor to return read only handles so that it is not necessary to create a ne… (Closed)

Created:
8 years ago by siva
Modified:
7 years, 11 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Convert all symbol accessors to return read only handles so that it is not necessary to create a new handle in the code that uses it. Added a few more strings to the symbols list. Committed: https://code.google.com/p/dart/source/detail?r=16584

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 4

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+492 lines, -582 lines) Patch
M lib/double.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M lib/integers.cc View 1 2 3 2 chunks +2 lines, -3 lines 0 comments Download
M lib/isolate.cc View 1 2 3 1 chunk +3 lines, -8 lines 0 comments Download
M vm/ast.h View 1 2 3 4 chunks +5 lines, -5 lines 0 comments Download
M vm/ast.cc View 1 2 3 1 chunk +2 lines, -4 lines 0 comments Download
M vm/class_finalizer.cc View 1 2 3 2 chunks +4 lines, -3 lines 0 comments Download
M vm/code_generator.cc View 1 2 3 4 chunks +8 lines, -8 lines 0 comments Download
M vm/dart_api_impl.cc View 1 2 3 10 chunks +14 lines, -26 lines 0 comments Download
M vm/dart_api_message.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M vm/dart_entry.cc View 1 2 3 11 chunks +22 lines, -42 lines 0 comments Download
M vm/debugger.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M vm/debugger_api_impl.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M vm/exceptions.cc View 1 2 3 1 chunk +13 lines, -13 lines 0 comments Download
M vm/flow_graph_builder.cc View 1 2 3 13 chunks +18 lines, -26 lines 0 comments Download
M vm/flow_graph_compiler_ia32.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M vm/flow_graph_compiler_x64.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M vm/flow_graph_optimizer.cc View 1 2 3 3 chunks +3 lines, -5 lines 0 comments Download
M vm/intermediate_language.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M vm/intermediate_language_ia32.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M vm/intermediate_language_x64.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M vm/intrinsifier_ia32.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M vm/megamorphic_cache_table.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M vm/object.cc View 1 2 3 40 chunks +124 lines, -198 lines 0 comments Download
M vm/object_test.cc View 1 2 3 2 chunks +4 lines, -6 lines 0 comments Download
M vm/parser.cc View 1 2 3 59 chunks +96 lines, -134 lines 0 comments Download
M vm/resolver_test.cc View 1 2 3 3 chunks +3 lines, -6 lines 0 comments Download
M vm/scanner.cc View 1 2 3 1 chunk +3 lines, -6 lines 0 comments Download
M vm/scopes.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M vm/scopes.cc View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M vm/stub_code_ia32.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M vm/stub_code_x64.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M vm/symbols.h View 1 2 3 5 chunks +119 lines, -33 lines 2 comments Download
M vm/symbols.cc View 1 2 3 6 chunks +26 lines, -29 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
siva
8 years ago (2012-12-22 00:30:51 UTC) #1
srdjan
LGTM https://chromiumcodereview.appspot.com/11667012/diff/3007/vm/intermediate_language_ia32.cc File vm/intermediate_language_ia32.cc (right): https://chromiumcodereview.appspot.com/11667012/diff/3007/vm/intermediate_language_ia32.cc#newcode1146 vm/intermediate_language_ia32.cc:1146: 0)); Document why 0. https://chromiumcodereview.appspot.com/11667012/diff/3007/vm/intermediate_language_x64.cc File vm/intermediate_language_x64.cc (right): ...
7 years, 12 months ago (2012-12-26 17:45:29 UTC) #2
siva
https://chromiumcodereview.appspot.com/11667012/diff/3007/vm/intermediate_language_ia32.cc File vm/intermediate_language_ia32.cc (right): https://chromiumcodereview.appspot.com/11667012/diff/3007/vm/intermediate_language_ia32.cc#newcode1146 vm/intermediate_language_ia32.cc:1146: 0)); Added a constant symbol Symbols::kNullCharCodeSymbolOffset and use that ...
7 years, 11 months ago (2013-01-02 19:30:55 UTC) #3
Florian Schneider
DBC: https://chromiumcodereview.appspot.com/11667012/diff/17015/vm/symbols.h File vm/symbols.h (right): https://chromiumcodereview.appspot.com/11667012/diff/17015/vm/symbols.h#newcode118 vm/symbols.h:118: V(JSSyntaxRegExp, "_JSSyntaxRegExp") \ Maybe also rename the other ...
7 years, 11 months ago (2013-01-03 14:39:51 UTC) #4
siva
7 years, 11 months ago (2013-01-03 17:54:35 UTC) #5
Message was sent while issue was closed. 
https://chromiumcodereview.appspot.com/11667012/diff/17015/vm/symbols.h
File vm/symbols.h (right):

https://chromiumcodereview.appspot.com/11667012/diff/17015/vm/symbols.h#newco...
vm/symbols.h:118: V(JSSyntaxRegExp, "_JSSyntaxRegExp")                          
              \
Good point.

I will rename these to be consistent with the _ names.

On 2013/01/03 14:39:51, Florian Schneider wrote:
> Maybe also rename the other accessors for private names:
> 
> V(_JSSyntaxRegExp, "_JSSyntaxRegExp") 
> 
> I don't have a strong preference, but I'd consistently insert or leave out the
> "_" on the name of the accessors for private name symbols.

Powered by Google App Engine
This is Rietveld 408576698