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

Refactor special case for how canonical enclosing containers are calculated #3925

Merged
merged 1 commit into from
Nov 12, 2024

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 <scheglov@google.com>
Auto-Submit: Devon Carew <devoncarew@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
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