VM inconsistent handling of fromEnvironment when running from sources

[sigmundch]

The default value of String.fromEnvironment behaves differently when using kernel inputs (or snapshots) than when running from source on the VM.

Here is a sample repro:

main() => print(const String.fromEnvironment("HI")  == "");

When running:

out/ReleaseX64/dart foo.dart  # (a)
out/ReleaseX64NNBD/dart --enable-experiment=non-nullable foo.dart # (b)
./pkg/front_end/tool/fasta compile --target vm --platform out/ReleaseX64NNBD/vm_platform_strong.dill  --enable-experiment=non-nullable  foo.dart
out/ReleaseX64NNBD/dart --enable-experiment=non-nullable foo.dart.dill # (c)

(a) prints false
(b) prints true
(c) prints false

dart2js in NNBD prints false in this example as well.

The constant evaluation logic in the CFE is looking at the value of defaultValue from the call site, but it is not processing the default value from the constructor definition at this time, so I can see why the kernel file contains the constant null and why dart2js also prints false. I thought however that all constant evaluation was done in the CFE, so I was surprised to see that (b) prints true.

Note: the breaking change to make String.fromEnvironment have a default value is not finalized yet: #40678

If the decision is to move forward with the change to use defaultValue='', the constant evaluator needs to be updated here to handle it properly:
https://github.com/dart-lang/sdk/blob/master/pkg/front_end/lib/src/fasta/kernel/constant_evaluator.dart#L1988

/cc @johnniwinther - FYI this is a piece in the CFE constant evaluator that may need attention. Currently it embeds the default value implicitly with that "null", you may be able to extract the default value from the target procedure instead.

[sigmundch]

(sorry I made a couple typos in my original repro instructions, went ahead and updated them in place)

[alexmarkov]

Similar to #40501.

[johnniwinther]

With https://dart-review.googlesource.com/c/sdk/+/139642 the CFE now reads the default value from the declaration.

[liamappelbe]

I've confirmed this was fixed by the fixes to #40501