Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2145. Add more tests for Variable #2146

Merged
merged 4 commits into from
Jul 26, 2023
Merged

Conversation

sgrekhov
Copy link
Contributor

It's just the first part of the changes

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I just noted a couple of typos and suggested that we should avoid non-standard leading whitespace by adding a comment.

Language/Variables/covariant_t01.dart Outdated Show resolved Hide resolved
Language/Variables/covariant_t01.dart Outdated Show resolved Hide resolved
Language/Variables/multiple_t03.dart Outdated Show resolved Hide resolved
Language/Variables/multiple_t03.dart Outdated Show resolved Hide resolved
Language/Variables/static_variable_t02.dart Outdated Show resolved Hide resolved
Language/Variables/static_variable_t02.dart Outdated Show resolved Hide resolved
Language/Variables/static_variable_t02.dart Outdated Show resolved Hide resolved
Language/Variables/static_variable_t02.dart Outdated Show resolved Hide resolved
Language/Variables/static_variable_t01.dart Outdated Show resolved Hide resolved
Language/Variables/static_variable_t02.dart Outdated Show resolved Hide resolved
Copy link
Contributor Author

@sgrekhov sgrekhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All requested changes are made except Language/Variables/multiple_t03.dart. I think it makes sense to change the name and assertion of the test but I can't find the appropriate statement in the spec

Language/Variables/multiple_t03.dart Outdated Show resolved Hide resolved
@sgrekhov sgrekhov requested a review from eernstg July 21, 2023 14:38
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Responded to comment)

Language/Variables/multiple_t03.dart Outdated Show resolved Hide resolved
@sgrekhov
Copy link
Contributor Author

Found that we have no such tests at all! Created #2147 and deleted multiple_t03.dart from the current PR as not relevant

@sgrekhov sgrekhov requested a review from eernstg July 24, 2023 09:11
@sgrekhov
Copy link
Contributor Author

Friendly ping

@eernstg
Copy link
Member

eernstg commented Jul 25, 2023

Ah, I'll take a look tomorrow.

Add a `static late` case along with the `late static` case. The point is that `static late` _does_ exist in the grammar, but `late static` is always a syntax error, so we might have a bug where `static late` is accepted in this case because it is accepted in some cases, and that one is perhaps more likely than the bug where `late static` is accepted, anywhere.
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I added a static late case to go along with the late static case.

@eernstg eernstg merged commit a4d7265 into dart-lang:master Jul 26, 2023
1 of 2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 31, 2023
2023-07-28 sgrekhov22@gmail.com dart-lang/co19#2142. Fix typo (dart-lang/co19#2155)
2023-07-26 sgrekhov22@gmail.com dart-lang/co19#2149. Fix new errors due to changes with the constant evaluator (dart-lang/co19#2152)
2023-07-26 sgrekhov22@gmail.com dart-lang/co19#2145. Add more tests for Variable (dart-lang/co19#2146)
2023-07-25 sgrekhov22@gmail.com dart-lang/co19#2142. Add syntax tests (dart-lang/co19#2148)

Change-Id: I6ee222b74fc092d93a1b76d35d89a008a97056be
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316590
Commit-Queue: William Hesse <whesse@google.com>
Reviewed-by: William Hesse <whesse@google.com>
@sgrekhov sgrekhov deleted the co19-2145 branch November 14, 2023 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants