Skip to content

Commit 45a088f

Browse files
scheglovCommit Queue
authored andcommitted
Fine. Include even private elements into manifests.
While only some elements can be referenced in constants in valid code, still other elements might somehow affect public elements. I think we will get more robust implementation, if we put all elements, including private into manifests. I don't want to guess: can this affect constants, or not? The performance different is about 6% on computing library manifests on analyzer/ + analysis_server/ + linter/. And computing initial set of manifests itself is 5% of total work, so it is about 0.3% total slowdown. Also adds static getters and methods to make it more representative. Change-Id: I8994b8f84befbce695fd48fc657e4350109f6cfe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/418720 Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Paul Berry <[email protected]>
1 parent f7edb90 commit 45a088f

File tree

2 files changed

+447
-38
lines changed

2 files changed

+447
-38
lines changed

pkg/analyzer/lib/src/fine/library_manifest.dart

Lines changed: 98 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ class LibraryManifestBuilder {
124124
element.typeParameters2,
125125
(typeParameters) {
126126
classItem.members.clear();
127-
_addInterfaceConstructors(
127+
_addStaticExecutables(
128128
encodingContext: encodingContext,
129-
interfaceElement: element,
129+
instanceElement: element,
130130
interfaceItem: classItem,
131131
);
132132
_addInstanceExecutables(
@@ -144,12 +144,6 @@ class LibraryManifestBuilder {
144144
required Name nameObj,
145145
required ExecutableElement2OrMember element,
146146
}) {
147-
// Skip private names, cannot be used outside this library.
148-
// TODO(scheglov): reconsider for static that can be reference from const
149-
if (!nameObj.isPublic) {
150-
return;
151-
}
152-
153147
var lookupName = nameObj.name.asLookupName;
154148

155149
switch (element) {
@@ -227,10 +221,6 @@ class LibraryManifestBuilder {
227221
required ConstructorElementImpl2 element,
228222
required LookupName lookupName,
229223
}) {
230-
// Constructors can be referenced by constants.
231-
// So, we include all of them, including private.
232-
// This is a general rule for "static" elements.
233-
234224
var item = _getOrBuildElementItem(element, () {
235225
return InterfaceConstructorItem.fromElement(
236226
name: lookupName,
@@ -242,24 +232,6 @@ class LibraryManifestBuilder {
242232
interfaceItem.members[lookupName] = item;
243233
}
244234

245-
void _addInterfaceConstructors({
246-
required EncodeContext encodingContext,
247-
required InterfaceElementImpl2 interfaceElement,
248-
required ClassItem interfaceItem,
249-
}) {
250-
for (var constructor in interfaceElement.constructors2) {
251-
var lookupName = constructor.name3?.asLookupName;
252-
if (lookupName != null) {
253-
_addInterfaceConstructor(
254-
encodingContext: encodingContext,
255-
interfaceItem: interfaceItem,
256-
element: constructor,
257-
lookupName: lookupName,
258-
);
259-
}
260-
}
261-
}
262-
263235
void _addReExports() {
264236
for (var libraryElement in libraryElements) {
265237
var libraryUri = libraryElement.uri;
@@ -295,6 +267,54 @@ class LibraryManifestBuilder {
295267
}
296268
}
297269

270+
void _addStaticExecutables({
271+
required EncodeContext encodingContext,
272+
required InstanceElementImpl2 instanceElement,
273+
required ClassItem interfaceItem,
274+
}) {
275+
if (instanceElement is InterfaceElementImpl2) {
276+
for (var constructor in instanceElement.constructors2) {
277+
var lookupName = constructor.name3?.asLookupName;
278+
if (lookupName != null) {
279+
_addInterfaceConstructor(
280+
encodingContext: encodingContext,
281+
interfaceItem: interfaceItem,
282+
element: constructor,
283+
lookupName: lookupName,
284+
);
285+
}
286+
}
287+
}
288+
289+
for (var getter in instanceElement.getters2) {
290+
if (getter.isStatic) {
291+
var lookupName = getter.name3?.asLookupName;
292+
if (lookupName != null) {
293+
_addInstanceGetter(
294+
encodingContext: encodingContext,
295+
instanceItem: interfaceItem,
296+
element: getter,
297+
lookupName: lookupName,
298+
);
299+
}
300+
}
301+
}
302+
303+
for (var method in instanceElement.methods2) {
304+
if (method.isStatic) {
305+
var lookupName = method.name3?.asLookupName;
306+
if (lookupName != null) {
307+
_addInstanceMethod(
308+
encodingContext: encodingContext,
309+
instanceItem: interfaceItem,
310+
element: method,
311+
lookupName: lookupName,
312+
);
313+
}
314+
}
315+
}
316+
}
317+
298318
void _addTopLevelFunction({
299319
required EncodeContext encodingContext,
300320
required Map<LookupName, TopLevelItem> newItems,
@@ -612,6 +632,11 @@ class _LibraryMatch {
612632
interfaceElement: element,
613633
item: item,
614634
);
635+
_matchStaticExecutables(
636+
matchContext: matchContext,
637+
element: element,
638+
item: item,
639+
);
615640

616641
_matchInstanceExecutables(
617642
matchContext: matchContext,
@@ -625,15 +650,10 @@ class _LibraryMatch {
625650
bool _matchInstanceExecutable({
626651
required MatchContext interfaceMatchContext,
627652
required Map<LookupName, InstanceMemberItem> members,
628-
required Name nameObj,
653+
required LookupName lookupName,
629654
required ExecutableElement2 executable,
630655
}) {
631-
// Skip private names, cannot be used outside this library.
632-
if (!nameObj.isPublic) {
633-
return true;
634-
}
635-
636-
var item = members[nameObj.name.asLookupName];
656+
var item = members[lookupName];
637657

638658
switch (executable) {
639659
case GetterElement2OrMember():
@@ -685,7 +705,7 @@ class _LibraryMatch {
685705
if (!_matchInstanceExecutable(
686706
interfaceMatchContext: matchContext,
687707
members: item.members,
688-
nameObj: nameObj,
708+
lookupName: nameObj.name.asLookupName,
689709
executable: executable,
690710
)) {
691711
structureMismatched.add(executable);
@@ -735,6 +755,46 @@ class _LibraryMatch {
735755
}
736756
}
737757

758+
void _matchStaticExecutables({
759+
required MatchContext matchContext,
760+
required ClassElementImpl2 element,
761+
required ClassItem item,
762+
}) {
763+
// TODO(scheglov): it looks that we repeat iterations
764+
// We do it for structural matching, and then for adding.
765+
for (var getters in element.getters2) {
766+
if (getters.isStatic) {
767+
var lookupName = getters.name3?.asLookupName;
768+
if (lookupName != null) {
769+
if (!_matchInstanceExecutable(
770+
interfaceMatchContext: matchContext,
771+
members: item.members,
772+
lookupName: lookupName,
773+
executable: getters,
774+
)) {
775+
structureMismatched.add(getters);
776+
}
777+
}
778+
}
779+
}
780+
781+
for (var method in element.methods2) {
782+
if (method.isStatic) {
783+
var lookupName = method.name3?.asLookupName;
784+
if (lookupName != null) {
785+
if (!_matchInstanceExecutable(
786+
interfaceMatchContext: matchContext,
787+
members: item.members,
788+
lookupName: lookupName,
789+
executable: method,
790+
)) {
791+
structureMismatched.add(method);
792+
}
793+
}
794+
}
795+
}
796+
}
797+
738798
bool _matchTopFunction({
739799
required LookupName? name,
740800
required TopLevelFunctionElementImpl element,

0 commit comments

Comments
 (0)