Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(669)

Issue 1386363002: Make ResolveVariableReferencesTask depend on RESOLVED_UNIT3. (Closed)

Created:
5 years, 2 months ago by Paul Berry
Modified:
5 years, 2 months ago
CC:
reviews_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Make ResolveVariableReferencesTask depend on RESOLVED_UNIT3. Technically, ResolveVariableReferencesTask only requires the information guaranteed by RESOLVED_UNIT1 to do its job. However, later tasks, such as ResolveFunctionBodiesInUnitTask, require the information computed by ResolveUnitTypeNamesTask (which produces RESOLVED_UNIT3). Normally this is not a problem, because ResolveFunctionBodiesInUnitTask depends (via a rather complex dependency chain) upon LIBRARY_ELEMENT5, which in turn depends on ResolveLibraryTypeNamesTask, which depends on RESOLVED_UNIT3. So ResolvuUnitTypeNamesTask winds up getting run. However, it's possible that due to analysis cache pressure, the resolved unit will get thrown away, while the library element is preserved. If this happens, then when the resolved unit is being recomputed, it won't be necessary to run ResolveLibraryTypeNamesTask in order to recompute LIBRARY_ELEMENT5 (becase LIBRARY_ELEMENT5 is still cached). As a result, nothing is left that depends on RESOLVED_UNIT3, so there is nothing to force ResolveUnitTypeNamesTask to run. The outward manifestation of this problem is that when trying to resolve a library, a TypeName will be found whose ".type" property is unexpectedly null, causing an exception in the ResolverVisitor. The easy fix is to make ResolveVariableReferencesTask depend on RESOLVED_UNIT3, so there is a complete dependency chain through all the RESOLVED_UNIT[N] results that doesn't rely on any LIBRARY_ELEMENT[N] intermediates. This is not an ideal fix because it forces us to compute RESOLVED_UNITs 2 and 3 even if they might not be needed. Fortunately, computing these RESOLVED_UNITs should be fast, so I don't expect a significant performance penalty. R=brianwilkerson@google.com Committed: https://github.com/dart-lang/sdk/commit/e40b231771bfb24411dff6bba7178869636427cd

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M pkg/analyzer/lib/src/task/dart.dart View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 5 (1 generated)
Paul Berry
5 years, 2 months ago (2015-10-07 00:05:00 UTC) #2
Brian Wilkerson
LGTM
5 years, 2 months ago (2015-10-07 00:26:48 UTC) #3
Paul Berry
Committed patchset #1 (id:1) manually as e40b231771bfb24411dff6bba7178869636427cd (presubmit successful).
5 years, 2 months ago (2015-10-07 01:21:16 UTC) #4
scheglov
5 years, 2 months ago (2015-10-07 02:47:26 UTC) #5
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698