Chloe Stefantsova (Gerrit)

unread,

Jan 14, 2026, 6:34:49 AM (8 days ago) Jan 14

to Chloe Stefantsova, Johnni Winther, Alexander Markov, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Alexander Markov and Johnni Winther

Chloe Stefantsova voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Alexander Markov
Johnni Winther

Submit Requirements:

Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Johnni Winther (Gerrit)

unread,

Jan 14, 2026, 8:00:59 AM (8 days ago) Jan 14

to Chloe Stefantsova, Alexander Markov, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Alexander Markov and Chloe Stefantsova

Johnni Winther voted and added 5 comments

Votes added by Johnni Winther

Code-Review

+1

5 comments

File pkg/kernel/lib/src/ast/functions.dart

Line 45, Patchset 8 (Latest): internalPositionalParameters.cast();

Johnni Winther . unresolved

I'm a bit worried about the runtime overhead of this.

Line 52, Patchset 8 (Latest): internalNamedParameters.cast();

Johnni Winther . unresolved

Ditto.

File pkg/kernel/lib/src/ast/statements.dart

Line 1195, Patchset 8 (Latest): PositionalParameter? internalException;

Johnni Winther . unresolved

Shouldn't we have a `CatchParameter` for this and `stackTrace` ?

File pkg/kernel/lib/src/ast/variables.dart

Line 617, Patchset 8 (Latest):

    required Expression? defaultValue,
    bool isCovariantByDeclaration = false,
    bool isRequired = false,
    bool isInitializingFormal = false,
    bool isSuperInitializingFormal = false,
    bool isFinal = false,
    bool hasDeclaredDefaultType = false,
    bool isLowered = false,
    bool isSynthesized = false,
    bool isWildcard = false,
  }) : super(

Johnni Winther . unresolved

Couldn't these be super parameters?

Line 739, Patchset 8 (Latest):

      required Expression? defaultValue,
      bool isCovariantByDeclaration = false,
      bool isRequired = false,
      bool isInitializingFormal = false,
      bool isSuperInitializingFormal = false,
      bool isFinal = false,
      bool hasDeclaredDefaultType = false,
      bool isLowered = false,
      bool isSynthesized = false,
      bool isWildcard = false})
      : super(

Johnni Winther . unresolved

Ditto

Open in Gerrit

Related details

Attention is currently required from:

Alexander Markov
Chloe Stefantsova

Submit Requirements:

Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Alexander Markov (Gerrit)

unread,

Jan 14, 2026, 9:16:07 AM (8 days ago) Jan 14

to Chloe Stefantsova, Alexander Markov, Johnni Winther, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Chloe Stefantsova

Alexander Markov voted and added 1 comment

Votes added by Alexander Markov

Code-Review

+1

1 comment

File pkg/kernel/lib/src/ast/functions.dart

Line 45, Patchset 8 (Latest): internalPositionalParameters.cast();

Johnni Winther . unresolved

I'm a bit worried about the runtime overhead of this.

Alexander Markov

Maybe `PositionalParameter` can just implement `VariableDeclaration` interface (for now), so this cast won't be needed.

Open in Gerrit

Related details

Attention is currently required from:

Chloe Stefantsova

Submit Requirements:

Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chloe Stefantsova (Gerrit)

unread,

Jan 16, 2026, 4:11:12 AM (6 days ago) Jan 16

to Chloe Stefantsova, Alexander Markov, Johnni Winther, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Alexander Markov and Johnni Winther

Chloe Stefantsova voted and added 6 comments

Votes added by Chloe Stefantsova

Commit-Queue

+1

6 comments

Patchset-level comments

File-level comment, Patchset 9 (Latest):

Chloe Stefantsova . resolved

Thank you for taking a look and for the suggestions, Johnni and Alex! I've updated the CL accordingly, and it seems to remove the most of the overhead. PTAL.

File pkg/kernel/lib/src/ast/functions.dart

Line 45, Patchset 8: internalPositionalParameters.cast();

Johnni Winther . resolved

I'm a bit worried about the runtime overhead of this.

Alexander Markov

Maybe `PositionalParameter` can just implement `VariableDeclaration` interface (for now), so this cast won't be needed.

Chloe Stefantsova

Thanks for the suggestion, Alex! Making `FunctionParameter` implement `VariableDeclaration` solved the issue, even though it goes against the design. It works as a workaround until we commit to the new model and can reverse the implementation relation.

Line 52, Patchset 8: internalNamedParameters.cast();

Johnni Winther . resolved

Ditto.

Chloe Stefantsova

Done

File pkg/kernel/lib/src/ast/statements.dart

Line 1195, Patchset 8: PositionalParameter? internalException;

Johnni Winther . resolved

Shouldn't we have a `CatchParameter` for this and `stackTrace` ?

Chloe Stefantsova

Now that `FunctionParameter` implements `VariableDeclaration`, this change is not needed. But I like the idea of a `CatchParameter`.

File pkg/kernel/lib/src/ast/variables.dart

Line 617, Patchset 8: required Expression? defaultValue,


    bool isCovariantByDeclaration = false,
    bool isRequired = false,
    bool isInitializingFormal = false,
    bool isSuperInitializingFormal = false,
    bool isFinal = false,
    bool hasDeclaredDefaultType = false,
    bool isLowered = false,
    bool isSynthesized = false,
    bool isWildcard = false,
  }) : super(

Johnni Winther . resolved

Couldn't these be super parameters?

Chloe Stefantsova

Done

Line 739, Patchset 8: required Expression? defaultValue,


      bool isCovariantByDeclaration = false,
      bool isRequired = false,
      bool isInitializingFormal = false,
      bool isSuperInitializingFormal = false,
      bool isFinal = false,
      bool hasDeclaredDefaultType = false,
      bool isLowered = false,
      bool isSynthesized = false,
      bool isWildcard = false})
      : super(

Johnni Winther . resolved

Ditto

Chloe Stefantsova

Done

Open in Gerrit

Related details

Attention is currently required from:

Alexander Markov
Johnni Winther

Submit Requirements:

Code-Owners

Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Johnni Winther (Gerrit)

unread,

Jan 16, 2026, 6:06:53 AM (6 days ago) Jan 16

to Chloe Stefantsova, Alexander Markov, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Alexander Markov and Chloe Stefantsova

Johnni Winther voted and added 3 comments

Votes added by Johnni Winther

Code-Review

+1

3 comments

File pkg/front_end/lib/src/kernel/cfe_verifier.dart

Line 48, Patchset 9 (Parent):

  // Coverage-ignore(suite): Not run.
  void reportError(

Johnni Winther . unresolved

Looks like you have a verifier error.

File pkg/front_end/test/spell_checking_list_common.txt

Line 1590, Patchset 9 (Latest):infering

Johnni Winther . unresolved

This should be "inferring".

File pkg/kernel/lib/src/ast/functions.dart

Line 127, Patchset 9 (Latest): static DartType _getTypeOfVariable(ExpressionVariable node) => node.type;

Johnni Winther . unresolved

Should this change be delayed until we more to the new model?

Open in Gerrit

Related details

Attention is currently required from:

Alexander Markov
Chloe Stefantsova

Submit Requirements:

Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Chloe Stefantsova (Gerrit)

unread,

Jan 19, 2026, 6:09:10 AM (3 days ago) Jan 19

to Chloe Stefantsova, Johnni Winther, Alexander Markov, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Alexander Markov and Johnni Winther

Chloe Stefantsova voted and added 4 comments

Votes added by Chloe Stefantsova

Commit-Queue

+1

4 comments

Patchset-level comments

File-level comment, Patchset 10 (Latest):

Chloe Stefantsova . resolved

Thanks for another review, Johnni. I've applied your suggestions.

Johnni, Alex, PTAL.

File pkg/front_end/lib/src/kernel/cfe_verifier.dart

Line 48, Patchset 9 (Parent):

  // Coverage-ignore(suite): Not run.
  void reportError(

Johnni Winther . resolved

Looks like you have a verifier error.

Chloe Stefantsova

Thanks for spotting this! Fixed.

File pkg/front_end/test/spell_checking_list_common.txt

Line 1590, Patchset 9:infering

Johnni Winther . resolved

This should be "inferring".

Chloe Stefantsova

Done

File pkg/kernel/lib/src/ast/functions.dart

Line 127, Patchset 9: static DartType _getTypeOfVariable(ExpressionVariable node) => node.type;

Johnni Winther . resolved

Should this change be delayed until we more to the new model?

Chloe Stefantsova

I've reverted the change in this line and other similar places elsewhere in the CL, so we don't get the increased polymorphism until we need it.

Open in Gerrit

Related details

Attention is currently required from:

Alexander Markov
Johnni Winther

Submit Requirements:

Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Johnni Winther (Gerrit)

unread,

Jan 21, 2026, 7:30:27 AM (17 hours ago) Jan 21

to Chloe Stefantsova, Alexander Markov, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Alexander Markov and Chloe Stefantsova

Johnni Winther voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Alexander Markov
Chloe Stefantsova

Submit Requirements:

Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Alexander Markov (Gerrit)

unread,

Jan 21, 2026, 8:46:58 AM (16 hours ago) Jan 21

to Chloe Stefantsova, Alexander Markov, Johnni Winther, Commit Queue, Jens Johansen, dart-fe-te...@google.com, rev...@dartlang.org

Attention needed from Chloe Stefantsova

Alexander Markov voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Chloe Stefantsova

Submit Requirements:

Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[XL] Change in dart/sdk[main]: [cfe] Add function parameters to closure contexts

Chloe Stefantsova (Gerrit)

Chloe Stefantsova voted Commit-Queue+1

Related details

Johnni Winther (Gerrit)

Johnni Winther voted and added 5 comments

Votes added by Johnni Winther

5 comments

Related details

Alexander Markov (Gerrit)

Alexander Markov voted and added 1 comment

Votes added by Alexander Markov

1 comment

Related details

Chloe Stefantsova (Gerrit)

Chloe Stefantsova voted and added 6 comments

Votes added by Chloe Stefantsova

6 comments

Related details

Johnni Winther (Gerrit)

Johnni Winther voted and added 3 comments

Votes added by Johnni Winther

3 comments

Related details

Chloe Stefantsova (Gerrit)

Chloe Stefantsova voted and added 4 comments

Votes added by Chloe Stefantsova

4 comments

Related details

Johnni Winther (Gerrit)

Johnni Winther voted Code-Review+1

Related details

Alexander Markov (Gerrit)

Alexander Markov voted Code-Review+1

Related details