Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vm] Default values of noSuchMethod forwarder are missed in case of mixin application #53656

Closed
sgrekhov opened this issue Sep 29, 2023 · 5 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.

Comments

@sgrekhov
Copy link
Contributor

According to Dart specification "10.2.2 The Method noSuchMethod"

A noSuchMethod forwarder is a concrete member of C with the signature
taken from the interface of C, and with the same default value for each optional
parameter

But in case of mixin application on VM default values of optional parameters are missed. Proof

String log = "";

class C {
  int m1(int v, [String s = "s1"]);

  int m2(int v, {String s = "s2"});

  dynamic noSuchMethod(Invocation inv) {
    for (int i = 0; i < inv.positionalArguments.length; i++) {
      log += "${inv.positionalArguments[i]};";
    }
    for (int i = 0; i < inv.namedArguments.length; i++) {
      log += "s=${inv.namedArguments[Symbol("s")]};";
    }
    return 42;
  }
}

mixin M {
  int m1(int v, [String s = "s1"]);

  int m2(int v, {String s = "s2"});

  dynamic noSuchMethod(Invocation inv) {
    for (int i = 0; i < inv.positionalArguments.length; i++) {
      log += "${inv.positionalArguments[i]};";
    }
    for (int i = 0; i < inv.namedArguments.length; i++) {
      log += "s=${inv.namedArguments[Symbol("s")]};";
    }
    return 42;
  }
}

class MA = Object with M;

main() {
  C().m1(1);
  print(log); // 1;s1;
  log = "";
  C().m2(2);
  print(log); // 2;s=s2;
  log = "";

  MA().m1(1);
  print(log); // 1;
  log = "";
  MA().m2(2);
  print(log); // 2;
}

No this issue on DartPad. DartPad output is

1;s1;
2;s=s2;
1;s1;
2;s=s2;

So, I believe, this is VM issue

Tested on Dart SDK version: 3.2.0-202.0.dev (dev) (Tue Sep 26 21:06:42 2023 -0700) on "windows_x64"

@sgrekhov sgrekhov added the area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. label Sep 29, 2023
@eernstg
Copy link
Member

eernstg commented Sep 29, 2023

We do have an ongoing discussion about the semantics of this exact topic in dart-lang/language#3331, but I believe it's safe to say that this example is outside that discussion: There is no doubt that the noSuchMethod forwarders should be used, and they should use the default value from the declarations that introduced this member into the interface when said default value is well defined (that is, we have one value, not zero and not more than one).

However, @johnniwinther, I would expect noSuchMethod forwarders/throwers to be generated by the CFE, and I thought that every backend would simply rely on using the default value which was specified explicitly in the generated forwarder. Is that not correct?

@lrhn
Copy link
Member

lrhn commented Sep 29, 2023

It's likely that the difference comes from the mixin application not making a copy of the abstract method declarations of the mixin into the application class.
These members are only introduced as signatures through the implicit implements M, and interface signatures do not have default values.

And that's a point where the specification is misleading, when it says the signature of the interface C, and with the same default values, when the interface has no default values.

@eernstg
Copy link
Member

eernstg commented Sep 29, 2023

These members are only introduced as signatures through the implicit implements M

We have specified that noSuchMethod forwarders are concrete member declarations, though implicitly induced (in the example: into the class C and into the class MA), very much like the getters/setters that are implicitly induced by variable declarations. A noSuchMethod forwarding method is not just a member signature, if it were then it couldn't be torn off.

Moreover, noSuchMethod forwarders should have 'the same default value for each optional parameter'.

This particular phrase is ambiguous, as discussed in dart-lang/language#3331, but I don't think there is any reasonable interpretation of this phrase and the text around it which amounts to "just forget all about the default values".

@biggs0125
Copy link

I'm working on this change which might address these missing default values as well:
https://dart-review.googlesource.com/c/sdk/+/328601

@alexmarkov
Copy link
Contributor

The incorrect behavior is caused by the missing forwarding stub in the mixin application class. Duplicate to #53677.

@alexmarkov alexmarkov self-assigned this Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

5 participants