Make it a compile-time error if a library has `part "uri";` twice for the same uri.

Make it a compile-time error if a library has `part "uri";` twice for the same uri.

If the part declaration is non-empty, it will always cause a compile-time error anyway due to duplicate declarations in the library scope. This just makes it explicit that it's an error in all cases, and avoids the special case of the empty part file being silently accepted until you put something in it.

R=eernst@google.com

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

[Lasse Reichstein Nielsen, 2016-09-26 12:18:35]

lrn@google.com changed reviewers:
+ eernst@google.com, floitsch@google.com

[eernst, 2016-09-26 14:26:37]

LGTM, with a comment that considers whether we should worry about "normalizing"
the URIs before comparing them.

https://codereview.chromium.org/2371743002/diff/1/docs/language/dartLangSpec.tex
File docs/language/dartLangSpec.tex (right):

https://codereview.chromium.org/2371743002/diff/1/docs/language/dartLangSpec....
docs/language/dartLangSpec.tex:6947: 
At first, it seemed reasonable to put a criterion here which allows for
detecting that two textually distinct URIs denote the same part file (I guess
that's not a "library" so we need to say 'file' here).

However, the spec says 'This specification does not discuss the interpretation
of URIs, with the following exceptions.', where the exceptions includes 'any
relative URI is interpreted as relative to the location of the current library.'

So we can have trivial differences (like "foo.d\art" vs. "f\u{6F}o.dart") if
'URI' means "source code" rather than "semantic value of source code", and we
can have non-normalized relative paths. Otherwise we can't have different URIs
and rely on any spec support for concluding that they denote the same file.

I conclude that we can't actually have any meaningful checks for "different URI,
but same file", so I'd tend to prefer that we forget about it.

[Lasse Reichstein Nielsen, 2016-09-26 16:40:57]

https://codereview.chromium.org/2371743002/diff/1/docs/language/dartLangSpec.tex
File docs/language/dartLangSpec.tex (right):

https://codereview.chromium.org/2371743002/diff/1/docs/language/dartLangSpec....
docs/language/dartLangSpec.tex:6947: 
I considered something similar. Since I wrote the current Uri class
implementation, I know exactly which normalizations we apply, but that's not
necessarily the only possible normalizations. Specifying them seems overkill, so
I'd prefer to leave it up to the implementation to decide which URIs are
considered equal and which are not, as long as it's consistent and actually
identical strings do generate equal URIs.

[Lasse Reichstein Nielsen, 2016-10-05 09:12:26]

Description was changed from

==========
Make it a compile-time error if a library has `part "uri";` twice for the same
uri.

If the part declaration is non-empty, it will always cause a compile-time error
anyway due to duplicate declarations in the library scope. This just makes it
explicit that it's an error in all cases, and avoids the special case of the
empty part file being silently accepted until you put something in it.
==========

to

==========
Make it a compile-time error if a library has `part "uri";` twice for the same
uri.

If the part declaration is non-empty, it will always cause a compile-time error
anyway due to duplicate declarations in the library scope. This just makes it
explicit that it's an error in all cases, and avoids the special case of the
empty part file being silently accepted until you put something in it.

R=eernst@google.com

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

[Lasse Reichstein Nielsen, 2016-10-05 09:12:29]

Committed patchset #1 (id:1) manually as
cc6d93b57c697e2afc86431e5b580dc2c1ae89a7 (presubmit successful).

[siva, 2016-10-06 19:35:27]

asiva@google.com changed reviewers:
+ asiva@google.com

[siva, 2016-10-06 19:35:28]

This just seems to add to the parsing cost of having to maintain a list of
'part' URIs seen and checking against this list without any apparent benefit.

We were already reporting duplicate class, field or function errors when a part
'uri' is specified.

[Lasse Reichstein Nielsen, 2016-10-07 06:48:55]

On 2016/10/06 19:35:28, siva wrote:
> This just seems to add to the parsing cost of having to maintain a list of
> 'part' URIs seen and checking against this list without any apparent benefit.
> 
> We were already reporting duplicate class, field or function errors when a
part
> 'uri' is specified.

I wouldn't worry about this in the VM for now.
At some point you will have the Kernel reject the program before it's even
passed to the VM, and until then, any non-empty part that's part'ed more than
once will still be rejected due to the duplicate declarations.