[L] Change in dart/sdk[main]: DeCo. AST for PrimaryConstructorDeclaration and ClassBody.

0 views
Skip to first unread message

Brian Wilkerson (Gerrit)

unread,
Oct 10, 2025, 11:25:39 AM (11 days ago) Oct 10
to Konstantin Shcheglov, Johnni Winther, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Johnni Winther, Kallen Tu and Konstantin Shcheglov

Brian Wilkerson added 4 comments

File pkg/analyzer/lib/src/dart/ast/ast.dart
Line 2895, Patchset 3 (Latest):sealed class ClassBody implements AstNode {}
Brian Wilkerson . unresolved

I like `ClassBody`, but it would be more consistent if we also used it for extensions and mixins. I understand that only one of the two subclasses could validly be used in either of these cases, but I think the consistency (and possibly the ability to pass a `ClassBody` around no matter what kind of parent it has) would make it worthwhile.

It would also be a good way to handle recovery if someone types an extension or a mixing with an empty body. It would make it easier for the fix to find the semicolon.

Line 3380, Patchset 3 (Latest):sealed class ClassNamePart implements AstNode {
Brian Wilkerson . unresolved

I don't like the name `ClassNamePart` because it's more than the name of the class. In it's most complete form it represents the name, type parameters, and primary constructor.

What would you think of calling this a `ClassHeader`? I'm not completely thrilled with the name because I would tend to include the `extends`, `with`, and `implements` clauses in the header, but I don't have a better name at this point and I think it might be better than `ClassNamePart`.

Also, as with `ClassBody`, it would be more consistent if we used this class for all of the top-level declarations that can have both a name and type parameters, even though some of those declarations can't have a primary constructor and hence would never return one of the two subclasses.

Line 12530, Patchset 3 (Latest):final class IdentifierWithTypeParametersImpl extends ClassNamePartImpl
Brian Wilkerson . resolved

Would it be worthwhile to have a `ClassHeaderWithIdentifierImpl` subclass for cases where there are no type parameters (with a getter that just returns `null`)? Just a thought, not something I care about either way.

Line 19818, Patchset 3 (Latest):abstract final class PrimaryConstructorName implements AstNode {
Brian Wilkerson . unresolved

The classes `PrimaryConstructorName` and `PrimaryConstructorNameImpl` don't appear to be used. I'm not sure whether I'm just missing something or whether you forgot to remove them.

Open in Gerrit

Related details

Attention is currently required from:
  • Johnni Winther
  • Kallen Tu
  • Konstantin Shcheglov
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
Gerrit-Change-Number: 454322
Gerrit-PatchSet: 3
Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Kallen Tu <kall...@google.com>
Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Kallen Tu <kall...@google.com>
Gerrit-Attention: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Fri, 10 Oct 2025 15:25:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
unsatisfied_requirement
open
diffy

Konstantin Shcheglov (Gerrit)

unread,
Oct 10, 2025, 3:23:42 PM (11 days ago) Oct 10
to Johnni Winther, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson, Johnni Winther and Kallen Tu

Konstantin Shcheglov added 3 comments

File pkg/analyzer/lib/src/dart/ast/ast.dart
Line 2895, Patchset 3:sealed class ClassBody implements AstNode {}
Brian Wilkerson . unresolved

I like `ClassBody`, but it would be more consistent if we also used it for extensions and mixins. I understand that only one of the two subclasses could validly be used in either of these cases, but I think the consistency (and possibly the ability to pass a `ClassBody` around no matter what kind of parent it has) would make it worthwhile.

It would also be a good way to handle recovery if someone types an extension or a mixing with an empty body. It would make it easier for the fix to find the semicolon.

Konstantin Shcheglov

I agree that we can at least use `ClassBodyWithMembers` for extensions and mixins, this already will let us pass it as `ClassBody` somewhere.

I'm not sure whether going up to `ClassBody` is useful. There is a small drawback, we would have to check the type. Recovery can be tricky, but it feels wrong to tie representation to recovery. There are many ways to wring wrong class body, e.g. unbalanced `{` and `}`, or write nothing at all, and using `;` is just one of them.

We also have enums, which unfortunately does not fit at all: we have `leftBracket`, `constants`, `semicolon`, `members`, `rightBracket`. Theoretically `EnumBody` is a superset of `ClassBodyWithMembers`, adding `constants` and `semicolon`. But AFAIK we previously avoided subclasses concrete node classes. And this would also violate LSP, e.g. removing the first member is not removing everything from the end of this member to `{`, there are constants now. So, maybe make `EnumBody` a subtype of `ClassBody`?

Line 3380, Patchset 3:sealed class ClassNamePart implements AstNode {
Brian Wilkerson . unresolved

I don't like the name `ClassNamePart` because it's more than the name of the class. In it's most complete form it represents the name, type parameters, and primary constructor.

What would you think of calling this a `ClassHeader`? I'm not completely thrilled with the name because I would tend to include the `extends`, `with`, and `implements` clauses in the header, but I don't have a better name at this point and I think it might be better than `ClassNamePart`.

Also, as with `ClassBody`, it would be more consistent if we used this class for all of the top-level declarations that can have both a name and type parameters, even though some of those declarations can't have a primary constructor and hence would never return one of the two subclasses.

Konstantin Shcheglov

Yeah, unfortunately I also consider "header" to be everything from modifiers and `class` keyword to the body of the class, so including `extends`, `with`, and `implements`. So, I don't like calling it `ClassHeader`. It is a part of the declaration that contains the name (1), so `ClassNamePart`. Using the word "part" also feel lame to me, but it has benefits: short and descriptive (if you buy my explanation (1)).

Line 19818, Patchset 3:abstract final class PrimaryConstructorName implements AstNode {
Brian Wilkerson . resolved

The classes `PrimaryConstructorName` and `PrimaryConstructorNameImpl` don't appear to be used. I'm not sure whether I'm just missing something or whether you forgot to remove them.

Konstantin Shcheglov

I accidentally referenced the wrong name. Fixed.

Open in Gerrit

Related details

Attention is currently required from:
  • Brian Wilkerson
  • Johnni Winther
  • Kallen Tu
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
Gerrit-Change-Number: 454322
Gerrit-PatchSet: 5
Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Kallen Tu <kall...@google.com>
Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
Gerrit-Attention: Kallen Tu <kall...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Fri, 10 Oct 2025 19:23:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Brian Wilkerson <brianwi...@google.com>
unsatisfied_requirement
open
diffy

Brian Wilkerson (Gerrit)

unread,
Oct 10, 2025, 5:26:14 PM (11 days ago) Oct 10
to Konstantin Shcheglov, Johnni Winther, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Johnni Winther, Kallen Tu and Konstantin Shcheglov

Brian Wilkerson added 3 comments

File pkg/analyzer/lib/src/dart/ast/ast.dart
Line 1783, Patchset 5 (Latest):sealed class BlockClassBody implements ClassBody {
Brian Wilkerson . resolved

I like this name better.

Line 2895, Patchset 3:sealed class ClassBody implements AstNode {}
Brian Wilkerson . unresolved

I like `ClassBody`, but it would be more consistent if we also used it for extensions and mixins. I understand that only one of the two subclasses could validly be used in either of these cases, but I think the consistency (and possibly the ability to pass a `ClassBody` around no matter what kind of parent it has) would make it worthwhile.

It would also be a good way to handle recovery if someone types an extension or a mixing with an empty body. It would make it easier for the fix to find the semicolon.

Konstantin Shcheglov

I agree that we can at least use `ClassBodyWithMembers` for extensions and mixins, this already will let us pass it as `ClassBody` somewhere.

I'm not sure whether going up to `ClassBody` is useful. There is a small drawback, we would have to check the type. Recovery can be tricky, but it feels wrong to tie representation to recovery. There are many ways to wring wrong class body, e.g. unbalanced `{` and `}`, or write nothing at all, and using `;` is just one of them.

We also have enums, which unfortunately does not fit at all: we have `leftBracket`, `constants`, `semicolon`, `members`, `rightBracket`. Theoretically `EnumBody` is a superset of `ClassBodyWithMembers`, adding `constants` and `semicolon`. But AFAIK we previously avoided subclasses concrete node classes. And this would also violate LSP, e.g. removing the first member is not removing everything from the end of this member to `{`, there are constants now. So, maybe make `EnumBody` a subtype of `ClassBody`?

Brian Wilkerson

I'm not sure whether going up to `ClassBody` is useful.

I'm fine with using the more specific subtype where applicable.

... it feels wrong to tie representation to recovery.

It feels wrong to consider representation to be more important than the UX, and recovery is a big part of the UX. So I think that feeling is not a good design principle.

Of course, in this particular case we could still search through the token stream to find the semicolon in order to fix the problem, so this is more about the implementation cost than it is about the UX, so I'm not going to fight for making the type more general.

But AFAIK we previously avoided subclasses concrete node classes.

True.

And this would also violate LSP ...

I don't understand that point.

So, maybe make `EnumBody` a subtype of `ClassBody`?

I think that would be reasonable. The presence of enum constants makes it feel less consistent than I was originally thinking it would be, but it would probably prove to be a good choice anyway.

Line 3380, Patchset 3:sealed class ClassNamePart implements AstNode {
Brian Wilkerson . unresolved

I don't like the name `ClassNamePart` because it's more than the name of the class. In it's most complete form it represents the name, type parameters, and primary constructor.

What would you think of calling this a `ClassHeader`? I'm not completely thrilled with the name because I would tend to include the `extends`, `with`, and `implements` clauses in the header, but I don't have a better name at this point and I think it might be better than `ClassNamePart`.

Also, as with `ClassBody`, it would be more consistent if we used this class for all of the top-level declarations that can have both a name and type parameters, even though some of those declarations can't have a primary constructor and hence would never return one of the two subclasses.

Konstantin Shcheglov

Yeah, unfortunately I also consider "header" to be everything from modifiers and `class` keyword to the body of the class, so including `extends`, `with`, and `implements`. So, I don't like calling it `ClassHeader`. It is a part of the declaration that contains the name (1), so `ClassNamePart`. Using the word "part" also feel lame to me, but it has benefits: short and descriptive (if you buy my explanation (1)).

Brian Wilkerson

I agree that 'part' feels lame. Yes, this subsection of the grammar is part of the declaration, but `ClassNamePart` sounds like it's claiming to be part of the _name_, which it isn't. The grammar piece is also part of the header, so `ClassHeaderPart` would be more accurate, though only slightly better.

The problem is that there's no coherent concept behind this production other than to avoid duplication in the grammar (and by extension in the node structure). It isn't a thing, it's just a semi-random collection of nodes. But `SharedSubsetOfAClassDeclaration` would be horrid.

We could also take inspiration from a comment above ("The name of the class ... or a primary constructor.") and call it `ClassNameOrPrimaryConstructor`, since it's a thing that could be either of those.

I don't have a good solution. :-/

Open in Gerrit

Related details

Attention is currently required from:
  • Johnni Winther
  • Kallen Tu
  • Konstantin Shcheglov
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
Gerrit-Change-Number: 454322
Gerrit-PatchSet: 5
Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Kallen Tu <kall...@google.com>
Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Kallen Tu <kall...@google.com>
Gerrit-Attention: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Fri, 10 Oct 2025 21:26:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Brian Wilkerson <brianwi...@google.com>
Comment-In-Reply-To: Konstantin Shcheglov <sche...@google.com>
unsatisfied_requirement
open
diffy

Konstantin Shcheglov (Gerrit)

unread,
Oct 10, 2025, 6:09:34 PM (11 days ago) Oct 10
to Johnni Winther, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson, Johnni Winther and Kallen Tu

Konstantin Shcheglov added 2 comments

File pkg/analyzer/lib/src/dart/ast/ast.dart
Line 2895, Patchset 3:sealed class ClassBody implements AstNode {}
Brian Wilkerson . unresolved

I like `ClassBody`, but it would be more consistent if we also used it for extensions and mixins. I understand that only one of the two subclasses could validly be used in either of these cases, but I think the consistency (and possibly the ability to pass a `ClassBody` around no matter what kind of parent it has) would make it worthwhile.

It would also be a good way to handle recovery if someone types an extension or a mixing with an empty body. It would make it easier for the fix to find the semicolon.

Konstantin Shcheglov

I agree that we can at least use `ClassBodyWithMembers` for extensions and mixins, this already will let us pass it as `ClassBody` somewhere.

I'm not sure whether going up to `ClassBody` is useful. There is a small drawback, we would have to check the type. Recovery can be tricky, but it feels wrong to tie representation to recovery. There are many ways to wring wrong class body, e.g. unbalanced `{` and `}`, or write nothing at all, and using `;` is just one of them.

We also have enums, which unfortunately does not fit at all: we have `leftBracket`, `constants`, `semicolon`, `members`, `rightBracket`. Theoretically `EnumBody` is a superset of `ClassBodyWithMembers`, adding `constants` and `semicolon`. But AFAIK we previously avoided subclasses concrete node classes. And this would also violate LSP, e.g. removing the first member is not removing everything from the end of this member to `{`, there are constants now. So, maybe make `EnumBody` a subtype of `ClassBody`?

Brian Wilkerson

I'm not sure whether going up to `ClassBody` is useful.

I'm fine with using the more specific subtype where applicable.

... it feels wrong to tie representation to recovery.

It feels wrong to consider representation to be more important than the UX, and recovery is a big part of the UX. So I think that feeling is not a good design principle.

Of course, in this particular case we could still search through the token stream to find the semicolon in order to fix the problem, so this is more about the implementation cost than it is about the UX, so I'm not going to fight for making the type more general.

But AFAIK we previously avoided subclasses concrete node classes.

True.

And this would also violate LSP ...

I don't understand that point.

So, maybe make `EnumBody` a subtype of `ClassBody`?

I think that would be reasonable. The presence of enum constants makes it feel less consistent than I was originally thinking it would be, but it would probably prove to be a good choice anyway.

Konstantin Shcheglov

I'm not sure whether going up to `ClassBody` is useful.

I'm fine with using the more specific subtype where applicable.

... it feels wrong to tie representation to recovery.

It feels wrong to consider representation to be more important than the UX, and recovery is a big part of the UX. So I think that feeling is not a good design principle.

This is fair, I agree that UX is the starting point.


> Of course, in this particular case we could still search through the token stream to find the semicolon in order to fix the problem, so this is more about the implementation cost than it is about the UX, so I'm not going to fight for making the type more general.

Yes, this is what I thought, that we have ability to implement the fix without necessary representing it as a node.


> But AFAIK we previously avoided subclasses concrete node classes.

True.

And this would also violate LSP ...

I don't understand that point.

LSP here is "Liskov Substitution Principle".


> > So, maybe make `EnumBody` a subtype of `ClassBody`?
>
> I think that would be reasonable. The presence of enum constants makes it feel less consistent than I was originally thinking it would be, but it would probably prove to be a good choice anyway.

OK, will update the CL a bit later, after another discussion.

Line 3380, Patchset 3:sealed class ClassNamePart implements AstNode {
Brian Wilkerson . unresolved

I don't like the name `ClassNamePart` because it's more than the name of the class. In it's most complete form it represents the name, type parameters, and primary constructor.

What would you think of calling this a `ClassHeader`? I'm not completely thrilled with the name because I would tend to include the `extends`, `with`, and `implements` clauses in the header, but I don't have a better name at this point and I think it might be better than `ClassNamePart`.

Also, as with `ClassBody`, it would be more consistent if we used this class for all of the top-level declarations that can have both a name and type parameters, even though some of those declarations can't have a primary constructor and hence would never return one of the two subclasses.

Konstantin Shcheglov

Yeah, unfortunately I also consider "header" to be everything from modifiers and `class` keyword to the body of the class, so including `extends`, `with`, and `implements`. So, I don't like calling it `ClassHeader`. It is a part of the declaration that contains the name (1), so `ClassNamePart`. Using the word "part" also feel lame to me, but it has benefits: short and descriptive (if you buy my explanation (1)).

Brian Wilkerson

I agree that 'part' feels lame. Yes, this subsection of the grammar is part of the declaration, but `ClassNamePart` sounds like it's claiming to be part of the _name_, which it isn't. The grammar piece is also part of the header, so `ClassHeaderPart` would be more accurate, though only slightly better.

The problem is that there's no coherent concept behind this production other than to avoid duplication in the grammar (and by extension in the node structure). It isn't a thing, it's just a semi-random collection of nodes. But `SharedSubsetOfAClassDeclaration` would be horrid.

We could also take inspiration from a comment above ("The name of the class ... or a primary constructor.") and call it `ClassNameOrPrimaryConstructor`, since it's a thing that could be either of those.

I don't have a good solution. :-/

Konstantin Shcheglov

I discussed this with LLMs, and got a few options.

`ClassDesignator`, field `designator`.

`ClassNameSpecifier`, field `nameSpecifier`.

`ClassDeclarator` - probably not good, too close to `ClassDeclaration`, field `declarator`.

Also `ClassNameDesignator`, but it might be "wet water"; although I like that it says "name" explicitly, field `nameDesignator`.

Open in Gerrit

Related details

Attention is currently required from:
  • Brian Wilkerson
  • Johnni Winther
  • Kallen Tu
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
Gerrit-Change-Number: 454322
Gerrit-PatchSet: 5
Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Kallen Tu <kall...@google.com>
Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
Gerrit-Attention: Kallen Tu <kall...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Fri, 10 Oct 2025 22:09:31 +0000
unsatisfied_requirement
open
diffy

Kallen Tu (Gerrit)

unread,
Oct 10, 2025, 6:49:52 PM (11 days ago) Oct 10
to Konstantin Shcheglov, Johnni Winther, Brian Wilkerson, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson, Johnni Winther and Konstantin Shcheglov

Kallen Tu added 1 comment

File pkg/analyzer/lib/src/dart/ast/ast.dart
Line 3380, Patchset 3:sealed class ClassNamePart implements AstNode {
Brian Wilkerson . unresolved

I don't like the name `ClassNamePart` because it's more than the name of the class. In it's most complete form it represents the name, type parameters, and primary constructor.

What would you think of calling this a `ClassHeader`? I'm not completely thrilled with the name because I would tend to include the `extends`, `with`, and `implements` clauses in the header, but I don't have a better name at this point and I think it might be better than `ClassNamePart`.

Also, as with `ClassBody`, it would be more consistent if we used this class for all of the top-level declarations that can have both a name and type parameters, even though some of those declarations can't have a primary constructor and hence would never return one of the two subclasses.

Konstantin Shcheglov

Yeah, unfortunately I also consider "header" to be everything from modifiers and `class` keyword to the body of the class, so including `extends`, `with`, and `implements`. So, I don't like calling it `ClassHeader`. It is a part of the declaration that contains the name (1), so `ClassNamePart`. Using the word "part" also feel lame to me, but it has benefits: short and descriptive (if you buy my explanation (1)).

Brian Wilkerson

I agree that 'part' feels lame. Yes, this subsection of the grammar is part of the declaration, but `ClassNamePart` sounds like it's claiming to be part of the _name_, which it isn't. The grammar piece is also part of the header, so `ClassHeaderPart` would be more accurate, though only slightly better.

The problem is that there's no coherent concept behind this production other than to avoid duplication in the grammar (and by extension in the node structure). It isn't a thing, it's just a semi-random collection of nodes. But `SharedSubsetOfAClassDeclaration` would be horrid.

We could also take inspiration from a comment above ("The name of the class ... or a primary constructor.") and call it `ClassNameOrPrimaryConstructor`, since it's a thing that could be either of those.

I don't have a good solution. :-/

Konstantin Shcheglov

I discussed this with LLMs, and got a few options.

`ClassDesignator`, field `designator`.

`ClassNameSpecifier`, field `nameSpecifier`.

`ClassDeclarator` - probably not good, too close to `ClassDeclaration`, field `declarator`.

Also `ClassNameDesignator`, but it might be "wet water"; although I like that it says "name" explicitly, field `nameDesignator`.

Kallen Tu

My two cents is I'm fine with either `ClassNamePart`, `ClassHeaderPart` or `ClassHeader`. Naming it designator or specifier isn't super clear IMO.

Open in Gerrit

Related details

Attention is currently required from:
  • Brian Wilkerson
  • Johnni Winther
  • Konstantin Shcheglov
Submit Requirements:
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
Gerrit-Change-Number: 454322
Gerrit-PatchSet: 5
Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
Gerrit-Reviewer: Kallen Tu <kall...@google.com>
Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
Gerrit-Attention: Konstantin Shcheglov <sche...@google.com>
Gerrit-Attention: Johnni Winther <johnni...@google.com>
Gerrit-Comment-Date: Fri, 10 Oct 2025 22:49:48 +0000
unsatisfied_requirement
open
diffy

Johnni Winther (Gerrit)

unread,
Oct 13, 2025, 5:20:40 AM (8 days ago) Oct 13
to Konstantin Shcheglov, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
Attention needed from Brian Wilkerson and Konstantin Shcheglov

Johnni Winther voted Code-Review+1

Code-Review+1
Open in Gerrit

Related details

Attention is currently required from:
  • Brian Wilkerson
  • Konstantin Shcheglov
Submit Requirements:
    • requirement is not 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
    Gerrit-Change-Number: 454322
    Gerrit-PatchSet: 5
    Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
    Gerrit-Reviewer: Kallen Tu <kall...@google.com>
    Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Attention: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Comment-Date: Mon, 13 Oct 2025 09:20:34 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Brian Wilkerson (Gerrit)

    unread,
    Oct 13, 2025, 10:38:36 AM (8 days ago) Oct 13
    to Konstantin Shcheglov, Johnni Winther, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
    Attention needed from Konstantin Shcheglov

    Brian Wilkerson added 1 comment

    File pkg/analyzer/lib/src/dart/ast/ast.dart
    Line 3380, Patchset 3:sealed class ClassNamePart implements AstNode {
    Brian Wilkerson . unresolved

    I don't like the name `ClassNamePart` because it's more than the name of the class. In it's most complete form it represents the name, type parameters, and primary constructor.

    What would you think of calling this a `ClassHeader`? I'm not completely thrilled with the name because I would tend to include the `extends`, `with`, and `implements` clauses in the header, but I don't have a better name at this point and I think it might be better than `ClassNamePart`.

    Also, as with `ClassBody`, it would be more consistent if we used this class for all of the top-level declarations that can have both a name and type parameters, even though some of those declarations can't have a primary constructor and hence would never return one of the two subclasses.

    Konstantin Shcheglov

    Yeah, unfortunately I also consider "header" to be everything from modifiers and `class` keyword to the body of the class, so including `extends`, `with`, and `implements`. So, I don't like calling it `ClassHeader`. It is a part of the declaration that contains the name (1), so `ClassNamePart`. Using the word "part" also feel lame to me, but it has benefits: short and descriptive (if you buy my explanation (1)).

    Brian Wilkerson

    I agree that 'part' feels lame. Yes, this subsection of the grammar is part of the declaration, but `ClassNamePart` sounds like it's claiming to be part of the _name_, which it isn't. The grammar piece is also part of the header, so `ClassHeaderPart` would be more accurate, though only slightly better.

    The problem is that there's no coherent concept behind this production other than to avoid duplication in the grammar (and by extension in the node structure). It isn't a thing, it's just a semi-random collection of nodes. But `SharedSubsetOfAClassDeclaration` would be horrid.

    We could also take inspiration from a comment above ("The name of the class ... or a primary constructor.") and call it `ClassNameOrPrimaryConstructor`, since it's a thing that could be either of those.

    I don't have a good solution. :-/

    Konstantin Shcheglov

    I discussed this with LLMs, and got a few options.

    `ClassDesignator`, field `designator`.

    `ClassNameSpecifier`, field `nameSpecifier`.

    `ClassDeclarator` - probably not good, too close to `ClassDeclaration`, field `declarator`.

    Also `ClassNameDesignator`, but it might be "wet water"; although I like that it says "name" explicitly, field `nameDesignator`.

    Kallen Tu

    My two cents is I'm fine with either `ClassNamePart`, `ClassHeaderPart` or `ClassHeader`. Naming it designator or specifier isn't super clear IMO.

    Brian Wilkerson

    I agree that the names from the LLM aren't good.

    `ClassDesignator` is what a class name is when used elsewhere in code (such as in a type annotation), not what the declaration of the class name is.

    `ClassNameDesignator` is even worse. It means a thing that refers to the name of the class, not to the class itself. Too many levels of indirection to be useful.

    `ClassNamePart` and `ClassNameSpecifier` are too limited; they sound like they're only the part that declares the class name.

    `ClassHeader` is too grandiose; it's not the whole header.

    `ClassHeaderPart` is accurate, but vague; it could be any part of the class header. Nevertheless, it's the best we've come up with so far.

    As I said before, the problem is that there's no semantic for these two pieces of the class header, so naming it is really hard. Vague but accurate might be the best we can hope for. That or not trying to artificially group them together.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Konstantin Shcheglov
    Submit Requirements:
    • requirement is not 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
    Gerrit-Change-Number: 454322
    Gerrit-PatchSet: 5
    Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
    Gerrit-Reviewer: Kallen Tu <kall...@google.com>
    Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Attention: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Comment-Date: Mon, 13 Oct 2025 14:38:32 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Brian Wilkerson <brianwi...@google.com>
    Comment-In-Reply-To: Kallen Tu <kall...@google.com>
    Comment-In-Reply-To: Konstantin Shcheglov <sche...@google.com>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Konstantin Shcheglov (Gerrit)

    unread,
    Oct 13, 2025, 1:04:05 PM (8 days ago) Oct 13
    to Johnni Winther, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
    Attention needed from Brian Wilkerson and Kallen Tu

    Konstantin Shcheglov added 1 comment

    File pkg/analyzer/lib/src/dart/ast/ast.dart
    Konstantin Shcheglov

    Now, with your explanation, I see that `ClassDesignator` and `ClassNameDesignator` are not good.

    `NodeWithClassName`, `ClassNameContainer`, `ClassName`.

    I read `ClassNamePart` with the right pause: "class" <pause> "name part".

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Brian Wilkerson
    • Kallen Tu
    Submit Requirements:
    • requirement is not 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
    Gerrit-Change-Number: 454322
    Gerrit-PatchSet: 5
    Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
    Gerrit-Reviewer: Kallen Tu <kall...@google.com>
    Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
    Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
    Gerrit-Attention: Kallen Tu <kall...@google.com>
    Gerrit-Comment-Date: Mon, 13 Oct 2025 17:04:01 +0000
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Konstantin Shcheglov (Gerrit)

    unread,
    Oct 13, 2025, 10:29:45 PM (8 days ago) Oct 13
    to Johnni Winther, Brian Wilkerson, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
    Attention needed from Brian Wilkerson, Johnni Winther and Kallen Tu

    Konstantin Shcheglov added 1 comment

    File pkg/analyzer/lib/src/dart/ast/ast.dart
    Line 2895, Patchset 3:sealed class ClassBody implements AstNode {}
    Brian Wilkerson . unresolved

    I like `ClassBody`, but it would be more consistent if we also used it for extensions and mixins. I understand that only one of the two subclasses could validly be used in either of these cases, but I think the consistency (and possibly the ability to pass a `ClassBody` around no matter what kind of parent it has) would make it worthwhile.

    Konstantin Shcheglov

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Brian Wilkerson
    • Johnni Winther
    • Kallen Tu
    Submit Requirements:
      • requirement is not satisfiedCode-Owners
      • requirement is not satisfiedCode-Review
      • 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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
      Gerrit-Change-Number: 454322
      Gerrit-PatchSet: 7
      Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
      Gerrit-Reviewer: Kallen Tu <kall...@google.com>
      Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Attention: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Attention: Kallen Tu <kall...@google.com>
      Gerrit-Attention: Johnni Winther <johnni...@google.com>
      Gerrit-Comment-Date: Tue, 14 Oct 2025 02:29:41 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Brian Wilkerson <brianwi...@google.com>
      Comment-In-Reply-To: Konstantin Shcheglov <sche...@google.com>
      unsatisfied_requirement
      open
      diffy

      Brian Wilkerson (Gerrit)

      unread,
      Oct 14, 2025, 12:04:51 PM (7 days ago) Oct 14
      to Konstantin Shcheglov, Brian Wilkerson, Johnni Winther, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
      Attention needed from Johnni Winther, Kallen Tu and Konstantin Shcheglov

      Brian Wilkerson voted and added 3 comments

      Votes added by Brian Wilkerson

      Code-Review+1

      3 comments

      File pkg/analyzer/lib/src/dart/ast/ast.dart
      Line 2895, Patchset 3:sealed class ClassBody implements AstNode {}
      Brian Wilkerson . resolved
      Brian Wilkerson

      Thanks.

      Line 3380, Patchset 3:sealed class ClassNamePart implements AstNode {
      Brian Wilkerson . resolved
      Brian Wilkerson

      I still don't like it. It isn't the 'name' part, it's the 'name, type parameters, and possibly primary constructor part'.

      But it isn't reasonable to block a CL for an issue that nobody can solve, so I won't.

      Line 7487, Patchset 7 (Latest): /// The body of the class declaration.
      Brian Wilkerson . unresolved

      nit: "class" --> "enum"

      (Not worth holding up the CL over.)

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Johnni Winther
      • Kallen Tu
      • Konstantin Shcheglov
      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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
      Gerrit-Change-Number: 454322
      Gerrit-PatchSet: 7
      Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
      Gerrit-Reviewer: Kallen Tu <kall...@google.com>
      Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Attention: Kallen Tu <kall...@google.com>
      Gerrit-Attention: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Attention: Johnni Winther <johnni...@google.com>
      Gerrit-Comment-Date: Tue, 14 Oct 2025 16:04:46 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Comment-In-Reply-To: Brian Wilkerson <brianwi...@google.com>
      satisfied_requirement
      open
      diffy

      Konstantin Shcheglov (Gerrit)

      unread,
      Oct 15, 2025, 11:47:52 AM (6 days ago) Oct 15
      to Brian Wilkerson, Johnni Winther, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
      Attention needed from Johnni Winther and Kallen Tu

      Konstantin Shcheglov added 1 comment

      File pkg/analyzer/lib/src/dart/ast/ast.dart
      Line 7487, Patchset 7: /// The body of the class declaration.
      Brian Wilkerson . resolved

      nit: "class" --> "enum"

      (Not worth holding up the CL over.)

      Konstantin Shcheglov

      Done

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Johnni Winther
      • Kallen Tu
      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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
      Gerrit-Change-Number: 454322
      Gerrit-PatchSet: 8
      Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
      Gerrit-Reviewer: Kallen Tu <kall...@google.com>
      Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Attention: Kallen Tu <kall...@google.com>
      Gerrit-Attention: Johnni Winther <johnni...@google.com>
      Gerrit-Comment-Date: Wed, 15 Oct 2025 15:47:49 +0000
      satisfied_requirement
      open
      diffy

      Konstantin Shcheglov (Gerrit)

      unread,
      Oct 15, 2025, 12:19:04 PM (6 days ago) Oct 15
      to Brian Wilkerson, Johnni Winther, Kallen Tu, Commit Queue, dart-analys...@google.com, rev...@dartlang.org
      Attention needed from Johnni Winther and Kallen Tu

      Konstantin Shcheglov voted Commit-Queue+2

      Commit-Queue+2
      Gerrit-Comment-Date: Wed, 15 Oct 2025 16:19:01 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      open
      diffy

      Commit Queue (Gerrit)

      unread,
      Oct 15, 2025, 12:19:16 PM (6 days ago) Oct 15
      to Konstantin Shcheglov, Brian Wilkerson, Johnni Winther, Kallen Tu, dart-analys...@google.com, rev...@dartlang.org

      Commit Queue submitted the change with unreviewed changes

      Unreviewed changes

      7 is the latest approved patch-set.
      The change was submitted with unreviewed changes in the following files:

      ```
      The name of the file: pkg/analyzer/lib/src/dart/ast/ast.dart
      Insertions: 1, Deletions: 1.

      @@ -7484,7 +7484,7 @@
      /// The `augment` keyword, or `null` if the keyword was absent.
      Token? get augmentKeyword;

      - /// The body of the class declaration.
      + /// The body of the enum declaration.
      ///
      /// Will become not `null` when [Feature.declaring_constructors] is
      /// implemented, replaces [leftBracket], [constants], [semicolon], [members],
      ```

      Change information

      Commit message:
      DeCo. AST for PrimaryConstructorDeclaration and ClassBody.
      Change-Id: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
      Reviewed-by: Brian Wilkerson <brianwi...@google.com>
      Commit-Queue: Konstantin Shcheglov <sche...@google.com>
      Files:
      • M pkg/analyzer/api.txt
      • M pkg/analyzer/lib/analysis_rule/rule_visitor_registry.dart
      • M pkg/analyzer/lib/analysis_rule/rule_visitor_registry.g.dart
      • M pkg/analyzer/lib/dart/ast/ast.dart
      • M pkg/analyzer/lib/dart/ast/visitor.dart
      • M pkg/analyzer/lib/dart/ast/visitor.g.dart
      • M pkg/analyzer/lib/src/dart/ast/ast.dart
      • M pkg/analyzer/lib/src/dart/ast/ast.g.dart
      • M pkg/analyzer/lib/src/dart/ast/to_source_visitor.dart
      • M pkg/analyzer/lib/src/lint/linter_visitor.dart
      • M pkg/analyzer/lib/src/lint/linter_visitor.g.dart
      Change size: XL
      Delta: 11 files changed, 1128 insertions(+), 0 deletions(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Brian Wilkerson
      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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
      Gerrit-Change-Number: 454322
      Gerrit-PatchSet: 9
      Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      open
      diffy
      satisfied_requirement

      Paul Berry (Gerrit)

      unread,
      Oct 20, 2025, 5:39:37 PM (20 hours ago) Oct 20
      to Konstantin Shcheglov, Commit Queue, Brian Wilkerson, Johnni Winther, Kallen Tu, dart-analys...@google.com, rev...@dartlang.org
      Attention needed from Konstantin Shcheglov

      Paul Berry voted and added 2 comments

      Votes added by Paul Berry

      Code-Review+1

      2 comments

      Patchset-level comments
      File pkg/analyzer/lib/src/dart/ast/ast.dart
      Line 3049, Patchset 9 (Latest): /// implemented, replaces [name].
      Paul Berry . unresolved

      Shouldn't this be `replaces [name] and [typeParameters]`?

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Konstantin Shcheglov
      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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
      Gerrit-Change-Number: 454322
      Gerrit-PatchSet: 9
      Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
      Gerrit-Reviewer: Kallen Tu <kall...@google.com>
      Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Paul Berry <paul...@google.com>
      Gerrit-Attention: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Comment-Date: Mon, 20 Oct 2025 21:39:34 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      open
      diffy

      Konstantin Shcheglov (Gerrit)

      unread,
      Oct 20, 2025, 8:36:53 PM (17 hours ago) Oct 20
      to Commit Queue, Paul Berry, Brian Wilkerson, Johnni Winther, Kallen Tu, dart-analys...@google.com, rev...@dartlang.org

      Konstantin Shcheglov added 1 comment

      File pkg/analyzer/lib/src/dart/ast/ast.dart
      Line 3049, Patchset 9 (Latest): /// implemented, replaces [name].
      Paul Berry . resolved

      Shouldn't this be `replaces [name] and [typeParameters]`?

      Konstantin Shcheglov

      This was fixed in a way in https://dart-review.googlesource.com/c/sdk/+/455820, replaced completely.

      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: I06cfa02d8c48c85e2ddd54d7f8308d187d7de5a1
      Gerrit-Change-Number: 454322
      Gerrit-PatchSet: 9
      Gerrit-Owner: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Brian Wilkerson <brianwi...@google.com>
      Gerrit-Reviewer: Johnni Winther <johnni...@google.com>
      Gerrit-Reviewer: Kallen Tu <kall...@google.com>
      Gerrit-Reviewer: Konstantin Shcheglov <sche...@google.com>
      Gerrit-Reviewer: Paul Berry <paul...@google.com>
      Gerrit-Comment-Date: Tue, 21 Oct 2025 00:36:51 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      Comment-In-Reply-To: Paul Berry <paul...@google.com>
      satisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages