Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2741853003:
Document internals of compiler (Closed)
|
Created:
3 years, 9 months ago by Siggi Cherem (dart-lang) Modified:
3 years, 9 months ago CC:
reviews_dartlang.org Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionDocument internals of compiler
BUG=
R=efortuna@google.com, johnniwinther@google.com
Committed: https://github.com/dart-lang/sdk/commit/ff4b06d02e5a256e7de996d649ab632e508d7049
Patch Set 1 #
Total comments: 34
Patch Set 2 : cl comments #Messages
Total messages: 7 (2 generated)
sigmund@google.com changed reviewers: + efortuna@google.com, johnniwinther@google.com, sra@google.com
This is not complete, but a start. We can add more and iterate on this as we go.
lgtm https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md File pkg/compiler/README.md (right): https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:73: 4. **type check**: Type check every reachable member This is a substep of 3 (for each reachable...). https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:110: 4. **type check**: (same as old compiler) Put this after `Resolve it....` https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:390: * `lib/src/core_types.dart`: provides an interface to lookup basic elements Now common_elements.dart https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:408: This is run at a time when no code is generated yet, so the decisions made This is run after resolution. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:428: * `lib/src/enqueue.dart`: this is the basic algorithm that adds thigs as they thigs -> things https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:565: * `tool/perf.dart`: used by our benchmark runners to measure performance of some Long lines, here and below. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:712: `lib/src/tokens` All but `lib/src/tokens/token_map` is now in fasta, and `lib/src/tokens/token_map` is no longer used.
lgtm. Nice writeup, Siggi! I learned a number of things! https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md File pkg/compiler/README.md (right): https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:24: * The kernel ASTs could be used as a model, so this might be a noop or just nit: noop -> no-op https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:180: * **backend**: pieces of the compiler that were target-specific. good one. thanks for this explanation. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:265: * `lib/compiler.dart`: a legacy API that now is implemented by adapting calls to question: does anyone still use this API? https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:272: funtionallity is publicly exposed. funtionallity -> functionality https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:325: corresponding objects. This was added recently to simplify how options where where -> were https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:343: * `lib/src/constants/expressions.dart`: this is how constants are reprensented represented https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:347: command line. In particular, the constant `1 == 1.0` is represented an an -> "as an" ? https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:348: expression that includes this binary expression and `const "this binary expression" being the "==" ? https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:476: * `lib/src/parser/partial_elements.dart`: representation of elements in the ah HA! I had wondered about this. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:566: frontend pieces of dart2js. We shuld be able to deleted it in the near future deleted -> delete
Thanks for the comments! Landing shortly https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md File pkg/compiler/README.md (right): https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:24: * The kernel ASTs could be used as a model, so this might be a noop or just On 2017/03/10 19:31:22, Emily Fortuna wrote: > nit: noop -> no-op Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:73: 4. **type check**: Type check every reachable member On 2017/03/10 13:42:57, Johnni Winther wrote: > This is a substep of 3 (for each reachable...). Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:110: 4. **type check**: (same as old compiler) On 2017/03/10 13:42:57, Johnni Winther wrote: > Put this after `Resolve it....` Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:180: * **backend**: pieces of the compiler that were target-specific. On 2017/03/10 19:31:21, Emily Fortuna wrote: > good one. thanks for this explanation. :) https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:265: * `lib/compiler.dart`: a legacy API that now is implemented by adapting calls to On 2017/03/10 19:31:22, Emily Fortuna wrote: > question: does anyone still use this API? Yes - `pub build` does for sure, dart-pad might as well. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:272: funtionallity is publicly exposed. On 2017/03/10 19:31:22, Emily Fortuna wrote: > funtionallity -> functionality Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:325: corresponding objects. This was added recently to simplify how options where On 2017/03/10 19:31:21, Emily Fortuna wrote: > where -> were Done. thanks :)! https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:343: * `lib/src/constants/expressions.dart`: this is how constants are reprensented On 2017/03/10 19:31:22, Emily Fortuna wrote: > represented Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:347: command line. In particular, the constant `1 == 1.0` is represented an On 2017/03/10 19:31:22, Emily Fortuna wrote: > an -> "as an" ? Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:348: expression that includes this binary expression and `const On 2017/03/10 19:31:22, Emily Fortuna wrote: > "this binary expression" being the "==" ? correct, I went ahead and clarified the text. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:390: * `lib/src/core_types.dart`: provides an interface to lookup basic elements On 2017/03/10 13:42:56, Johnni Winther wrote: > Now common_elements.dart Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:408: This is run at a time when no code is generated yet, so the decisions made On 2017/03/10 13:42:57, Johnni Winther wrote: > This is run after resolution. Thanks, Made this clearer. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:428: * `lib/src/enqueue.dart`: this is the basic algorithm that adds thigs as they On 2017/03/10 13:42:57, Johnni Winther wrote: > thigs -> things Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:476: * `lib/src/parser/partial_elements.dart`: representation of elements in the On 2017/03/10 19:31:22, Emily Fortuna wrote: > ah HA! I had wondered about this. :) https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:565: * `tool/perf.dart`: used by our benchmark runners to measure performance of some On 2017/03/10 13:42:57, Johnni Winther wrote: > Long lines, here and below. Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:566: frontend pieces of dart2js. We shuld be able to deleted it in the near future On 2017/03/10 19:31:21, Emily Fortuna wrote: > deleted -> delete Done. https://codereview.chromium.org/2741853003/diff/1/pkg/compiler/README.md#newc... pkg/compiler/README.md:712: `lib/src/tokens` On 2017/03/10 13:42:56, Johnni Winther wrote: > All but `lib/src/tokens/token_map` is now in fasta, and > `lib/src/tokens/token_map` is no longer used. Done.
Description was changed from ========== Document internals of compiler BUG= ========== to ========== Document internals of compiler BUG= R=efortuna@google.com, johnniwinther@google.com Committed: https://github.com/dart-lang/sdk/commit/ff4b06d02e5a256e7de996d649ab632e508d7049 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as ff4b06d02e5a256e7de996d649ab632e508d7049 (presubmit successful). |
