Make parts hermetic, fixes #531

Make parts hermetic, fixes #531

R=paulberry@google.com, sigmund@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/38e8a7622950f5fd59b406bc80866e5fbeb1fd1b

[Jennifer Messerly, 2016-04-28 20:03:46]

jmesserly@google.com changed reviewers:
+ paulberry@google.com, sigmund@google.com

[Paul Berry, 2016-04-28 20:12:33]

lgtm

https://codereview.chromium.org/1930133002/diff/1/lib/src/compiler/compiler.dart
File lib/src/compiler/compiler.dart (right):

https://codereview.chromium.org/1930133002/diff/1/lib/src/compiler/compiler.d...
lib/src/compiler/compiler.dart:59: var implicitParts = new HashSet<Source>();
Nit: this is a confusing name.  It seems to imply that we are binning parts into
two categories (those that are explicit and those that are implicit), so no part
should ever wind up in both sets.  But that's not what's happening; under normal
conditions both sets will be equal.

Consider renaming to something like `usedParts`.

https://codereview.chromium.org/1930133002/diff/1/lib/src/compiler/compiler.d...
lib/src/compiler/compiler.dart:294: final unusedPartErrorCode = const
CompileTimeErrorCode(
Should this really be an error?  It's harmless, and I could see it arising as
part of a normal iterative development process.

[Jennifer Messerly, 2016-04-28 20:18:24]

Thanks Paul, great feedback, PTAL

https://codereview.chromium.org/1930133002/diff/1/lib/src/compiler/compiler.dart
File lib/src/compiler/compiler.dart (right):

https://codereview.chromium.org/1930133002/diff/1/lib/src/compiler/compiler.d...
lib/src/compiler/compiler.dart:59: var implicitParts = new HashSet<Source>();
On 2016/04/28 20:12:33, Paul Berry wrote:
> Nit: this is a confusing name.  It seems to imply that we are binning parts
into
> two categories (those that are explicit and those that are implicit), so no
part
> should ever wind up in both sets.  But that's not what's happening; under
normal
> conditions both sets will be equal.
> 
> Consider renaming to something like `usedParts`.

good idea, done

https://codereview.chromium.org/1930133002/diff/1/lib/src/compiler/compiler.d...
lib/src/compiler/compiler.dart:294: final unusedPartErrorCode = const
CompileTimeErrorCode(
On 2016/04/28 20:12:33, Paul Berry wrote:
> Should this really be an error?  It's harmless, and I could see it arising as
> part of a normal iterative development process.

Yeah, that's a good question. I'm fine taking it out. It just seemed odd we were
asked to compile a file and decided to silently ignore it. :)

Changed to be a Warning. Happy to downgrade to a hint too.

[Paul Berry, 2016-04-28 20:21:14]

lgtm

[Siggi Cherem (dart-lang), 2016-04-28 20:35:33]

lgtm

[Jennifer Messerly, 2016-04-28 20:37:35]

Description was changed from

==========
Make parts hermetic, fixes #531
==========

to

==========
Make parts hermetic, fixes #531

R=paulberry@google.com, sigmund@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/38e8a7622950f5fd59b406bc8086...
==========

[Jennifer Messerly, 2016-04-28 20:37:36]

Committed patchset #3 (id:40001) manually as
38e8a7622950f5fd59b406bc80866e5fbeb1fd1b (presubmit successful).