Sdk: DDC: "undefined" leaked by conditional return breaks default arguments value

[greglittlefield-wf]

We're running into some cases where, in DDC, a Dart function is returning undefined, and when that value is being passed into a function with conditional arguments, it's treating it as an unspecified argument.

Reduced test case

void main() {
  // Prints "arg: default value".
  fnWithDefaultArgValue();
  // Prints "arg: returned value".
  fnWithDefaultArgValue(fnWithConditionalReturn(true));
  // In DDC, unexpectedly prints "arg: default value".
  // In dart2js, prints "arg: null" as expected.
  fnWithDefaultArgValue(fnWithConditionalReturn(false));
}

void fnWithDefaultArgValue([dynamic arg = 'default value']) {
  print('arg: $arg');
}

dynamic fnWithConditionalReturn(bool shouldReturn) {
  if (shouldReturn) {
    return 'returned value';
  }
}

Compiled DDC:

  foo.fnWithDefaultArgValue = function(arg) {
    if (arg === void 0) arg = "default value";
    core.print("arg: " + dart.str(arg));
  };
  foo.fnWithConditionalReturn = function(shouldReturn) {
    if (dart.test(shouldReturn)) {
      return "returned value";
    }
  };

Here fnWithConditionalReturn is returning undefined when shouldReturn is false, causing the if (arg === void 0) check to pass, even though the argument was specified.

To help guard against other cases where undefined is leaked, could this instead check arguments.length? Or should most of those leaks get fixed instead?

---

Also, tangentially related, is there a reason these parameters are initialized via separate statements, as opposed to in ES6 default parameter values?

[todd-beckman]

I second the question about the default parameters. I believe that would be the preferable option, but I am not sure if there was a reason against it.

If there is a good reason not to use default parameters, the arguments.length approach is pretty straightforward. I have a PR for this here: https://dart-review.googlesource.com/c/sdk/+/99714.