[XL] Change in dart/sdk[main]: [cfe][PrimaryConstructors] Delay adding initializers

0 views
Skip to first unread message

Jens Johansen (Gerrit)

unread,
Dec 18, 2025, 5:09:26 AM (3 days ago) Dec 18
to Johnni Winther, Commit Queue, dart-fe-te...@google.com, rev...@dartlang.org
Attention needed from Johnni Winther

Jens Johansen voted and added 3 comments

Votes added by Jens Johansen

Code-Review+1

3 comments

Commit Message
Line 9, Patchset 6 (Latest):This simplifies how constructor initializers are added to the SourceConstructorBuilder. Previously the initializers were added from both within the body builder, prior to inference, and in the resolver, post inference; and initializer consistency was during for each addition.
Jens Johansen . unresolved

?

Line 11, Patchset 6 (Latest):Now all initializers are collected during body building, inferred, and them checked for consistency before being added to the SourceConstructorBuilder.
Jens Johansen . unresolved

then

Line 13, Patchset 6 (Latest):This change paves the way for handing primary constructor body declarations which requires a more high-level handling due to it splitting the definition of primary constructors in the two distinct syntactical units which needs to be handled partially, one at a time, during body building.
Jens Johansen . unresolved

handling?

Open in Gerrit

Related details

Attention is currently required from:
  • Johnni Winther
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement 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: I56d376022dfe77340a4a5beee074610d1e3119c6
Gerrit-Change-Number: 468780
Gerrit-PatchSet: 6
Gerrit-Owner: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Jens Johansen <je...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Thu, 18 Dec 2025 10:09:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Johnni Winther (Gerrit)

unread,
Dec 18, 2025, 5:49:49 AM (3 days ago) Dec 18
to Jens Johansen, Commit Queue, dart-fe-te...@google.com, rev...@dartlang.org

Johnni Winther added 3 comments

Commit Message
Line 9, Patchset 6:This simplifies how constructor initializers are added to the SourceConstructorBuilder. Previously the initializers were added from both within the body builder, prior to inference, and in the resolver, post inference; and initializer consistency was during for each addition.
Jens Johansen . resolved

?

Johnni Winther

Done

Line 11, Patchset 6:Now all initializers are collected during body building, inferred, and them checked for consistency before being added to the SourceConstructorBuilder.
Jens Johansen . resolved

then

Johnni Winther

Done

Line 13, Patchset 6:This change paves the way for handing primary constructor body declarations which requires a more high-level handling due to it splitting the definition of primary constructors in the two distinct syntactical units which needs to be handled partially, one at a time, during body building.
Jens Johansen . resolved

handling?

Johnni Winther

Done

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement 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: I56d376022dfe77340a4a5beee074610d1e3119c6
Gerrit-Change-Number: 468780
Gerrit-PatchSet: 7
Gerrit-Owner: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Jens Johansen <je...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Thu, 18 Dec 2025 10:49:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Jens Johansen <je...@google.com>
satisfied_requirement
open
diffy

Johnni Winther (Gerrit)

unread,
Dec 18, 2025, 5:49:54 AM (3 days ago) Dec 18
to Jens Johansen, Commit Queue, dart-fe-te...@google.com, rev...@dartlang.org

Johnni Winther voted Commit-Queue+2

Commit-Queue+2
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement 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: I56d376022dfe77340a4a5beee074610d1e3119c6
Gerrit-Change-Number: 468780
Gerrit-PatchSet: 7
Gerrit-Owner: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Jens Johansen <je...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Thu, 18 Dec 2025 10:49:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Commit Queue (Gerrit)

unread,
Dec 18, 2025, 5:50:09 AM (3 days ago) Dec 18
to Johnni Winther, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Commit Queue submitted the change

Unreviewed changes

6 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Change information

Commit message:
[cfe][PrimaryConstructors] Delay adding initializers

This simplifies how constructor initializers are added to the SourceConstructorBuilder. Previously the initializers were added from both within the body builder, prior to inference, and in the resolver, post inference; and initializer consistency was checked for each addition.

Now all initializers are collected during body building, inferred, and then checked for consistency before being added to the SourceConstructorBuilder.

This change paves the way for handling primary constructor body declarations which requires a more high-level handling due to it splitting the definition of primary constructors in the two distinct syntactical units which needs to be handled partially, one at a time, during body building.
Change-Id: I56d376022dfe77340a4a5beee074610d1e3119c6
Reviewed-by: Jens Johansen <je...@google.com>
Commit-Queue: Johnni Winther <johnni...@google.com>
Files:
  • M pkg/front_end/lib/src/fragment/constructor/body_builder_context.dart
  • M pkg/front_end/lib/src/fragment/constructor/declaration.dart
  • M pkg/front_end/lib/src/fragment/constructor/encoding.dart
  • M pkg/front_end/lib/src/fragment/field/encoding.dart
  • M pkg/front_end/lib/src/kernel/body_builder.dart
  • M pkg/front_end/lib/src/kernel/body_builder_context.dart
  • M pkg/front_end/lib/src/kernel/body_builder_helpers.dart
  • M pkg/front_end/lib/src/kernel/resolver.dart
  • M pkg/front_end/lib/src/kernel/resolver_helpers.dart
  • M pkg/front_end/lib/src/source/source_constructor_builder.dart
  • M pkg/front_end/lib/src/type_inference/inference_visitor.dart
  • M pkg/front_end/testcases/coverage/external_test.dart.strong.expect
  • M pkg/front_end/testcases/coverage/external_test.dart.strong.modular.expect
  • M pkg/front_end/testcases/coverage/external_test.dart.strong.transformed.expect
  • M pkg/front_end/testcases/coverage/static2_test.dart.strong.expect
  • M pkg/front_end/testcases/coverage/static2_test.dart.strong.modular.expect
  • M pkg/front_end/testcases/coverage/static2_test.dart.strong.outline.expect
  • A pkg/front_end/testcases/coverage/static2_test.dart.strong.transformed.expect
  • M pkg/front_end/testcases/enhanced_enums/external_constructor.dart.strong.expect
  • M pkg/front_end/testcases/enhanced_enums/external_constructor.dart.strong.modular.expect
  • M pkg/front_end/testcases/enhanced_enums/external_constructor.dart.strong.outline.expect
  • M pkg/front_end/testcases/enhanced_enums/external_constructor.dart.strong.transformed.expect
  • M pkg/front_end/testcases/extension_types/issue53212.dart.strong.expect
  • M pkg/front_end/testcases/extension_types/issue53212.dart.strong.modular.expect
  • M pkg/front_end/testcases/extension_types/issue53212.dart.strong.outline.expect
  • M pkg/front_end/testcases/extension_types/issue53212.dart.strong.transformed.expect
  • M pkg/front_end/testcases/extension_types/issue53625.dart.strong.expect
  • M pkg/front_end/testcases/extension_types/issue53625.dart.strong.modular.expect
  • M pkg/front_end/testcases/extension_types/issue53625.dart.strong.outline.expect
  • M pkg/front_end/testcases/extension_types/issue53625.dart.strong.transformed.expect
  • A pkg/front_end/testcases/extension_types/void.dart
  • A pkg/front_end/testcases/extension_types/void.dart.strong.expect
  • A pkg/front_end/testcases/extension_types/void.dart.strong.modular.expect
  • A pkg/front_end/testcases/extension_types/void.dart.strong.outline.expect
  • A pkg/front_end/testcases/extension_types/void.dart.strong.transformed.expect
  • A pkg/front_end/testcases/extension_types/void.dart.textual_outline.expect
  • A pkg/front_end/testcases/extension_types/void.dart.textual_outline_modelled.expect
  • M pkg/front_end/testcases/general/many_errors.dart.strong.expect
  • M pkg/front_end/testcases/general/many_errors.dart.strong.modular.expect
  • M pkg/front_end/testcases/general/many_errors.dart.strong.transformed.expect
  • A pkg/front_end/testcases/general/void_field.dart
  • A pkg/front_end/testcases/general/void_field.dart.strong.expect
  • A pkg/front_end/testcases/general/void_field.dart.strong.modular.expect
  • A pkg/front_end/testcases/general/void_field.dart.strong.outline.expect
  • A pkg/front_end/testcases/general/void_field.dart.strong.transformed.expect
  • A pkg/front_end/testcases/general/void_field.dart.textual_outline.expect
  • A pkg/front_end/testcases/general/void_field.dart.textual_outline_modelled.expect
  • M pkg/front_end/testcases/modular.status
  • M pkg/front_end/testcases/offsets/step_through_patterns.dart.strong.expect
  • M pkg/front_end/testcases/offsets/step_through_patterns.dart.strong.modular.expect
  • M pkg/front_end/testcases/offsets/step_through_patterns.dart.strong.transformed.expect
  • M pkg/front_end/testcases/offsets/switch_encoding.dart.strong.expect
  • M pkg/front_end/testcases/offsets/switch_encoding.dart.strong.modular.expect
  • M pkg/front_end/testcases/offsets/switch_encoding.dart.strong.outline.expect
  • M pkg/front_end/testcases/offsets/switch_encoding.dart.strong.transformed.expect
  • M pkg/front_end/testcases/outline.status
  • M pkg/front_end/testcases/strong.status
  • M pkg/front_end/testcases/super_parameters/issue48142.dart.strong.outline.expect
  • M pkg/front_end/testcases/super_parameters/super_parameters_with_types_and_default_values.dart.strong.outline.expect
  • M pkg/front_end/testcases/wildcard_variables/initializing_formals.dart.strong.outline.expect
Change size: XL
Delta: 60 files changed, 649 insertions(+), 494 deletions(-)
Branch: refs/heads/main
Submit Requirements:
  • requirement satisfiedCode-Review: +1 by Jens Johansen
Open in Gerrit
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: merged
Gerrit-Project: sdk
Gerrit-Branch: main
Gerrit-Change-Id: I56d376022dfe77340a4a5beee074610d1e3119c6
Gerrit-Change-Number: 468780
Gerrit-PatchSet: 8
Gerrit-Owner: Johnni Winther <johnni...@google.com>
open
diffy
satisfied_requirement

Nicholas Shahan (Gerrit)

unread,
Dec 18, 2025, 9:17:01 PM (3 days ago) Dec 18
to Johnni Winther, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org
Attention needed from Johnni Winther

Nicholas Shahan added 1 comment

Patchset-level comments
File-level comment, Patchset 8 (Latest):
Nicholas Shahan . unresolved

@johnni...@google.com
This change appears to have broken the source mapped locations of constructor invocations in a stepping test that uses dot shorthands to invoke the constructors.

I bisected and this is indeed the culprit CL but I haven't investigated enough to understand exactly what changed. I'm hoping you might be able to spot something with the context you have.

The failing test run is here:
https://github.com/dart-lang/webdev/actions/runs/20339474962/job/58434543540

The logs are verbose but the TLDR is that when running this code, setting a breakpoint and stepping through a few of the expected locations are wrong. Instead of pointing to the constructor the source location seems to point to one invocation of the constructor instead.

Failing Test Files:

App under test:

Failure message:
```
 Expected: [
          'main.dart:20:3',
          'main.dart:11:15',
          'main.dart:9:10',
          'main.dart:9:16',
          'main.dart:11:20',
          'main.dart:22:3',
          'main.dart:12:25',
          'main.dart:9:10',
          'main.dart:9:16',
          'main.dart:12:27',
          'main.dart:24:3',
          'main.dart:13:22',
          'main.dart:9:10',
          'main.dart:9:16',
          'main.dart:13:24',
          'main.dart:25:3'
        ]
Actual: [
          'main.dart:20:3',
          'main.dart:11:15',
          'main.dart:13:26',
          'main.dart:9:16',
          'main.dart:11:20',
          'main.dart:22:3',
          'main.dart:12:25',
          'main.dart:13:26',
          'main.dart:9:16',
          'main.dart:12:27',
          'main.dart:24:3',
          'main.dart:13:22',
          'main.dart:13:26',
          'main.dart:9:16',
          'main.dart:13:24',
          'main.dart:25:3'
        ]
  Which: at location [2] is 'main.dart:13:26' instead of 'main.dart:9:10'
```
Open in Gerrit

Related details

Attention is currently required from:
  • Johnni Winther
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement 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: I56d376022dfe77340a4a5beee074610d1e3119c6
Gerrit-Change-Number: 468780
Gerrit-PatchSet: 8
Gerrit-Owner: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Jens Johansen <je...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-CC: Nicholas Shahan <nsh...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Fri, 19 Dec 2025 02:16:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
open
diffy

Johnni Winther (Gerrit)

unread,
Dec 19, 2025, 8:05:28 AM (2 days ago) Dec 19
to Commit Queue, Nicholas Shahan, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Johnni Winther added 1 comment

Patchset-level comments
Johnni Winther

This CL adds file offsets to `FieldInitializer` nodes generated for initializing formals. These were needed to provide the correct location for reported errors, but likely trigger the extra step you see. This is a case of the CFE wanting file offsets but DDC _not_ wanting them but we have no other signal than `TreeNode.fileOffset`.

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
  • requirement 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: I56d376022dfe77340a4a5beee074610d1e3119c6
Gerrit-Change-Number: 468780
Gerrit-PatchSet: 8
Gerrit-Owner: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Jens Johansen <je...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-CC: Nicholas Shahan <nsh...@google.com>
Gerrit-Comment-Date: Fri, 19 Dec 2025 13:05:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nicholas Shahan <nsh...@google.com>
satisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages