[L] Change in dart/sdk[main]: Elements. Issue 62826. Support aliases for DynamicType, NeverType, Vo...
Paul Berry (Gerrit)
Paul Berry added 3 comments![]( "Open in Gerrit")
CHANGELOG entry and commit message should explain that it is no longer safe to assume that there is a single instance of `DynamicType`, `NeverType`, and `VoidType`.
/// The unique instance of this class.This doc comment is now incorrect. Same problem with `NeverTypeImpl` and `VoidTypeImpl`.
Should we consider deprecating or removing this field entirely to avoid confusion?
if (T is DynamicTypeImpl) {In some parts of this CL, `identical(t, DynamicTypeImpl.instance)` is changed to `t == DynamicTypeImpl.instance`; in other parts (like here), it is changed to `t is DynamicTypeImpl`. Which is better, and why? The CL should pick one consistent approach, and it should be documented in the commit message (as well as the CHANGELOG, so that authors of analyzer clients know what to do).
Related details
- Johnni Winther
- Konstantin Shcheglov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov voted and added 3 comments![]( "Open in Gerrit")
Votes added by Konstantin Shcheglov
| Commit-Queue | +1 |
CHANGELOG entry and commit message should explain that it is no longer safe to assume that there is a single instance of `DynamicType`, `NeverType`, and `VoidType`.
Done.
This doc comment is now incorrect. Same problem with `NeverTypeImpl` and `VoidTypeImpl`.
Should we consider deprecating or removing this field entirely to avoid confusion?
I updated the comments.
We still need values for comparison.
In some parts of this CL, `identical(t, DynamicTypeImpl.instance)` is changed to `t == DynamicTypeImpl.instance`; in other parts (like here), it is changed to `t is DynamicTypeImpl`. Which is better, and why? The CL should pick one consistent approach, and it should be documented in the commit message (as well as the CHANGELOG, so that authors of analyzer clients know what to do).
Authors of the analyzer clients don't get access to `DynamicTypeImpl.instance`.
`t is DynamicTypeImpl` is better, just one type check, no virtual method invocation.
Related details
- Johnni Winther
- Paul Berry
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Berry (Gerrit)
Paul Berry voted and added 2 comments![]( "Open in Gerrit")
Votes added by Paul Berry
| Code-Review | +1 |
2 comments
lgtm assuming commit message and CHANGELOG are updated.
or equality operators (`==`) instead of `identical()`.Since you have decided that using `is` checks is better (and I agree with your reasoning), the commit message and the CHANGELOG should reflect this decision. Don't suggest using equality operators.
Related details
- Johnni Winther
- Konstantin Shcheglov
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Konstantin Shcheglov (Gerrit)
Konstantin Shcheglov voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
- Johnni Winther
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
Commit Queue submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
5 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: pkg/analyzer/CHANGELOG.md
Insertions: 1, Deletions: 3.
@@ -1,9 +1,7 @@
## 12.1.0-dev
* Support type aliases for `dynamic`, `Never`, and `void`. As a result, it is no longer safe to assume that
- there is a single instance of `DynamicType`, `NeverType`, or `VoidType`. Use type checks (`is DynamicType`)
- or equality operators (`==`) instead of `identical()`.
-
+ there is a single instance of `DynamicType`, `NeverType`, or `VoidType`. Use type checks (`is DynamicType`) instead of `identical()`.
## 12.0.0
```
Change information
Elements. Issue 62826. Support aliases for DynamicType, NeverType, VoidType.
As a result, it is no longer safe to assume that
there is a single instance of `DynamicType`, `NeverType`, or `VoidType`. Use type checks (`is DynamicType`) instead of `identical()`.
- M pkg/analyzer/CHANGELOG.md
- M pkg/analyzer/lib/src/dart/element/element.dart
- M pkg/analyzer/lib/src/dart/element/normalize.dart
- M pkg/analyzer/lib/src/dart/element/subtype.dart
- M pkg/analyzer/lib/src/dart/element/top_merge.dart
- M pkg/analyzer/lib/src/dart/element/type.dart
- M pkg/analyzer/lib/src/dart/element/type_system.dart
- M pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart
- M pkg/analyzer/lib/src/dart/resolver/function_expression_invocation_resolver.dart
- M pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart
- M pkg/analyzer/lib/src/dart/resolver/yield_statement_resolver.dart
- M pkg/analyzer/lib/src/error/bool_expression_verifier.dart
- M pkg/analyzer/lib/src/generated/error_detection_helpers.dart
- M pkg/analyzer/lib/src/summary2/bundle_reader.dart
- M pkg/analyzer/test/src/dart/resolution/named_type_test.dart
- M pkg/analyzer/test/src/dart/resolution/object_pattern_test.dart
- M pkg/analyzer/test/src/summary/elements/type_alias_test.dart
Code-Review: +1 by Paul Berry
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |