Kernel driver: pass options and use it for its compiler context

Kernel driver: pass options and use it for its compiler context

This ensures we don't accidentally override options (e.g. strongMode was being
overriden accidentally as a result).

R=scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/13a4906be935281832a0615363c3f67d918162a0

[Siggi Cherem (dart-lang), 2017-07-31 20:04:58]

sigmund@google.com changed reviewers:
+ scheglov@google.com

[Siggi Cherem (dart-lang), 2017-07-31 20:04:59]

https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):

https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
pkg/front_end/lib/src/incremental/kernel_driver.dart:64: final ProcessedOptions
_options;
Now that I'm adding this here, technically we could also remove _fileSystem,
_byteStore, _logger (and use _options.* for those).

Happy to do that change together with this CL if you prefer, if not, we can do
that as a follow up.

We could also technically pull out uriTranslator as well, but since it's async
it might need some more work.

[scheglov, 2017-07-31 20:12:40]

LGTM

https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):

https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
pkg/front_end/lib/src/incremental/kernel_driver.dart:64: final ProcessedOptions
_options;
On 2017/07/31 20:04:59, Siggi Cherem (dart-lang) wrote:
> Now that I'm adding this here, technically we could also remove _fileSystem,
> _byteStore, _logger (and use _options.* for those).
> 
> Happy to do that change together with this CL if you prefer, if not, we can do
> that as a follow up.
> 
> We could also technically pull out uriTranslator as well, but since it's async
> it might need some more work.

Let's leave it as is for now, I will review how often these fields are used and
decide whether to pull them, of just access directly from the options object.

[Siggi Cherem (dart-lang), 2017-07-31 20:18:35]

On 2017/07/31 20:12:40, scheglov wrote:
> LGTM
> 
>
https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
> File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):
> 
>
https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
> pkg/front_end/lib/src/incremental/kernel_driver.dart:64: final
ProcessedOptions
> _options;
> On 2017/07/31 20:04:59, Siggi Cherem (dart-lang) wrote:
> > Now that I'm adding this here, technically we could also remove _fileSystem,
> > _byteStore, _logger (and use _options.* for those).
> > 
> > Happy to do that change together with this CL if you prefer, if not, we can
do
> > that as a follow up.
> > 
> > We could also technically pull out uriTranslator as well, but since it's
async
> > it might need some more work.
> 
> Let's leave it as is for now, I will review how often these fields are used
and
> decide whether to pull them, of just access directly from the options object.

Sounds good. Makes sense to keep some of the fields, then we would just remove
the arguments from the constructor to ensure we don't get a mix of arguments
that are inconsistent (e.g. a filesystem that doens't match options.fileSystem),
right?

[scheglov, 2017-07-31 20:21:14]

On 2017/07/31 20:18:35, Siggi Cherem (dart-lang) wrote:
> On 2017/07/31 20:12:40, scheglov wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
> > File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):
> > 
> >
>
https://codereview.chromium.org/2992773002/diff/20001/pkg/front_end/lib/src/i...
> > pkg/front_end/lib/src/incremental/kernel_driver.dart:64: final
> ProcessedOptions
> > _options;
> > On 2017/07/31 20:04:59, Siggi Cherem (dart-lang) wrote:
> > > Now that I'm adding this here, technically we could also remove
_fileSystem,
> > > _byteStore, _logger (and use _options.* for those).
> > > 
> > > Happy to do that change together with this CL if you prefer, if not, we
can
> do
> > > that as a follow up.
> > > 
> > > We could also technically pull out uriTranslator as well, but since it's
> async
> > > it might need some more work.
> > 
> > Let's leave it as is for now, I will review how often these fields are used
> and
> > decide whether to pull them, of just access directly from the options
object.
> 
> Sounds good. Makes sense to keep some of the fields, then we would just remove
> the arguments from the constructor to ensure we don't get a mix of arguments
> that are inconsistent (e.g. a filesystem that doens't match
options.fileSystem),
> right?

Correct.

[Siggi Cherem (dart-lang), 2017-07-31 20:22:56]

Description was changed from

==========
Kernel driver: pass options and use it for its compiler context

This ensures we don't accidentally override options (e.g. strongMode was being
overriden accidentally as a result).
==========

to

==========
Kernel driver: pass options and use it for its compiler context

This ensures we don't accidentally override options (e.g. strongMode was being
overriden accidentally as a result).

R=scheglov@google.com

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

[Siggi Cherem (dart-lang), 2017-07-31 20:22:57]

Committed patchset #2 (id:20001) manually as
13a4906be935281832a0615363c3f67d918162a0 (presubmit successful).