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

Change all late final Iterable fields to Lists #3649

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Feb 6, 2024

I noticed a late final Iterable<...> field and thought to myself, "uh oh! That could be an expensive field!" The idea is that Iterables are lazy, so even if the field is "final", the Iterable calculations are re-executed on each access. So storing a lazily evaluated Iterable on a field seemed very scary 😱 . It may warrant a lint rule.

Luckily, as I walked through the cases I found with grep, most of the values at runtime are Lists, not Iterables. Shwew. Still, I ran through and changed all of the signatures for future-proofing.

Two fields that were Iterables were converted to Lists. I did not benchmark.


  • 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.

}
var relativePath =
pathContext.relative(fullName, from: package.packagePath);
if (!pathContext.isWithin(fullName, package.packagePath) &&
Copy link
Member Author

Choose a reason for hiding this comment

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

I confess I don't know how the formatter got into this state...

@srawlins srawlins requested a review from kallentu February 6, 2024 23:09
@srawlins srawlins merged commit d0c53b9 into dart-lang:main Feb 8, 2024
9 checks passed
@srawlins srawlins deleted the no-late-final-iterable branch February 8, 2024 01:23
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Feb 13, 2024
Revisions updated by `dart tools/rev_sdk_deps.dart`.

crypto (https://github.com/dart-lang/crypto/compare/c954015..f059196):
  f059196  2024-02-08  Kevin Moore  Test dart2wasm (dart-archive/crypto#162)

dartdoc (https://github.com/dart-lang/dartdoc/compare/457c34e..f152c01):
  f152c013  2024-02-08  Sam Rawlins  Tidy spacing in templates (dart-lang/dartdoc#3652)
  d0c53b9f  2024-02-07  Sam Rawlins  Change all late final Iterable fields to Lists (dart-lang/dartdoc#3649)
  8c9c7790  2024-02-06  Sam Rawlins  Remove unnecessary ExtensionTarget mixin (dart-lang/dartdoc#3648)
  d2f90c5a  2024-02-06  Sam Rawlins  Fix the 'serve testing-package' task to generate docs (dart-lang/dartdoc#3647)

ecosystem (https://github.com/dart-lang/ecosystem/compare/7d8ec47..3e4f286):
  3e4f286  2024-02-08  Moritz  Add ignore flag for `publish.yaml` (dart-lang/ecosystem#237)

lints (https://github.com/dart-lang/lints/compare/90a61e4..ead7708):
  ead7708  2024-02-08  Devon Carew  add library_annotations; remove package_prefixed_library_names (dart-lang/lints#179)

native (https://github.com/dart-lang/native/compare/876f9a1..cb9bd7e):
  cb9bd7ef  2024-02-09  Daco Harkes  Build hook spec (dart-lang/native#955)

sse (https://github.com/dart-lang/sse/compare/e483b14..af7d8d0):
  af7d8d0  2024-02-09  Ilya Yanok  Make `SseConnection` extend `StreamChannelMixin<String>` (dart-archive/sse#102)

Change-Id: I2e85dbfae00eea8de7b6f7efce1993bf00e243f0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352046
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
Auto-Submit: Devon Carew <devoncarew@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