[vm] Fix ArgumentsDescriptor uses in DoArgumentTypesMatch.

Use of PositionalCount() and PositionAt() as indices in the arguments
array must account for the type arguments if present. Otherwise, we'll
either skip checking the last positional argument (in the former case)
or check against the wrong arguments (in the latter case).

In nosuchmethod_forwarding_arguments_test.dart, add cases that check for
the above mistakes.

In require_named_args_strong_test.dart, use the more specific
throwsTypeError or throwsNoSuchMethod checks instead of the generic
throwsError to ensure the correct error is thrown.

TEST=Changed tests to add extra cases for failures that can happen.

Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try
Change-Id: If5e6c310d36d244bb0650ded54e32e583732584e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171947
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>

runtime/vm/object.cc

@@ -8135,8 +8135,8 @@ ObjectPtr Function::DoArgumentTypesMatch(
   const intptr_t arg_offset = args_desc.FirstArgIndex();
   // Only check explicit arguments.
   const intptr_t arg_start = arg_offset + NumImplicitParameters();
-  const intptr_t num_positional_args = args_desc.PositionalCount();
-  for (intptr_t arg_index = arg_start; arg_index < num_positional_args;
+  const intptr_t end_positional_args = arg_offset + args_desc.PositionalCount();
+  for (intptr_t arg_index = arg_start; arg_index < end_positional_args;
        ++arg_index) {
     argument ^= args.At(arg_index);
     // Adjust for type arguments when they're present.
@@ -8166,7 +8166,7 @@ ObjectPtr Function::DoArgumentTypesMatch(
        named_index++) {
     argument_name = args_desc.NameAt(named_index);
     ASSERT(argument_name.IsSymbol());
-    argument ^= args.At(args_desc.PositionAt(named_index));
+    argument ^= args.At(arg_offset + args_desc.PositionAt(named_index));
 
     // Try to find the named parameter that matches the provided argument.
     // Even when annotated with @required, named parameters are still stored

tests/language/nnbd/required_named_parameters/required_named_args_strong_test.dart

@@ -12,7 +12,7 @@ main() {
   dynamic f = func;
 
   // Invalid: Subtype may not redeclare optional parameters as required.
-  Expect.throws(() {
+  Expect.throwsTypeError(() {
     Function(
       String p0, {
       required int p1,
@@ -21,7 +21,7 @@ main() {
   });
 
   // Invalid: Subtype may not declare new required named parameters.
-  Expect.throws(() {
+  Expect.throwsTypeError(() {
     Function(
       String p0, {
       required int p1,
@@ -29,18 +29,18 @@ main() {
   });
 
   // Invalid: Invocation with explicit null required named argument.
-  Expect.throws(() {
+  Expect.throwsTypeError(() {
     f("", p1: null, p2: null);
   });
-  Expect.throws(() {
+  Expect.throwsTypeError(() {
     Function.apply(f, [""], {#p1: null, #p2: null});
   });
 
   // Invalid: Invocation that omits a required named argument.
-  Expect.throws(() {
+  Expect.throwsNoSuchMethodError(() {
     f("", p1: 100);
   });
-  Expect.throws(() {
+  Expect.throwsNoSuchMethodError(() {
     Function.apply(f, [""], {#p1: 100});
   });
 }

tests/language/nosuchmethod_forwarding/nosuchmethod_forwarding_arguments_test.dart

@@ -5,6 +5,9 @@
 // Testing that `noSuchMethod` forwarding properly handles optional, named and
 // type parameters, and result type checking.
 
+// VMOptions=--lazy-dispatchers
+// VMOptions=--no-lazy-dispatchers
+
 import 'package:expect/expect.dart';
 
 class A {
@@ -48,6 +51,16 @@ class A {
     } else if (invoke.memberName == #test8) {
       Expect.equals(1, invoke.positionalArguments.length);
       Expect.equals(null, invoke.positionalArguments[0]);
+    } else if (invoke.memberName == #test9) {
+      Expect.equals(invoke.typeArguments.length, 2);
+      Expect.equals(invoke.typeArguments[0].toString(), "num");
+      Expect.equals(invoke.typeArguments[1].toString(), "double");
+
+      Expect.equals(1, invoke.positionalArguments.length);
+      Expect.equals(4.2, invoke.positionalArguments[0]);
+
+      Expect.equals(1, invoke.namedArguments.length);
+      Expect.equals(3, invoke.namedArguments[#foo]);
     }
   }
 
@@ -63,6 +76,7 @@ class A {
   void set test7(int x);
 
   void test8([String? x]);
+  void test9<T, S extends T>(S x1, {T? foo});
 
   T allTogetherNow<T, S extends T>(S x1, {List<T> foo: const <Never>[]});
 }
@@ -97,6 +111,25 @@ main() {
   Expect.throwsTypeError(() => (a as dynamic).test7 = "hi");
 
   a.allTogetherNow<num, double>(2.0, foo: const <num>[3, 4]);
+  Expect.throwsTypeError(() =>
+      (a.allTogetherNow as dynamic)<int, double>(2.0, foo: const <num>[3, 4]));
+  Expect.throwsTypeError(() =>
+      (a.allTogetherNow as dynamic)<int, int>(2.0, foo: const <num>[3, 4]));
+  Expect.throwsTypeError(() => (a.allTogetherNow
+      as dynamic)<double, double>(2.0, foo: const <int>[3, 4]));
 
   a.test8();
+
+  a.test9<num, double>(4.2, foo: 3);
+  Expect.throwsTypeError(() => (a.test9 as dynamic)<int, double>(3, foo: 3));
+  Expect.throwsTypeError(
+      () => (a.test9 as dynamic)<double, double>(3, foo: 3.2));
+  // Added to check that uses of positions from the ArgumentsDescriptor in the
+  // VM properly offsets named argument positions if there are also type
+  // arguments. allTogetherNow doesn't work for this because the runtime type of
+  // the positional argument cannot be the same as the type of the named
+  // argument without the positional argument failing to match its own type,
+  // and positional argument types are usually checked first.
+  Expect.throwsTypeError(
+      () => (a.test9 as dynamic)<int, double>(4.2, foo: 3.2));
 }

tests/language_2/nosuchmethod_forwarding/nosuchmethod_forwarding_arguments_test.dart

@@ -5,6 +5,9 @@
 // Testing that `noSuchMethod` forwarding properly handles optional, named and
 // type parameters, and result type checking.
 
+// VMOptions=--lazy-dispatchers
+// VMOptions=--no-lazy-dispatchers
+
 import 'package:expect/expect.dart';
 
 class A {
@@ -47,6 +50,16 @@ class A {
     } else if (invoke.memberName == #test8) {
       Expect.equals(1, invoke.positionalArguments.length);
       Expect.equals(null, invoke.positionalArguments[0]);
+    } else if (invoke.memberName == #test9) {
+      Expect.equals(invoke.typeArguments.length, 2);
+      Expect.equals(invoke.typeArguments[0].toString(), "num");
+      Expect.equals(invoke.typeArguments[1].toString(), "double");
+
+      Expect.equals(1, invoke.positionalArguments.length);
+      Expect.equals(4.2, invoke.positionalArguments[0]);
+
+      Expect.equals(1, invoke.namedArguments.length);
+      Expect.equals(3, invoke.namedArguments[#foo]);
     }
   }
 
@@ -62,6 +75,7 @@ class A {
   void set test7(int x);
 
   void test8([String x]);
+  void test9<T, S extends T>(S x1, {T foo});
 
   T allTogetherNow<T, S extends T>(S x1, {List<T> foo: const <Null>[]});
 }
@@ -96,6 +110,25 @@ main() {
   Expect.throwsTypeError(() => (a as dynamic).test7 = "hi");
 
   a.allTogetherNow<num, double>(2.0, foo: const <num>[3, 4]);
+  Expect.throwsTypeError(() =>
+      (a.allTogetherNow as dynamic)<int, double>(2.0, foo: const <num>[3, 4]));
+  Expect.throwsTypeError(() =>
+      (a.allTogetherNow as dynamic)<int, int>(2.0, foo: const <num>[3, 4]));
+  Expect.throwsTypeError(() => (a.allTogetherNow
+      as dynamic)<double, double>(2.0, foo: const <int>[3, 4]));
 
   a.test8();
+
+  a.test9<num, double>(4.2, foo: 3);
+  Expect.throwsTypeError(() => (a.test9 as dynamic)<int, double>(3, foo: 3));
+  Expect.throwsTypeError(
+      () => (a.test9 as dynamic)<double, double>(3, foo: 3.2));
+  // Added to check that uses of positions from the ArgumentsDescriptor in the
+  // VM properly offsets named argument positions if there are also type
+  // arguments. allTogetherNow doesn't work for this because the runtime type of
+  // the positional argument cannot be the same as the type of the named
+  // argument without the positional argument failing to match its own type,
+  // and positional argument types are usually checked first.
+  Expect.throwsTypeError(
+      () => (a.test9 as dynamic)<int, double>(4.2, foo: 3.2));
 }