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

Issue 2871573003: Update DeltaProgram API and documentation. (Closed)

Created:
3 years, 7 months ago by scheglov
Modified:
3 years, 7 months ago
CC:
reviews_dartlang.org, dart-fe-team+reviews_google.com
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -18 lines) Patch
M pkg/front_end/lib/incremental_kernel_generator.dart View 1 chunk +13 lines, -4 lines 3 comments Download
M pkg/front_end/lib/src/incremental_kernel_generator_impl.dart View 1 chunk +1 line, -1 line 0 comments Download
M pkg/front_end/test/incremental_kernel_generator_test.dart View 4 chunks +7 lines, -13 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
scheglov
3 years, 7 months ago (2017-05-08 21:02:35 UTC) #1
Paul Berry
lgtm
3 years, 7 months ago (2017-05-08 21:12:16 UTC) #2
scheglov
Committed patchset #1 (id:1) manually as 9167ad0f55c928616d86e5e9a1ea6cbd49e61608 (presubmit successful).
3 years, 7 months ago (2017-05-08 21:13:16 UTC) #4
Siggi Cherem (dart-lang)
3 years, 7 months ago (2017-05-08 22:49:36 UTC) #5
Message was sent while issue was closed.
Let me know if we should chat more about it. I'm OK with the change because this
what we need to deliver right now, but it's true that the future is closer to
the API we had before (more details in my comment below)

https://codereview.chromium.org/2871573003/diff/1/pkg/front_end/lib/increment...
File pkg/front_end/lib/incremental_kernel_generator.dart (right):

https://codereview.chromium.org/2871573003/diff/1/pkg/front_end/lib/increment...
pkg/front_end/lib/incremental_kernel_generator.dart:25: /// TODO(scheglov) With
`main()` or entry point URI?
for the purpose of the implementation here - anything reachable from the entry
[source] URI given to the incremental generator. For hot-reload we'll always
give you the source that contains "main" as [source].

https://codereview.chromium.org/2871573003/diff/1/pkg/front_end/lib/increment...
pkg/front_end/lib/incremental_kernel_generator.dart:27: /// Also includes
outlines for the transitive closure of libraries that are
Using the terminology from earlier today, I'd rephrase this as:

  Also includes external references to other libraries 
  that were not modified or affected.

I want to especially keep it open that it may not include the entire outline of
those libraries.

https://codereview.chromium.org/2871573003/diff/1/pkg/front_end/lib/increment...
pkg/front_end/lib/incremental_kernel_generator.dart:32:
DeltaProgram(this.newProgram);
Let's add a TODO saying that [Program] will become a different type?

This is where I expect we'll have the "Bundle of modular builds".

The concept of "bundles" doesn't exist in Kernel yet, but what we want is a way
to put together separately compiled libraries without having to link references
by hand here.

In a way I think this why we had a Map<Uri, Program>. Each entry in the Map was
the modular-build of a library. The "Program" would have a null mainMethod, and
would mainly have the kernel of a library and external references to all other
libraries. Some libraries would be mentioned several times, but only one of the
entries in the Map would have the body of that library (all other references are
"external").

Powered by Google App Engine
This is Rietveld 408576698