Issue 49766. Fix memory leak with enum constants and Expando.

Instead of using Expando, we store this data in an object that
lives as long, as we need it to compute diagnostics.

Bug: https://github.com/dart-lang/sdk/issues/49766
Change-Id: I0c2a2e15f9e1ea92a415bae6e13bbbd373dc613b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255825
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Konstantin Shcheglov 2022-08-19 20:47:55 +00:00 committed by Commit Bot
parent e0cebd838e
commit e04a9e80ce
9 changed files with 111 additions and 37 deletions

View file

@ -230,10 +230,20 @@ class LibraryAnalyzer {
/// Compute [_constants] in all units.
void _computeConstants(Iterable<CompilationUnitImpl> units) {
final configuration = ConstantEvaluationConfiguration();
var constants = [
for (var unit in units) ..._findConstants(unit),
for (var unit in units)
..._findConstants(
unit: unit,
configuration: configuration,
),
];
computeConstants(_declaredVariables, constants, _libraryElement.featureSet);
computeConstants(
declaredVariables: _declaredVariables,
constants: constants,
featureSet: _libraryElement.featureSet,
configuration: configuration,
);
}
/// Compute diagnostics in [units], including errors and warnings, hints,
@ -497,6 +507,24 @@ class LibraryAnalyzer {
return errors.where((AnalysisError e) => !isIgnored(e)).toList();
}
/// Find constants in [unit] to compute.
List<ConstantEvaluationTarget> _findConstants({
required CompilationUnit unit,
required ConstantEvaluationConfiguration configuration,
}) {
ConstantFinder constantFinder = ConstantFinder(
configuration: configuration,
);
unit.accept(constantFinder);
var dependenciesFinder = ConstantExpressionsDependenciesFinder();
unit.accept(dependenciesFinder);
return [
...constantFinder.constantsToCompute,
...dependenciesFinder.dependencies,
];
}
RecordingErrorListener _getErrorListener(FileState file) =>
_errorListeners.putIfAbsent(file, () => RecordingErrorListener());
@ -1001,19 +1029,6 @@ class LibraryAnalyzer {
CompileTimeErrorCode.DUPLICATE_PART, partUri, [partSource.uri]);
}
}
/// Find constants in [unit] to compute.
static List<ConstantEvaluationTarget> _findConstants(CompilationUnit unit) {
ConstantFinder constantFinder = ConstantFinder();
unit.accept(constantFinder);
var dependenciesFinder = ConstantExpressionsDependenciesFinder();
unit.accept(dependenciesFinder);
return [
...constantFinder.constantsToCompute,
...dependenciesFinder.dependencies,
];
}
}
/// Analysis result for single file.

View file

@ -10,9 +10,17 @@ import 'package:analyzer/src/dart/constant/evaluation.dart';
import 'package:analyzer/src/dart/element/element.dart';
/// Compute values of the given [constants] with correct ordering.
void computeConstants(DeclaredVariables declaredVariables,
List<ConstantEvaluationTarget> constants, FeatureSet featureSet) {
var walker = _ConstantWalker(declaredVariables, featureSet);
void computeConstants({
required DeclaredVariables declaredVariables,
required List<ConstantEvaluationTarget> constants,
required FeatureSet featureSet,
required ConstantEvaluationConfiguration configuration,
}) {
var walker = _ConstantWalker(
declaredVariables: declaredVariables,
featureSet: featureSet,
configuration: configuration,
);
for (var constant in constants) {
walker.walk(walker._getNode(constant));
@ -39,9 +47,14 @@ class _ConstantNode extends graph.Node<_ConstantNode> {
class _ConstantWalker extends graph.DependencyWalker<_ConstantNode> {
final DeclaredVariables declaredVariables;
final FeatureSet featureSet;
final ConstantEvaluationConfiguration configuration;
final Map<ConstantEvaluationTarget, _ConstantNode> nodeMap = {};
_ConstantWalker(this.declaredVariables, this.featureSet);
_ConstantWalker({
required this.declaredVariables,
required this.featureSet,
required this.configuration,
});
@override
void evaluate(_ConstantNode node) {
@ -71,6 +84,7 @@ class _ConstantWalker extends graph.DependencyWalker<_ConstantNode> {
return ConstantEvaluationEngine(
declaredVariables: declaredVariables,
isNonNullableByDefault: featureSet.isEnabled(Feature.non_nullable),
configuration: configuration,
);
}

View file

@ -71,6 +71,7 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
declaredVariables: declaredVariables,
isNonNullableByDefault:
_currentLibrary.featureSet.isEnabled(Feature.non_nullable),
configuration: ConstantEvaluationConfiguration(),
);
@override

View file

@ -29,11 +29,24 @@ import 'package:analyzer/src/generated/constant.dart';
import 'package:analyzer/src/generated/engine.dart';
import 'package:analyzer/src/task/api/model.dart';
/// During evaluation of enum constants we might need to report an error
/// that is associated with the [InstanceCreationExpression], but this
/// expression is synthetic. Instead, we remember the corresponding
/// [EnumConstantDeclaration] and report the error on it.
final enumConstantErrorNodes = Expando<EnumConstantDeclaration>();
class ConstantEvaluationConfiguration {
/// During evaluation of enum constants we might need to report an error
/// that is associated with the [InstanceCreationExpression], but this
/// expression is synthetic. Instead, we remember the corresponding
/// [EnumConstantDeclaration] and report the error on it.
final Map<Expression, EnumConstantDeclaration> _enumConstants = {};
void addEnumConstant({
required EnumConstantDeclaration declaration,
required Expression initializer,
}) {
_enumConstants[initializer] = declaration;
}
AstNode errorNode(AstNode node) {
return _enumConstants[node] ?? node;
}
}
/// Helper class encapsulating the methods for evaluating constants and
/// constant instance creation expressions.
@ -44,6 +57,8 @@ class ConstantEvaluationEngine {
/// Whether the `non-nullable` feature is enabled.
final bool _isNonNullableByDefault;
final ConstantEvaluationConfiguration configuration;
/// Initialize a newly created [ConstantEvaluationEngine].
///
/// [declaredVariables] is the set of variables declared on the command
@ -51,6 +66,7 @@ class ConstantEvaluationEngine {
ConstantEvaluationEngine({
required DeclaredVariables declaredVariables,
required bool isNonNullableByDefault,
required this.configuration,
}) : _declaredVariables = declaredVariables,
_isNonNullableByDefault = isNonNullableByDefault;
@ -2599,7 +2615,7 @@ class _InstanceCreationEvaluator {
declaredVariables,
errorReporter,
library,
enumConstantErrorNodes[node] ?? node,
evaluationEngine.configuration.errorNode(node),
constructor,
typeArguments,
namedNodes: namedNodes,

View file

@ -80,6 +80,8 @@ class ConstantExpressionsDependenciesFinder extends RecursiveAstVisitor {
/// constructors, constant constructor invocations, and annotations found in
/// those compilation units.
class ConstantFinder extends RecursiveAstVisitor<void> {
final ConstantEvaluationConfiguration configuration;
/// The elements and AST nodes whose constant values need to be computed.
List<ConstantEvaluationTarget> constantsToCompute =
<ConstantEvaluationTarget>[];
@ -88,6 +90,10 @@ class ConstantFinder extends RecursiveAstVisitor<void> {
/// treated as "const".
bool treatFinalInstanceVarAsConst = false;
ConstantFinder({
required this.configuration,
});
@override
void visitAnnotation(Annotation node) {
super.visitAnnotation(node);
@ -148,8 +154,11 @@ class ConstantFinder extends RecursiveAstVisitor<void> {
var element = node.declaredElement2 as ConstFieldElementImpl;
constantsToCompute.add(element);
var constantInitializer = element.constantInitializer!;
enumConstantErrorNodes[constantInitializer] = node;
final initializer = element.constantInitializer!;
configuration.addEnumConstant(
declaration: node,
initializer: initializer,
);
}
@override

View file

@ -1594,7 +1594,12 @@ class ConstructorElementImpl extends ExecutableElementImpl
/// of formal parameters, are evaluated.
void computeConstantDependencies() {
if (!isConstantEvaluated) {
computeConstants(context.declaredVariables, [this], library.featureSet);
computeConstants(
declaredVariables: context.declaredVariables,
constants: [this],
featureSet: library.featureSet,
configuration: ConstantEvaluationConfiguration(),
);
}
}
}
@ -1683,7 +1688,12 @@ mixin ConstVariableElement implements ElementImpl, ConstantEvaluationTarget {
'[reference: $reference]',
);
}
computeConstants(context.declaredVariables, [this], library.featureSet);
computeConstants(
declaredVariables: context.declaredVariables,
constants: [this],
featureSet: library.featureSet,
configuration: ConstantEvaluationConfiguration(),
);
}
return evaluationResult?.value;
}
@ -2159,8 +2169,12 @@ class ElementAnnotationImpl implements ElementAnnotation {
@override
DartObject? computeConstantValue() {
if (evaluationResult == null) {
computeConstants(context.declaredVariables, [this],
compilationUnit.library.featureSet);
computeConstants(
declaredVariables: context.declaredVariables,
constants: [this],
featureSet: compilationUnit.library.featureSet,
configuration: ConstantEvaluationConfiguration(),
);
}
return evaluationResult?.value;
}

View file

@ -113,6 +113,7 @@ class ConstantEvaluator {
declaredVariables: DeclaredVariables(),
isNonNullableByDefault:
_library.featureSet.isEnabled(Feature.non_nullable),
configuration: ConstantEvaluationConfiguration(),
),
_library,
errorReporter));

View file

@ -370,6 +370,7 @@ class LinterContextImpl implements LinterContext {
var evaluationEngine = ConstantEvaluationEngine(
declaredVariables: declaredVariables,
isNonNullableByDefault: isEnabled(Feature.non_nullable),
configuration: ConstantEvaluationConfiguration(),
);
var dependencies = <ConstantEvaluationTarget>[];
@ -378,9 +379,10 @@ class LinterContextImpl implements LinterContext {
);
computeConstants(
declaredVariables,
dependencies,
libraryElement.featureSet,
declaredVariables: declaredVariables,
constants: dependencies,
featureSet: libraryElement.featureSet,
configuration: ConstantEvaluationConfiguration(),
);
var visitor = ConstantVisitor(
@ -484,9 +486,10 @@ class LinterContextImpl implements LinterContext {
var dependenciesFinder = ConstantExpressionsDependenciesFinder();
node.accept(dependenciesFinder);
computeConstants(
declaredVariables,
dependenciesFinder.dependencies.toList(),
libraryElement.featureSet,
declaredVariables: declaredVariables,
constants: dependenciesFinder.dependencies.toList(),
featureSet: libraryElement.featureSet,
configuration: ConstantEvaluationConfiguration(),
);
var listener = _ConstantAnalysisErrorListener();

View file

@ -2020,6 +2020,7 @@ class ConstantVisitorTestSupport extends PubPackageResolutionTest {
declaredVariables: DeclaredVariables.fromMap(declaredVariables),
isNonNullableByDefault:
unit.featureSet.isEnabled(Feature.non_nullable),
configuration: ConstantEvaluationConfiguration(),
),
this.result.libraryElement as LibraryElementImpl,
errorReporter,