Skip to content

Commit cc6ef3a

Browse files
stereotype441Commit Queue
authored andcommitted
[analyzer] Avoid call from one visit method to another in resolver.
When `PrefixedIdentifierResolver.resolve` rewrites a prefixed identifier expression to a property access (which it does when the prefix is an expression having record type), instead of making a call from `ResolverVisitor.visitPropertyAccess` to `ResolverVisitor.visitPropertyAccess` to resolve the rewritten expression, use a private helper method for both code paths. This ensures that all `visit` methods in the `ResolverVisitor` are called solely from `ExpressionImpl.accept` or from `ExpressionImpl.resolveExpression` (at least as far as expressions are concerned). This will pave the way for a follow-up CL, in which I will make sure that all expression resolution is done through the `TypeAnalyzer.analyzeExpression` method. That will in turn allow the analyzer to use the shared implementation of null-shorting introduced in https://dart-review.googlesource.com/c/sdk/+/399480. Change-Id: I782e10be2f58fdc0991b6d917f81bedd856485a9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/399627 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent f5d6144 commit cc6ef3a

File tree

2 files changed

+65
-71
lines changed

2 files changed

+65
-71
lines changed

pkg/_fe_analyzer_shared/lib/src/type_inference/shared_inference_log.dart

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,7 @@ abstract interface class SharedInferenceLogWriter<
194194
void exitElement(Object node);
195195

196196
/// Called when type inference has finished inferring an expression.
197-
///
198-
/// [reanalyze] should be `true` if type inference will follow up by
199-
/// re-inferring the same expression in a different form (and hence, it's not
200-
/// necessary to assign a type to [node]).
201-
void exitExpression(Object node, {bool reanalyze = false});
197+
void exitExpression(Object node);
202198

203199
/// Called when type inference has finished inferring an AST node associated
204200
/// with extension override syntax.
@@ -527,20 +523,14 @@ abstract class SharedInferenceLogWriterImpl<
527523
}
528524

529525
@override
530-
void exitExpression(Object node, {bool reanalyze = false}) {
526+
void exitExpression(Object node) {
531527
checkCall(
532528
method: 'exitExpression',
533529
arguments: [node],
534-
namedArguments: {if (reanalyze) 'reanalyze': reanalyze},
535530
expectedNode: node,
536531
expectedKind: StateKind.expression);
537532
bool typeRecorded = (state as ExpressionState).typeRecorded;
538-
if (reanalyze) {
539-
if (typeRecorded) {
540-
fail('Tried to reanalyze after already recording a static type');
541-
}
542-
addEvent(new Event(message: 'WILL REANALYZE AS OTHER EXPRESSION'));
543-
} else if (!typeRecorded) {
533+
if (!typeRecorded) {
544534
fail('Failed to record a type for $state');
545535
}
546536
popState();

pkg/analyzer/lib/src/generated/resolver.dart

Lines changed: 62 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3411,8 +3411,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
34113411
var rewrittenPropertyAccess =
34123412
_prefixedIdentifierResolver.resolve(node, contextType: contextType);
34133413
if (rewrittenPropertyAccess != null) {
3414-
inferenceLogWriter?.exitExpression(node, reanalyze: true);
3415-
visitPropertyAccess(rewrittenPropertyAccess, contextType: contextType);
3414+
_resolvePropertyAccess(rewrittenPropertyAccess, contextType);
34163415
// We did record that `node` was replaced with `rewrittenPropertyAccess`.
34173416
// But if `rewrittenPropertyAccess` was itself rewritten, replace the
34183417
// rewrite result of `node`.
@@ -3423,6 +3422,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
34233422
}
34243423
return true;
34253424
}());
3425+
inferenceLogWriter?.exitExpression(node);
34263426
return;
34273427
}
34283428
_insertImplicitCallReference(
@@ -3450,62 +3450,7 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
34503450
inferenceLogWriter?.enterExpression(node, contextType);
34513451
checkUnreachableNode(node);
34523452

3453-
var target = node.target;
3454-
if (target != null) {
3455-
analyzeExpression(
3456-
target, SharedTypeSchemaView(UnknownInferredType.instance));
3457-
popRewrite();
3458-
}
3459-
3460-
if (node.isNullAware) {
3461-
_startNullAwareAccess(node, node.target);
3462-
}
3463-
3464-
var result = _propertyElementResolver.resolvePropertyAccess(
3465-
node: node,
3466-
hasRead: true,
3467-
hasWrite: false,
3468-
);
3469-
3470-
var element = result.readElement;
3471-
3472-
var propertyName = node.propertyName;
3473-
propertyName.staticElement = element;
3474-
3475-
DartType type;
3476-
if (element is MethodElement) {
3477-
type = element.type;
3478-
} else if (element is PropertyAccessorElement && element.isGetter) {
3479-
type = result.getType!;
3480-
} else if (result.functionTypeCallType != null) {
3481-
type = result.functionTypeCallType!;
3482-
} else if (result.recordField != null) {
3483-
type = result.recordField!.type;
3484-
} else if (result.atDynamicTarget) {
3485-
type = DynamicTypeImpl.instance;
3486-
} else {
3487-
type = InvalidTypeImpl.instance;
3488-
}
3489-
3490-
if (!isConstructorTearoffsEnabled) {
3491-
// Only perform a generic function instantiation on a [PrefixedIdentifier]
3492-
// in pre-constructor-tearoffs code. In constructor-tearoffs-enabled code,
3493-
// generic function instantiation is performed at assignability check
3494-
// sites.
3495-
// TODO(srawlins): Switch all resolution to use the latter method, in a
3496-
// breaking change release.
3497-
type = inferenceHelper.inferTearOff(node, propertyName, type,
3498-
contextType: contextType);
3499-
}
3500-
3501-
propertyName.setPseudoExpressionStaticType(type);
3502-
node.recordStaticType(type, resolver: this);
3503-
var replacement =
3504-
insertGenericFunctionInstantiation(node, contextType: contextType);
3505-
3506-
nullShortingTermination(node, rewrittenExpression: replacement);
3507-
_insertImplicitCallReference(replacement, contextType: contextType);
3508-
nullSafetyDeadCodeVerifier.verifyPropertyAccess(node);
3453+
_resolvePropertyAccess(node, contextType);
35093454
inferenceLogWriter?.exitExpression(node);
35103455
}
35113456

@@ -4140,6 +4085,65 @@ class ResolverVisitor extends ThrowingAstVisitor<void>
41404085
callReference.setPseudoExpressionStaticType(callMethodType);
41414086
}
41424087

4088+
void _resolvePropertyAccess(PropertyAccessImpl node, DartType contextType) {
4089+
var target = node.target;
4090+
if (target != null) {
4091+
analyzeExpression(
4092+
target, SharedTypeSchemaView(UnknownInferredType.instance));
4093+
popRewrite();
4094+
}
4095+
4096+
if (node.isNullAware) {
4097+
_startNullAwareAccess(node, node.target);
4098+
}
4099+
4100+
var result = _propertyElementResolver.resolvePropertyAccess(
4101+
node: node,
4102+
hasRead: true,
4103+
hasWrite: false,
4104+
);
4105+
4106+
var element = result.readElement;
4107+
4108+
var propertyName = node.propertyName;
4109+
propertyName.staticElement = element;
4110+
4111+
DartType type;
4112+
if (element is MethodElement) {
4113+
type = element.type;
4114+
} else if (element is PropertyAccessorElement && element.isGetter) {
4115+
type = result.getType!;
4116+
} else if (result.functionTypeCallType != null) {
4117+
type = result.functionTypeCallType!;
4118+
} else if (result.recordField != null) {
4119+
type = result.recordField!.type;
4120+
} else if (result.atDynamicTarget) {
4121+
type = DynamicTypeImpl.instance;
4122+
} else {
4123+
type = InvalidTypeImpl.instance;
4124+
}
4125+
4126+
if (!isConstructorTearoffsEnabled) {
4127+
// Only perform a generic function instantiation on a [PrefixedIdentifier]
4128+
// in pre-constructor-tearoffs code. In constructor-tearoffs-enabled code,
4129+
// generic function instantiation is performed at assignability check
4130+
// sites.
4131+
// TODO(srawlins): Switch all resolution to use the latter method, in a
4132+
// breaking change release.
4133+
type = inferenceHelper.inferTearOff(node, propertyName, type,
4134+
contextType: contextType);
4135+
}
4136+
4137+
propertyName.setPseudoExpressionStaticType(type);
4138+
node.recordStaticType(type, resolver: this);
4139+
var replacement =
4140+
insertGenericFunctionInstantiation(node, contextType: contextType);
4141+
4142+
nullShortingTermination(node, rewrittenExpression: replacement);
4143+
_insertImplicitCallReference(replacement, contextType: contextType);
4144+
nullSafetyDeadCodeVerifier.verifyPropertyAccess(node);
4145+
}
4146+
41434147
/// Continues resolution of a [FunctionExpressionInvocation] that was created
41444148
/// from a rewritten [MethodInvocation]. The target function is already
41454149
/// resolved.

0 commit comments

Comments
 (0)