Skip to content

Conversation

@srawlins
Copy link
Member

@srawlins srawlins commented Nov 11, 2024

Work towards #3927

There is some complicated logic in the calculations for determining a canonical enclosing container, when we find ourselves examining a "hidden" interface. The list of "hidden" interfaces, which we should not consider when determining a canonical enclosing container, is exactly one: Interceptor. But it's important that it stay buried.

The logic was hard to read, so first, I split out a complicated if-condition into a few local variables. The logic also needs to track the "current" container in the inheritance list, and the "previous", and the "previous visible." This was complicated when using a for-in loop. I changed it to use a standard for loop with indices.

Then I saw that we only ever find outself asking a container for it's "memberByExample" in the case when we're looking for one of Object's members (toString, noSuchMethod, operator ==, hashCode, and runtimeType. We can inline the logic to fetch a member by example. This allows us to delete a late field on every Container.

Also, the package graph _invisibleAnnotations and _inheritThrough fields had very complicated initializers which I simplified with if-elements.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

…ulated

There is some complicated logic in the calculations for determining a canonical
enclosing container, when we find ourselves examining a "hidden" interface. The
list of "hidden" interfaces, which we should not consider when determining a
canonical enclosing container, is exactly one: `Interceptor`. But it's
important that it stay buried.

The logic was hard to read, so first, I split out a complicated if-condition
into a few local variables. The logic also needs to track the "current"
container in the inheritance list, and the "previous", and the "previous
visible." This was complicated when using a for-in loop. I changed it to use a
standard for loop with indices.

Then I saw that we only ever find outself asking a container for it's
"memberByExample" in the case when we're looking for one of Object's members
(`toString`, `noSuchMethod`, `operator ==`, `hashCode`, and `runtimeType`. We
can inline the logic to fetch a member by example. This allows us to delete a
late field on every Container.

Also, the package graph `_invisibleAnnotations` and `_inheritThrough` fields
had very complicated initializers which I simplified with if-elements.
@srawlins
Copy link
Member Author

Ready for review, @dart-lang/analyzer-team

@srawlins srawlins merged commit dcc239a into dart-lang:main Nov 12, 2024
7 checks passed
@srawlins srawlins deleted the memberbyexample branch November 12, 2024 20:34
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Nov 14, 2024
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/24c2a96..6bbd3d7):
  6bbd3d7b  2024-11-14  Sam Rawlins  Do not use SpecialClass to evaluate Enum or Interceptor (dart-lang/dartdoc#3928)
  dcc239a4  2024-11-12  Sam Rawlins  Refactor special case for how canonical enclosing containers are calculated (dart-lang/dartdoc#3925)
  80032309  2024-11-11  Sam Rawlins  Ignore static issue with overridden field (dart-lang/dartdoc#3926)

http (https://github.com/dart-lang/http/compare/03ced4d..2f954e1):
  2f954e1  2024-11-11  Brian Quinlan  Add macOS tests for `package:cupertino_http` (dart-lang/http#1403)

shelf (https://github.com/dart-lang/shelf/compare/1a141c7..0bb44cb):
  0bb44cb  2024-11-12  Devon Carew  add direct links to the package issues (dart-lang/shelf#455)

tools (https://github.com/dart-lang/tools/compare/66afa68..a6603a4):
  a6603a45  2024-11-11  Liam Appelbe  [coverage] Fix isolate resumption after service disposal (dart-lang/tools#1189)

Change-Id: I715f943ff69ae24b5126a7ddf883d31aed180632
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/395402
Reviewed-by: Konstantin Shcheglov <[email protected]>
Auto-Submit: Devon Carew <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
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.

2 participants