[L] Change in dart/sdk[main]: [record_use] Small fixes

0 views
Skip to first unread message

Moritz Sümmermann (Gerrit)

unread,
Sep 23, 2025, 7:58:17 AM (6 days ago) Sep 23
to Johnni Winther, Commit Queue, rev...@dartlang.org
Attention needed from Johnni Winther

Moritz Sümmermann added 1 comment

Patchset-level comments
File-level comment, Patchset 5 (Latest):
Moritz Sümmermann . resolved

@johnni...@google.com I don't quite understand how this `GetVariable` gets to occur - are there more such edge cases we should think of?

Open in Gerrit

Related details

Attention is currently required from:
  • Johnni Winther
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedCommit-Message-Has-TEST
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: sdk
Gerrit-Branch: main
Gerrit-Change-Id: Idd134424e714f2bfe0660a052277ae7d4f75cf95
Gerrit-Change-Number: 450400
Gerrit-PatchSet: 5
Gerrit-Owner: Moritz Sümmermann <mo...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Moritz Sümmermann <mo...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Tue, 23 Sep 2025 11:58:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
unsatisfied_requirement
open
diffy

Johnni Winther (Gerrit)

unread,
Sep 23, 2025, 9:19:38 AM (6 days ago) Sep 23
to Moritz Sümmermann, Daco Harkes, Commit Queue, rev...@dartlang.org
Attention needed from Daco Harkes and Moritz Sümmermann

Johnni Winther added 1 comment

Patchset-level comments
Moritz Sümmermann . resolved

@johnni...@google.com I don't quite understand how this `GetVariable` gets to occur - are there more such edge cases we should think of?

Johnni Winther

What source code triggers this?

Open in Gerrit

Related details

Attention is currently required from:
  • Daco Harkes
  • Moritz Sümmermann
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedCommit-Message-Has-TEST
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: sdk
Gerrit-Branch: main
Gerrit-Change-Id: Idd134424e714f2bfe0660a052277ae7d4f75cf95
Gerrit-Change-Number: 450400
Gerrit-PatchSet: 7
Gerrit-Owner: Moritz Sümmermann <mo...@google.com>
Gerrit-Reviewer: Daco Harkes <dacoh...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Moritz Sümmermann <mo...@google.com>
Gerrit-Attention: Moritz Sümmermann <mo...@google.com>
Gerrit-Attention: Daco Harkes <dacoh...@google.com>
Gerrit-Comment-Date: Tue, 23 Sep 2025 13:19:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Moritz Sümmermann <mo...@google.com>
unsatisfied_requirement
open
diffy

Moritz Sümmermann (Gerrit)

unread,
Sep 23, 2025, 9:21:41 AM (6 days ago) Sep 23
to Daco Harkes, Johnni Winther, Commit Queue, rev...@dartlang.org
Attention needed from Daco Harkes and Johnni Winther

Moritz Sümmermann added 1 comment

Patchset-level comments
Moritz Sümmermann . resolved

@johnni...@google.com I don't quite understand how this `GetVariable` gets to occur - are there more such edge cases we should think of?

Johnni Winther

What source code triggers this?

Moritz Sümmermann

`pkg/vm/testcases/transformations/record_use/named_with_function_arg.dart`, see `pkg/vm/testcases/transformations/record_use/named_with_function_arg.dart.aot.expect`. This test case fails without the change made in `pkg/vm/lib/transformations/record_use/record_call.dart`

Open in Gerrit

Related details

Attention is currently required from:
  • Daco Harkes
  • Johnni Winther
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedCommit-Message-Has-TEST
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: sdk
Gerrit-Branch: main
Gerrit-Change-Id: Idd134424e714f2bfe0660a052277ae7d4f75cf95
Gerrit-Change-Number: 450400
Gerrit-PatchSet: 7
Gerrit-Owner: Moritz Sümmermann <mo...@google.com>
Gerrit-Reviewer: Daco Harkes <dacoh...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Moritz Sümmermann <mo...@google.com>
Gerrit-Attention: Daco Harkes <dacoh...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Tue, 23 Sep 2025 13:21:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Moritz Sümmermann <mo...@google.com>
Comment-In-Reply-To: Johnni Winther <johnni...@google.com>
unsatisfied_requirement
open
diffy

Johnni Winther (Gerrit)

unread,
Sep 23, 2025, 9:27:08 AM (6 days ago) Sep 23
to Moritz Sümmermann, Daco Harkes, Commit Queue, rev...@dartlang.org
Attention needed from Daco Harkes and Johnni Winther

Johnni Winther added 1 comment

Patchset-level comments
Moritz Sümmermann . resolved

@johnni...@google.com I don't quite understand how this `GetVariable` gets to occur - are there more such edge cases we should think of?

Johnni Winther

What source code triggers this?

Johnni Winther

This is because of the `named-arguments-anywhere` feature. Because the calling convention puts positional arguments before named arguments we need to hoist the named arguments to preserve the evaluation order. This is done by wrapping the call in a `Let` expression that performs the evaluation of the named argument before the evaluation of the position argument.

Gerrit-Comment-Date: Tue, 23 Sep 2025 13:27:02 +0000
unsatisfied_requirement
open
diffy

Daco Harkes (Gerrit)

unread,
4:49 AM (5 hours ago) 4:49 AM
to Moritz Sümmermann, Johnni Winther, Commit Queue, rev...@dartlang.org
Attention needed from Johnni Winther and Moritz Sümmermann

Daco Harkes added 3 comments

Commit Message
Line 13, Patchset 7 (Latest):pkg/vm/testcases/transformations/record_use/named_with_function_arg.dart
Daco Harkes . unresolved

TEST= ....

File pkg/record_use/test/usage_test.dart
Line 84, Patchset 7 (Latest): 'lib_SHA1',
Daco Harkes . unresolved

Why is the order changing?

File pkg/vm/testcases/transformations/record_use/named_with_function_arg.dart
Line 1, Patchset 7 (Latest):import 'package:meta/meta.dart' show RecordUse;
Daco Harkes . unresolved

Should this have a copyright header?

Open in Gerrit

Related details

Attention is currently required from:
  • Johnni Winther
  • Moritz Sümmermann
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedCommit-Message-Has-TEST
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: sdk
Gerrit-Branch: main
Gerrit-Change-Id: Idd134424e714f2bfe0660a052277ae7d4f75cf95
Gerrit-Change-Number: 450400
Gerrit-PatchSet: 7
Gerrit-Owner: Moritz Sümmermann <mo...@google.com>
Gerrit-Reviewer: Daco Harkes <dacoh...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Moritz Sümmermann <mo...@google.com>
Gerrit-Attention: Moritz Sümmermann <mo...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Mon, 29 Sep 2025 08:49:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
unsatisfied_requirement
open
diffy

Moritz Sümmermann (Gerrit)

unread,
7:18 AM (2 hours ago) 7:18 AM
to Alexander Markov, Daco Harkes, Johnni Winther, Commit Queue, rev...@dartlang.org
Attention needed from Daco Harkes and Johnni Winther

Moritz Sümmermann added 3 comments

Commit Message
Line 13, Patchset 7:pkg/vm/testcases/transformations/record_use/named_with_function_arg.dart
Daco Harkes . resolved

TEST= ....

Moritz Sümmermann

I think both is fine as per the docs. "Dart VM changes must include a 'Tested: <description of how this change was tested>' footer. See also: https://github.com/dart-lang/sdk/wiki/Gerrit-Submit-Requirements#commit-message-has-test"

File pkg/record_use/test/usage_test.dart
Line 84, Patchset 7: 'lib_SHA1',
Daco Harkes . resolved

Why is the order changing?

Moritz Sümmermann

As before, all the arguments were stored as positional.

File pkg/vm/testcases/transformations/record_use/named_with_function_arg.dart
Line 1, Patchset 7:import 'package:meta/meta.dart' show RecordUse;
Daco Harkes . resolved

Should this have a copyright header?

Moritz Sümmermann

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Daco Harkes
  • Johnni Winther
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedCommit-Message-Has-TEST
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: sdk
Gerrit-Branch: main
Gerrit-Change-Id: Idd134424e714f2bfe0660a052277ae7d4f75cf95
Gerrit-Change-Number: 450400
Gerrit-PatchSet: 9
Gerrit-Owner: Moritz Sümmermann <mo...@google.com>
Gerrit-Reviewer: Daco Harkes <dacoh...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Moritz Sümmermann <mo...@google.com>
Gerrit-CC: Alexander Markov <alexm...@google.com>
Gerrit-Attention: Daco Harkes <dacoh...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Mon, 29 Sep 2025 11:18:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Daco Harkes <dacoh...@google.com>
unsatisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages