CFE resolves top-level variable to null with constants flag

[jmesserly]

Given this example:

import 'dart:convert' show json;

main() {
  print(json.decode('{}'));
}

With DDC's Kernel backend, and the constant patches from: #36479, DDC is getting the following Kernel tree:

main = test::main;
library from "file:///Users/jmesserly/scratch/test.dart" as test {

  import "dart:convert";

  static method main() → dynamic {
    core::print((#C1).{con::JsonCodec::decode}("{}"));
  }
}
constants  {
  #C1 = null
}

This leads it to output null.decode. This is causing (at least some of) the failures in this test run:
https://ci.chromium.org/p/dart/builders/try/ddc-linux-release-chrome-try/16764

For example, tests/language_2/map_test.dart fails due to the line above. The "show" doesn't matter.

I'm guessing this is an issue with cross-module constants? I'll poke around and see if I can figure it out. (also I'll sync to HEAD, in case this was fixed since my test run last night).

[jmesserly]

Note, the fix described here: #36535 (comment) would probably fix this issue too.

[jmesserly]

CC @askeksa-google ... I think I found a simple fix:

diff --git a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
index d35150db18..a43c39a7e3 100644
--- a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
@@ -360,7 +360,7 @@ class ConstantsTransformer extends Transformer {
   visitStaticGet(StaticGet node) {
     final Member target = node.target;
     if (target is Field && target.isConst) {
-      return evaluateAndTransformWithContext(node, target.initializer);
+      return evaluateAndTransformWithContext(node, node);
     } else if (target is Procedure && target.kind == ProcedureKind.Method) {
       return evaluateAndTransformWithContext(node, node);
     }

Alternatively the code could inline the target.isInExternalLibrary && target.initializer == null check from ConstantEvaluator.visitStaticGet. Basically it looks like transformer's visitStaticGet is trying to inline some of that logic (for performance?) so it misses the case of unevaluated constants.

---

I'm now testing a version of this fix, that also includes an attempt at fixing #36535 ... but I'm not sure if the APIs are what the CFE would want.

[jmesserly]

FWIW, the other way of fixing it would be:

diff --git a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
index d35150db18..994c1610f1 100644
--- a/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart
@@ -359,7 +359,8 @@ class ConstantsTransformer extends Transformer {
 
   visitStaticGet(StaticGet node) {
     final Member target = node.target;
-    if (target is Field && target.isConst) {
+    if (target is Field && target.isConst &&
+       (target.initializer != null || !target.isInExternalLibrary)) {
       return evaluateAndTransformWithContext(node, target.initializer);
     } else if (target is Procedure && target.kind == ProcedureKind.Method) {
       return evaluateAndTransformWithContext(node, node);

I verified that this fixes the issue too.

[askeksa-google]

I noticed this problem as well when looking at the code, but didn't mention it, since https://dart-review.googlesource.com/c/sdk/+/98677 will fix it (it includes the same fix as your first suggestion).

That part of the code will be rewritten by the fix for #36535 anyway (somewhat similarly to your second suggestion).

[kmillikin]

@jmesserly can you verify this is fixed?

[jmesserly]

I can confirm it's fixed!

I'm still working on fixing DDC with the constants flag; I found some unusual behavior that I'm not sure is intended (e.g. annotations appearing as ConstantExpressions whose value is an UnevaluatedConstant). It happens only with modular compilation+outline Kernel files. But I have a workaround for that, so it's not blocking.