Sdk: Analyzer CLI no longer detects duplicate parts

Created on 13 Jun 2016 · 11Comments · Source: dart-lang/sdk

The co19 test Language/Libraries_and_Scripts/Parts/compilation_t02 now fails on the analyzer, because the command-line analyzer no longer detects that the same part is included twice after cache dependencies are no longer checked, a change introduced in
https://codereview.chromium.org/2058253002

Is this intended?

@scheglov @bwilkerson

area-language type-bug

Source

whesse

Most helpful comment

I would be in favor of disallowing it.

bwilkerson on 24 Sep 2016

👍2

All 11 comments

The test Language/Libraries_and_Scripts/Parts/compilation_t01 also fails. It no longer detects that the same function is declared in both the part and the main file.

whesse on 13 Jun 2016

No, it isn't intended.

bwilkerson on 13 Jun 2016

That's strange.
I'm pretty sure I ran all the tests before committing the change.

And these tests pass even now, after I rollback the status suppression.

scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk ((093672f...))$ d-rat
Please set JAVA_HOME to /Library/Java/JavaVirtualMachines/jdk1.8.0_91.jdk/Contents/Home
xcodebuild -project dart.xcodeproj -target packages -configuration ReleaseIA32 SYMROOT=/Users/scheglov/Source/Dart/sdk.git/sdk/xcodebuild
Build settings from command line:
    SYMROOT = /Users/scheglov/Source/Dart/sdk.git/sdk/xcodebuild

** BUILD SUCCEEDED **

Test configuration: dart2analyzer_none_release_ia32
Total: 17649 tests
 * 122 tests will be skipped (18 skipped by design)
 * 35 tests are expected to be flaky but not crash
 * 0 tests are expected to flaky crash
 * 16427 tests are expected to pass
 * 29 tests are expected to fail that we won't fix
 * 1022 tests are expected to fail that we should fix
 * 2 tests are expected to crash that we should fix
 * 0 tests are allowed to timeout
 * 0 tests are skipped on browsers due to compile-time error
 * 12 could not be categorized or are in multiple categories

[02:07 | 100% | +17527 | -    0]

They also work in manual testing:

scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk/tests/co19/src ((3f0a4bc...))$
scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts ((3f0a4bc...))$ d-ra compilation_t01.dart
Analyzing [compilation_t01.dart]...
[error] The name 'foo' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 12, col 7)
1 error found.

scheglov@scheglov-macbookpro3:~/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts ((3f0a4bc...))$ d-ra compilation_t02.dart
Analyzing [compilation_t02.dart]...
[error] The name 'Annot' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 8, col 7)
[error] The name 'foo' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 12, col 7)
[error] The name 'bar' is already defined (/Users/scheglov/Source/Dart/sdk.git/sdk/tests/co19/src/Language/Libraries_and_Scripts/Parts/part_0.dart, line 13, col 5)
3 errors found.

scheglov on 13 Jun 2016

Yes, these are being flakey on the buildbot. So there may be some race condition, or timing, that affects this. I'll think about ways to reproduce this. This is on linux, btw, so maybe it doesn't happen on mac.

whesse on 13 Jun 2016

As I can see, the specification does not disallow referring to the same part twice. We started discussing this, but there was no resolution, and the specification has not been changes yet.

It seems like a right thing to do.
Who is in charge of the specification these days? @kevmoo @munificent @lrhn ?

scheglov on 23 Sep 2016

The language team is in charge of the specification, and they are already considering changing parts to use URI's rather than names, which will resolve this issue.

bwilkerson on 23 Sep 2016

Will it? This issue is not about part of, it's about part "my_part.dart" twice in the same library.

scheglov on 23 Sep 2016

Oh, sorry, I didn't read carefully enough. No, this is unrelated. I'll send an e-mail to the language team.

bwilkerson on 23 Sep 2016

There are basically three solutions:

Disallow it.
Allow it and let every part declaration become malformed due to duplicate declaration.
Allow it and if a part if included twice, it's only used once anyway.

The third is what we do for libraries - if you import the same library twice, it works because both imported names refer to the same declaration. I think we can do something similar for part inclusions.
Like for libraries, it means that you must always fetch each URI at most once. If a library has the same part twice and it fetches it twice from a server that sends different files, then everything is a mess.

The second is what the current specification give us. If you include the contents of the part file twice, it means that its declarations are declared twice in the same scope. Well, maybe, it's not precisely specified what