[L] Change in dart/sdk[main]: [record_use] Record kinds and disambiguators
Daco Harkes (Gerrit)
Daco Harkes voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Nate Biggs
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daco Harkes (Gerrit)
Daco Harkes added 1 comment![]( "Open in Gerrit")
- Johnni Winther
- Nate Biggs
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nate Biggs (Gerrit)
Nate Biggs added 4 comments![]( "Open in Gerrit")
Definition _getDefinition(Entity entity, String? metadata) {This code is very difficult to read.
(1) Can you add a comment describing the format of the `metadata` String input?
(2) Can we do something a little more type-safe than a `|` separated String? As is this feels very error prone.
final parts = metadata.split('|');Why use `|` as a separator? What about something like a static extension name that already has a `|` in it?
/// desugaring or lowering (like extension methods).I feel like this would ideally be handled by a shared `Visitor` that collects the information and then each backend can call it and access the collected information directly on that Visitor.
Or if that doesn't work then using a [MetadataRepository](https://github.com/dart-lang/sdk/blob/d5647cb755ef58ffd1f572438447dee013f84ea0/pkg/kernel/lib/src/ast/components.dart#L323) to attach metadata to the relevant nodes.
As mentioned above, IMO this annotation is holding too much information in a String blob and either of those options should allow you to do something more type-safe.
constant.value.startsWith('record-use:definition:')) {Avoid magic strings.
Related details
- Daco Harkes
- Johnni Winther
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |