Skip to content

Fix two issues with typealiases in protocol extensions #20654

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 26, 2018
Merged

Fix two issues with typealiases in protocol extensions #20654

merged 1 commit into from
Nov 26, 2018

Conversation

jrose-apple
Copy link
Contributor

  • The GenericSignatureBuilder assumed it didn't have to look in protocol extensions to resolve member types.

  • Serialization was incorrectly filtering out such typealiases when trying to resolve a cross-module reference to one.

rdar://problem/46103190

@jrose-apple
Fix two issues with typealiases in protocol extensions
0677826 
- The GenericSignatureBuilder assumed it didn't have to look in
  protocol extensions to resolve member types.

- Serialization was incorrectly filtering out such typealiases when
  trying to resolve a cross-module reference to one.

rdar://problem/46103190
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test source compatibility

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test compiler performance

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 0677826

@jrose-apple
Copy link
Contributor Author

Unrelated failure (SR-9265).

@swift-ci Please test Linux

slavapestov
slavapestov reviewed Nov 17, 2018
View reviewed changes
lib/AST/GenericSignatureBuilder.cpp
auto flags = OptionSet<NominalTypeDecl::LookupDirectFlags>();
flags |= NominalTypeDecl::LookupDirectFlags::IgnoreNewExtensions;
for (auto member : proto->lookupDirect(name, flags)) {
for (auto member : proto->lookupDirect(name)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to see this worked without any fallout!

DougGregor
DougGregor approved these changes Nov 17, 2018
View reviewed changes
Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Great! The reasons why we did this must have gone away.

@swift-ci
Copy link
Contributor

Build comment file:

Summary for master full

Unexpected test results, excluded stats for Tagged, NonEmpty, ProcedureKitCloud, GRDB, ModelAssistant, Wordy

Regressions found (see below)

Debug-batch

debug-batch brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
name old new delta delta_pct
Frontend.NumInstructionsExecuted 21,229,941,934,031 21,178,916,158,198 -51,025,775,833 -0.24%
LLVM.NumLLVMBytesOutput 911,490,058 911,496,838 6,780 0.0%
time.swift-driver.wall 2243.4s 2225.6s -17.8s -0.79%

debug-batch detailed

Regressed (6)
name old new delta delta_pct
Driver.NumDriverPipePolls 298,351 304,248 5,897 1.98% ⛔
Driver.NumDriverPipeReads 337,127 343,380 6,253 1.85% ⛔
Sema.ExtendedNominalRequest 2,670,293 2,749,032 78,739 2.95% ⛔
Sema.NumDeclsDeserialized 30,737,239 31,163,357 426,118 1.39% ⛔
Sema.NumLazyGenericEnvironments 6,125,945 6,308,583 182,638 2.98% ⛔
Sema.NumTypesDeserialized 10,846,354 11,171,118 324,764 2.99% ⛔
Improved (3)
name old new delta delta_pct
Sema.NumUnloadedLazyIterableDeclContexts 3,513,782 3,430,841 -82,941 -2.36% ✅
Sema.OverriddenDeclsRequest 4,072,733 4,021,236 -51,497 -1.26% ✅
Sema.USRGenerationRequest 6,088,012 5,989,407 -98,605 -1.62% ✅
Unchanged (delta < 1.0% or delta < 100.0ms) (86)
name old new delta delta_pct
AST.NumASTBytesAllocated 45,088,088,985 44,808,818,857 -279,270,128 -0.62%
AST.NumDecls 65,849 65,849 0 0.0%
AST.NumDependencies 152,685 152,685 0 0.0%
AST.NumImportedExternalDefinitions 960,699 963,832 3,133 0.33%
AST.NumInfixOperators 24,640 24,640 0 0.0%
AST.NumLinkLibraries 0 0 0 0.0%
AST.NumLoadedModules 180,845 180,845 0 0.0%
AST.NumLocalTypeDecls 113 113 0 0.0%
AST.NumObjCMethods 12,528 12,528 0 0.0%
AST.NumPostfixOperators 13 13 0 0.0%
AST.NumPrecedenceGroups 12,687 12,687 0 0.0%
AST.NumPrefixOperators 71 71 0 0.0%
AST.NumReferencedDynamicNames 101 101 0 0.0%
AST.NumReferencedMemberNames 3,046,645 3,046,645 0 0.0%
AST.NumReferencedTopLevelNames 210,981 210,981 0 0.0%
AST.NumSourceBuffers 288,503 288,503 0 0.0%
AST.NumSourceLines 2,107,547 2,107,547 0 0.0%
AST.NumSourceLinesPerSecond 1,182,964 1,189,831 6,867 0.58%
AST.NumTotalClangImportedEntities 3,563,141 3,560,286 -2,855 -0.08%
AST.NumUsedConformances 184,818 184,818 0 0.0%
Driver.ChildrenMaxRSS 68,624,693,248 68,381,003,776 -243,689,472 -0.36%
Driver.DriverDepCascadingDynamic 0 0 0 0.0%
Driver.DriverDepCascadingExternal 0 0 0 0.0%
Driver.DriverDepCascadingMember 0 0 0 0.0%
Driver.DriverDepCascadingNominal 0 0 0 0.0%
Driver.DriverDepCascadingTopLevel 0 0 0 0.0%
Driver.DriverDepDynamic 0 0 0 0.0%
Driver.DriverDepExternal 0 0 0 0.0%
Driver.DriverDepMember 0 0 0 0.0%
Driver.DriverDepNominal 0 0 0 0.0%
Driver.DriverDepTopLevel 0 0 0 0.0%
Driver.NumDriverJobsRun 13,529 13,529 0 0.0%
Driver.NumDriverJobsSkipped 0 0 0 0.0%
Driver.NumProcessFailures 0 0 0 0.0%
Frontend.MaxMallocUsage 351,781,744,256 351,133,734,528 -648,009,728 -0.18%
Frontend.NumInstructionsExecuted 21,229,941,934,031 21,178,916,158,198 -51,025,775,833 -0.24%
Frontend.NumProcessFailures 0 0 0 0.0%
IRModule.NumIRAliases 92,680 92,680 0 0.0%
IRModule.NumIRBasicBlocks 3,585,260 3,585,269 9 0.0%
IRModule.NumIRComdatSymbols 0 0 0 0.0%
IRModule.NumIRFunctions 1,631,420 1,631,431 11 0.0%
IRModule.NumIRGlobals 1,856,423 1,856,450 27 0.0%
IRModule.NumIRIFuncs 0 0 0 0.0%
IRModule.NumIRInsts 42,053,732 42,053,903 171 0.0%
IRModule.NumIRNamedMetaData 66,177 66,177 0 0.0%
IRModule.NumIRValueSymbols 3,118,357 3,118,386 29 0.0%
LLVM.NumLLVMBytesOutput 911,490,058 911,496,838 6,780 0.0%
Parse.NumFunctionsParsed 2,170,339 2,170,339 0 0.0%
Parse.NumIterableDeclContextParsed 865,730 865,730 0 0.0%
SILModule.NumSILGenDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILGenFunctions 1,286,702 1,286,963 261 0.02%
SILModule.NumSILGenGlobalVariables 26,821 26,821 0 0.0%
SILModule.NumSILGenVtables 10,171 10,171 0 0.0%
SILModule.NumSILGenWitnessTables 37,361 37,361 0 0.0%
SILModule.NumSILOptDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILOptFunctions 1,170,111 1,170,120 9 0.0%
SILModule.NumSILOptGlobalVariables 27,550 27,550 0 0.0%
SILModule.NumSILOptVtables 16,355 16,355 0 0.0%
SILModule.NumSILOptWitnessTables 72,508 72,508 0 0.0%
Sema.AccessLevelRequest 1,911,303 1,909,338 -1,965 -0.1%
Sema.DefaultAndMaxAccessLevelRequest 45,847 45,845 -2 -0.0%
Sema.EnumRawTypeRequest 13,205 13,205 0 0.0%
Sema.InheritedDeclsReferencedRequest 84,557,121 84,433,203 -123,918 -0.15%
Sema.InheritedTypeRequest 441,373 441,368 -5 -0.0%
Sema.IsDynamicRequest 1,535,740 1,535,740 0 0.0%
Sema.IsObjCRequest 1,332,824 1,331,993 -831 -0.06%
Sema.NamedLazyMemberLoadFailureCount 18,194 18,195 1 0.01%
Sema.NamedLazyMemberLoadSuccessCount 13,400,437 13,442,934 42,497 0.32%
Sema.NominalTypeLookupDirectCount 25,013,985 25,000,218 -13,767 -0.06%
Sema.NumConformancesDeserialized 3,867,724 3,850,107 -17,617 -0.46%
Sema.NumConstraintScopes 13,203,321 13,203,509 188 0.0%
Sema.NumConstraintsConsideredForEdgeContraction 30,907,956 30,907,677 -279 -0.0%
Sema.NumDeclsValidated 1,645,646 1,645,645 -1 -0.0%
Sema.NumFunctionsTypechecked 920,503 923,331 2,828 0.31%
Sema.NumGenericSignatureBuilders 871,621 869,998 -1,623 -0.19%
Sema.NumLazyGenericEnvironmentsLoaded 164,566 165,169 603 0.37%
Sema.NumLazyIterableDeclContexts 4,934,365 4,958,625 24,260 0.49%
Sema.NumLeafScopes 9,017,212 9,017,690 478 0.01%
Sema.NumTypesValidated 1,078,137 1,078,137 0 0.0%
Sema.RequirementRequest 54,623 54,622 -1 -0.0%
Sema.SelfBoundsFromWhereClauseRequest 50,283,180 50,034,417 -248,763 -0.49%
Sema.SetterAccessLevelRequest 108,626 108,626 0 0.0%
Sema.SuperclassDeclRequest 66,608,979 66,607,576 -1,403 -0.0%
Sema.SuperclassTypeRequest 30,270 30,270 0 0.0%
Sema.TypeDeclsFromWhereClauseRequest 26,261 26,259 -2 -0.01%
Sema.UnderlyingTypeDeclsReferencedRequest 2,295,859 2,295,788 -71 -0.0%

Release

release brief

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
name old new delta delta_pct
Frontend.NumInstructionsExecuted 24,043,403,970,540 24,060,940,620,180 17,536,649,640 0.07%
LLVM.NumLLVMBytesOutput 811,041,134 811,042,570 1,436 0.0%
time.swift-driver.wall 4452.9s 4454.3s 1.5s 0.03%

release detailed

Regressed (3)
name old new delta delta_pct
Sema.NumDeclsDeserialized 3,924,307 4,103,247 178,940 4.56% ⛔
Sema.NumLazyGenericEnvironments 790,967 852,470 61,503 7.78% ⛔
Sema.NumTypesDeserialized 2,120,240 2,177,994 57,754 2.72% ⛔
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (20)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 178,485 178,485 0 0.0%
AST.NumLoadedModules 11,404 11,404 0 0.0%
AST.NumTotalClangImportedEntities 602,601 602,601 0 0.0%
AST.NumUsedConformances 185,542 185,542 0 0.0%
IRModule.NumIRBasicBlocks 2,983,985 2,983,985 0 0.0%
IRModule.NumIRFunctions 1,338,470 1,338,470 0 0.0%
IRModule.NumIRGlobals 1,465,628 1,465,628 0 0.0%
IRModule.NumIRInsts 28,658,148 28,658,148 0 0.0%
IRModule.NumIRValueSymbols 2,620,088 2,620,088 0 0.0%
LLVM.NumLLVMBytesOutput 811,041,134 811,042,570 1,436 0.0%
SILModule.NumSILGenFunctions 561,121 561,121 0 0.0%
SILModule.NumSILOptFunctions 692,645 692,645 0 0.0%
Sema.NumConformancesDeserialized 1,563,562 1,563,562 0 0.0%
Sema.NumConstraintScopes 11,683,925 11,683,925 0 0.0%
Sema.NumDeclsValidated 855,460 855,460 0 0.0%
Sema.NumFunctionsTypechecked 450,388 450,388 0 0.0%
Sema.NumGenericSignatureBuilders 144,092 144,092 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 15,568 15,568 0 0.0%
Sema.NumLazyIterableDeclContexts 532,332 537,204 4,872 0.92%
Sema.NumTypesValidated 401,820 401,820 0 0.0%

@jrose-apple
Copy link
Contributor Author

I'm going to say we take the hit on these counters. Overall time didn't noticeably change.

@swift-ci Please smoke test

@jrose-apple jrose-apple merged commit 1f25ea5 into swiftlang:master Nov 26, 2018
@jrose-apple jrose-apple deleted the can-i-get-an-extension branch November 26, 2018 19:31
jrose-apple added a commit to jrose-apple/swift that referenced this pull request Jan 16, 2019
@jrose-apple
Fix two issues with typealiases in protocol extensions (swiftlang#20654)
0cfded0 
- The GenericSignatureBuilder assumed it didn't have to look in
  protocol extensions to resolve member types.

- Serialization was incorrectly filtering out such typealiases when
  trying to resolve a cross-module reference to one.

rdar://problem/46103190
(cherry picked from commit 1f25ea5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov approved these changes

@DougGregor DougGregor DougGregor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jrose-apple @swift-ci @slavapestov @DougGregor