Skip to content

Commit 3199847

Browse files
jensjohacommit-bot@chromium.org
jensjoha
authored and
[email protected]
committed
[CFE] Don't issue bogus NNBD Strong error as package sets a language version
Before this CL, if one compiled a script as strong nnbd (with nnbd enabled), where the script was marked as as a version where nnbd is enabled, but where the script lived in a package with a version that was too low one would get an error. The reason is that the language version is set twice: First when the library is created (but the file isn't read yet). This sets the version from the package. Here an error was issued. The second time is when the file is read and the file-version-marker is parsed (e.g. @Dart = 2.9). The error should only be given if - once the final verison has been set (i.e. after parsing the file) - the version is "wrong". To make matters worse the (bogus) error given even points to the files first position which can make it seem like it specificly points to the @Dart annotation and says it's wrong. This CL fixes the error so it's only issued when it should be. It does not fix pointing semi-weirdly. Fixes #42323 Bug: 42323 Change-Id: Id84ca06b2e4eca5912e65798ed2d86f8c89d71a4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153769 Commit-Queue: Jens Johansen <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 9ae7fd2 commit 3199847

8 files changed

Lines changed: 140 additions & 17 deletions

File tree

‎pkg/front_end/lib/src/fasta/source/source_library_builder.dart‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -453,15 +453,38 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
453453

454454
LanguageVersion get languageVersion => _languageVersion;
455455

456+
void markLanguageVersionFinal() {
457+
if (enableNonNullableInLibrary &&
458+
(loader.nnbdMode == NnbdMode.Strong ||
459+
loader.nnbdMode == NnbdMode.Agnostic)) {
460+
// In strong and agnostic mode, the language version is not allowed to
461+
// opt a library out of nnbd.
462+
if (!isNonNullableByDefault) {
463+
addPostponedProblem(messageStrongModeNNBDButOptOut,
464+
_languageVersion.charOffset, _languageVersion.charCount, fileUri);
465+
_languageVersion = new InvalidLanguageVersion(
466+
fileUri,
467+
_languageVersion.charOffset,
468+
_languageVersion.charCount,
469+
_languageVersion.isExplicit,
470+
loader.target.currentSdkVersion);
471+
}
472+
}
473+
_languageVersion.isFinal = true;
474+
}
475+
456476
@override
457477
void setLanguageVersion(Version version,
458478
{int offset: 0, int length: noLength, bool explicit: false}) {
479+
assert(!_languageVersion.isFinal);
459480
if (languageVersion.isExplicit) return;
460481

461482
if (version == null) {
462483
addPostponedProblem(
463484
messageLanguageVersionInvalidInDotPackages, offset, length, fileUri);
464485
if (_languageVersion is ImplicitLanguageVersion) {
486+
// If the package set an OK version, but the file set an invalid version
487+
// we want to use the package version.
465488
_languageVersion = new InvalidLanguageVersion(
466489
fileUri, offset, length, explicit, loader.target.currentSdkVersion);
467490
library.setLanguageVersion(_languageVersion.version);
@@ -480,6 +503,8 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
480503
length,
481504
fileUri);
482505
if (_languageVersion is ImplicitLanguageVersion) {
506+
// If the package set an OK version, but the file set an invalid version
507+
// we want to use the package version.
483508
_languageVersion = new InvalidLanguageVersion(
484509
fileUri, offset, length, explicit, loader.target.currentSdkVersion);
485510
library.setLanguageVersion(_languageVersion.version);
@@ -490,19 +515,6 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
490515
_languageVersion =
491516
new LanguageVersion(version, fileUri, offset, length, explicit);
492517
library.setLanguageVersion(version);
493-
494-
if (enableNonNullableInLibrary &&
495-
(loader.nnbdMode == NnbdMode.Strong ||
496-
loader.nnbdMode == NnbdMode.Agnostic)) {
497-
// In strong and agnostic mode, the language version is not allowed to
498-
// opt a library out of nnbd.
499-
if (!isNonNullableByDefault) {
500-
addPostponedProblem(
501-
messageStrongModeNNBDButOptOut, offset, length, fileUri);
502-
_languageVersion = new InvalidLanguageVersion(
503-
fileUri, offset, length, explicit, loader.target.currentSdkVersion);
504-
}
505-
}
506518
}
507519

508520
ConstructorReferenceBuilder addConstructorReference(Object name,
@@ -3780,6 +3792,7 @@ class LanguageVersion {
37803792
final int charOffset;
37813793
final int charCount;
37823794
final bool isExplicit;
3795+
bool isFinal = false;
37833796

37843797
LanguageVersion(this.version, this.fileUri, this.charOffset, this.charCount,
37853798
this.isExplicit);
@@ -3807,6 +3820,7 @@ class InvalidLanguageVersion implements LanguageVersion {
38073820
final int charCount;
38083821
final bool isExplicit;
38093822
final Version version;
3823+
bool isFinal = false;
38103824

38113825
InvalidLanguageVersion(this.fileUri, this.charOffset, this.charCount,
38123826
this.isExplicit, this.version);
@@ -3830,6 +3844,7 @@ class InvalidLanguageVersion implements LanguageVersion {
38303844
class ImplicitLanguageVersion implements LanguageVersion {
38313845
@override
38323846
final Version version;
3847+
bool isFinal = false;
38333848

38343849
ImplicitLanguageVersion(this.version);
38353850

‎pkg/front_end/lib/src/fasta/source/source_loader.dart‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,10 @@ class SourceLoader extends Loader {
262262
enableNonNullable: library.isNonNullableByDefault),
263263
languageVersionChanged:
264264
(Scanner scanner, LanguageVersionToken version) {
265-
library.setLanguageVersion(new Version(version.major, version.minor),
266-
offset: version.offset, length: version.length, explicit: true);
265+
if (!suppressLexicalErrors) {
266+
library.setLanguageVersion(new Version(version.major, version.minor),
267+
offset: version.offset, length: version.length, explicit: true);
268+
}
267269
scanner.configuration = new ScannerConfiguration(
268270
enableTripleShift: library.enableTripleShiftInLibrary,
269271
enableExtensionMethods: library.enableExtensionMethodsInLibrary,
@@ -290,6 +292,7 @@ class SourceLoader extends Loader {
290292
importUri, library.fileUri, result.lineStarts, source);
291293
}
292294
library.issuePostponedProblems();
295+
library.markLanguageVersionFinal();
293296
while (token is ErrorToken) {
294297
if (!suppressLexicalErrors) {
295298
ErrorToken error = token;
@@ -354,7 +357,7 @@ class SourceLoader extends Loader {
354357
// Part was included in multiple libraries. Skip it here.
355358
continue;
356359
}
357-
Token tokens = await tokenize(part);
360+
Token tokens = await tokenize(part, suppressLexicalErrors: true);
358361
if (tokens != null) {
359362
listener.uri = part.fileUri;
360363
parser.parseUnit(tokens);

‎pkg/front_end/test/incremental_load_from_dill_suite.dart‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import 'package:front_end/src/api_prototype/experimental_flags.dart';
2626
import "package:front_end/src/api_prototype/memory_file_system.dart"
2727
show MemoryFileSystem;
2828

29+
import 'package:front_end/src/base/nnbd_mode.dart' show NnbdMode;
30+
2931
import 'package:front_end/src/base/processed_options.dart'
3032
show ProcessedOptions;
3133

@@ -112,6 +114,7 @@ class Context extends ChainContext {
112114
@override
113115
Future<void> cleanUp(TestDescription description, Result result) async {
114116
await cleanupHelper?.outDir?.delete(recursive: true);
117+
cleanupHelper?.outDir = null;
115118
}
116119

117120
TestData cleanupHelper;
@@ -328,6 +331,12 @@ class NewWorldTest {
328331
Component component2;
329332
Component component3;
330333

334+
String doStringReplacements(String input) {
335+
String output = input.replaceAll("%NNBD_VERSION_MARKER%",
336+
"${enableNonNullableVersion.major}.${enableNonNullableVersion.minor}");
337+
return output;
338+
}
339+
331340
Future<Null> newWorldTest(
332341
TestData data,
333342
Context context,
@@ -458,6 +467,9 @@ class NewWorldTest {
458467
if (filename == ".packages") {
459468
packagesUri = uri;
460469
}
470+
if (world["enableStringReplacement"] == true) {
471+
data = doStringReplacements(data);
472+
}
461473
fs.entityForUri(uri).writeAsStringSync(data);
462474
}
463475
if (world["dotPackagesFile"] != null) {
@@ -481,6 +493,16 @@ class NewWorldTest {
481493
throw "Error on parsing experiments flags: $e");
482494
options.experimentalFlags = experimentalFlags;
483495
}
496+
if (world["nnbdMode"] != null) {
497+
String nnbdMode = world["nnbdMode"];
498+
switch (nnbdMode) {
499+
case "strong":
500+
options.nnbdMode = NnbdMode.Strong;
501+
break;
502+
default:
503+
throw "Not supported nnbd mode: $nnbdMode";
504+
}
505+
}
484506
}
485507
if (packagesUri != null) {
486508
options.packagesFileUri = packagesUri;

‎pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323.yaml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
# for details. All rights reserved. Use of this source code is governed by a
3+
# BSD-style license that can be found in the LICENSE.md file.
4+
5+
# This is not actually an incremental compiler test as much as a compiler test.
6+
# Test what happens when running a package file as strong NNBD (and it's marked
7+
# with a matching language version) in a package that's with a lower language
8+
# version (nothing should really happen --- it should just work).
9+
# See https://github.com/dart-lang/sdk/issues/42323.
10+
11+
type: newworld
12+
worlds:
13+
- entry: bin/runMe.dart
14+
experiments: non-nullable
15+
nnbdMode: strong
16+
enableStringReplacement: true
17+
sources:
18+
.dart_tool/package_config.json: |
19+
{
20+
"configVersion": 2,
21+
"packages": [
22+
{
23+
"name": "issue42323",
24+
"rootUri": "../",
25+
"packageUri": "lib/",
26+
"languageVersion": "2.8"
27+
}
28+
]
29+
}
30+
bin/runMe.dart: |
31+
// @dart = %NNBD_VERSION_MARKER%
32+
main() {}
33+
expectedLibraryCount: 1

‎pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323.yaml.world.1.expect‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
main = <No Member>;
2+
library from "org-dartlang-test:///bin/runMe.dart" as run {
3+
4+
static method main() → dynamic {}
5+
}

‎pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323_prime.yaml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
# for details. All rights reserved. Use of this source code is governed by a
3+
# BSD-style license that can be found in the LICENSE.md file.
4+
5+
# This is not actually an incremental compiler test as much as a compiler test.
6+
# Test what happens when running a package file as strong NNBD (and it's marked
7+
# with a non-nnbd language version) in a package that's with a lower language
8+
# version (an error should be reported).
9+
# Good "variant" of https://github.com/dart-lang/sdk/issues/42323.
10+
11+
type: newworld
12+
worlds:
13+
- entry: bin/runMe.dart
14+
experiments: non-nullable
15+
nnbdMode: strong
16+
errors: true
17+
sources:
18+
.dart_tool/package_config.json: |
19+
{
20+
"configVersion": 2,
21+
"packages": [
22+
{
23+
"name": "issue42323",
24+
"rootUri": "../",
25+
"packageUri": "lib/",
26+
"languageVersion": "2.8"
27+
}
28+
]
29+
}
30+
bin/runMe.dart: |
31+
// @dart = 2.8
32+
main() {}
33+
expectedLibraryCount: 1

‎pkg/front_end/testcases/incremental_initialize_from_dill/issue_42323_prime.yaml.world.1.expect‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
main = <No Member>;
2+
library from "org-dartlang-test:///bin/runMe.dart" as run {
3+
//
4+
// Problems in library:
5+
//
6+
// org-dartlang-test:///bin/runMe.dart:1:1: Error: A library can't opt out of null safety by default, when using sound null safety.
7+
// // @dart = 2.8
8+
// ^^^^^^^^^^^^^^
9+
//
10+
11+
static method main() → dynamic {}
12+
}

‎tests/dart2js/inferrer_is_int_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// literal might become an int at runtime.
77

88
import "package:expect/expect.dart";
9-
import '../language_2/compiler_annotations.dart';
9+
import '../language/compiler_annotations.dart';
1010

1111
@DontInline()
1212
callWithStringAndDouble(value) {

0 commit comments

Comments
 (0)