Skip to content
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.

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

Implementation-only import checking for types used in decls #23722

Merged
merged 4 commits into from Apr 8, 2019

Conversation

Projects
None yet
5 participants
@jrose-apple
Copy link
Member

jrose-apple commented Apr 2, 2019

Builds on John's #23702 to handle checking of types in decls, the same way we do for access control. Contains more duplicated code than I like, but appears to work, and may provide refactoring hints in the future.

Part of rdar://problem/48991061

@jrose-apple jrose-apple requested review from slavapestov and rjmccall Apr 2, 2019

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 2, 2019

TODO in this PR: test non-public stored properties in frozen structs + classes. Done!

TODO in general as part of validation: conformances?? WIP in #23808

@jrose-apple jrose-apple requested a review from brentdax Apr 2, 2019

@jrose-apple jrose-apple force-pushed the jrose-apple:type-disadvantage branch from 33500ef to 72b9218 Apr 2, 2019

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 2, 2019

@swift-ci Please test compiler performance

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 2, 2019

From the failed build. Looks like we're probably okay, but there was a small slowdown.

Summary for master full

Unexpected test results, excluded stats for Deferred, Tagged, ProcedureKit, Wordy, CoreStore

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 27,565,449,278,771 27,752,682,384,129 187,233,105,358 0.68%
LLVM.NumLLVMBytesOutput 972,294,632 972,295,094 462 0.0%
time.swift-driver.wall 2641.7s 2650.7s 9.0s 0.34%

debug-batch detailed

Regressed (3)
name old new delta delta_pct
AST.NumASTBytesAllocated 67,811,074,426 68,614,261,736 803,187,310 1.18% ⛔️
Sema.OverriddenDeclsRequest 7,586,710 7,719,656 132,946 1.75% ⛔️
Sema.USRGenerationRequest 12,707,012 12,971,812 264,800 2.08% ⛔️
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (97)
name old new delta delta_pct
AST.NumDecls 80,786 80,786 0 0.0%
AST.NumDependencies 205,072 205,062 -10 -0.0%
AST.NumImportedExternalDefinitions 1,092,562 1,092,562 0 0.0%
AST.NumInfixOperators 31,080 31,080 0 0.0%
AST.NumLinkLibraries 0 0 0 0.0%
AST.NumLoadedModules 244,881 244,881 0 0.0%
AST.NumLocalTypeDecls 108 108 0 0.0%
AST.NumObjCMethods 9,724 9,724 0 0.0%
AST.NumPostfixOperators 18 18 0 0.0%
AST.NumPrecedenceGroups 14,514 14,514 0 0.0%
AST.NumPrefixOperators 70 70 0 0.0%
AST.NumReferencedDynamicNames 92 92 0 0.0%
AST.NumReferencedMemberNames 3,511,542 3,511,542 0 0.0%
AST.NumReferencedTopLevelNames 271,022 271,022 0 0.0%
AST.NumSourceBuffers 331,664 331,664 0 0.0%
AST.NumSourceLines 2,432,547 2,432,547 0 0.0%
AST.NumSourceLinesPerSecond 2,060,772 2,068,449 7,677 0.37%
AST.NumTotalClangImportedEntities 4,136,677 4,149,402 12,725 0.31%
AST.NumUsedConformances 226,385 226,385 0 0.0%
Driver.ChildrenMaxRSS 106,906,710,016 107,152,728,064 246,018,048 0.23%
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 16,205 16,205 0 0.0%
Driver.NumDriverJobsSkipped 0 0 0 0.0%
Driver.NumDriverPipePolls 161,062 161,736 674 0.42%
Driver.NumDriverPipeReads 177,166 177,685 519 0.29%
Driver.NumProcessFailures 0 0 0 0.0%
Frontend.MaxMallocUsage 692,492,518,216 695,899,811,632 3,407,293,416 0.49%
Frontend.NumInstructionsExecuted 27,565,449,278,771 27,752,682,384,129 187,233,105,358 0.68%
Frontend.NumProcessFailures 0 0 0 0.0%
IRModule.NumIRAliases 105,540 105,540 0 0.0%
IRModule.NumIRBasicBlocks 3,849,136 3,849,136 0 0.0%
IRModule.NumIRComdatSymbols 0 0 0 0.0%
IRModule.NumIRFunctions 1,804,555 1,804,555 0 0.0%
IRModule.NumIRGlobals 1,857,056 1,857,056 0 0.0%
IRModule.NumIRIFuncs 0 0 0 0.0%
IRModule.NumIRInsts 47,456,421 47,456,421 0 0.0%
IRModule.NumIRNamedMetaData 78,402 78,402 0 0.0%
IRModule.NumIRValueSymbols 3,302,061 3,302,061 0 0.0%
LLVM.NumLLVMBytesOutput 972,294,632 972,295,094 462 0.0%
Parse.NumFunctionsParsed 142,585 142,585 0 0.0%
Parse.NumIterableDeclContextParsed 1,004,447 1,004,447 0 0.0%
SILModule.NumSILGenDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILGenFunctions 932,961 932,961 0 0.0%
SILModule.NumSILGenGlobalVariables 37,189 37,189 0 0.0%
SILModule.NumSILGenVtables 10,694 10,694 0 0.0%
SILModule.NumSILGenWitnessTables 39,362 39,362 0 0.0%
SILModule.NumSILOptDefaultWitnessTables 0 0 0 0.0%
SILModule.NumSILOptFunctions 1,326,972 1,326,972 0 0.0%
SILModule.NumSILOptGlobalVariables 37,947 37,947 0 0.0%
SILModule.NumSILOptVtables 17,207 17,207 0 0.0%
SILModule.NumSILOptWitnessTables 86,613 86,613 0 0.0%
Sema.AccessLevelRequest 2,338,362 2,342,152 3,790 0.16%
Sema.CustomAttrNominalRequest 0 0 0 0.0%
Sema.DefaultAndMaxAccessLevelRequest 54,592 54,597 5 0.01%
Sema.DefaultTypeRequest 318,780 318,780 0 0.0%
Sema.EnumRawTypeRequest 17,024 17,024 0 0.0%
Sema.ExtendedNominalRequest 3,743,430 3,751,599 8,169 0.22%
Sema.InheritedDeclsReferencedRequest 4,300,492 4,318,497 18,005 0.42%
Sema.InheritedTypeRequest 528,692 528,999 307 0.06%
Sema.IsDynamicRequest 1,828,610 1,828,610 0 0.0%
Sema.IsObjCRequest 1,565,869 1,567,151 1,282 0.08%
Sema.MangleLocalTypeDeclRequest 216 216 0 0.0%
Sema.NamedLazyMemberLoadFailureCount 18,930 18,974 44 0.23%
Sema.NamedLazyMemberLoadSuccessCount 17,460,151 17,455,771 -4,380 -0.03%
Sema.NominalTypeLookupDirectCount 29,220,636 29,274,598 53,962 0.18%
Sema.NumConformancesDeserialized 6,559,270 6,606,959 47,689 0.73%
Sema.NumConstraintScopes 15,055,397 15,061,743 6,346 0.04%
Sema.NumConstraintsConsideredForEdgeContraction 41,336,518 41,337,829 1,311 0.0%
Sema.NumDeclsDeserialized 48,193,494 48,555,033 361,539 0.75%
Sema.NumDeclsFinalized 1,644,178 1,644,178 0 0.0%
Sema.NumDeclsTypechecked 895,635 895,635 0 0.0%
Sema.NumDeclsValidated 1,945,079 1,945,079 0 0.0%
Sema.NumFunctionsTypechecked 949,206 949,206 0 0.0%
Sema.NumGenericSignatureBuilders 1,148,631 1,152,503 3,872 0.34%
Sema.NumLazyGenericEnvironments 9,727,871 9,775,858 47,987 0.49%
Sema.NumLazyGenericEnvironmentsLoaded 195,541 195,625 84 0.04%
Sema.NumLazyIterableDeclContexts 6,718,413 6,729,733 11,320 0.17%
Sema.NumLeafScopes 9,805,328 9,810,714 5,386 0.05%
Sema.NumTypesDeserialized 16,175,319 16,237,748 62,429 0.39%
Sema.NumTypesValidated 1,316,479 1,316,479 0 0.0%
Sema.NumUnloadedLazyIterableDeclContexts 4,407,042 4,398,594 -8,448 -0.19%
Sema.RequirementRequest 62,183 62,283 100 0.16%
Sema.SelfBoundsFromWhereClauseRequest 6,263,914 6,289,697 25,783 0.41%
Sema.SetterAccessLevelRequest 137,533 137,533 0 0.0%
Sema.SuperclassDeclRequest 66,990 67,107 117 0.17%
Sema.SuperclassTypeRequest 30,796 30,796 0 0.0%
Sema.TypeDeclsFromWhereClauseRequest 29,940 29,945 5 0.02%
Sema.UnderlyingTypeDeclsReferencedRequest 146,137 146,823 686 0.47%

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 25,587,848,415,902 25,582,356,306,260 -5,492,109,642 -0.02%
LLVM.NumLLVMBytesOutput 779,848,654 779,848,162 -492 -0.0%
time.swift-driver.wall 4598.0s 4604.4s 6.4s 0.14%

release detailed

Regressed (0)
name old new delta delta_pct
Improved (0)
name old new delta delta_pct
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
name old new delta delta_pct
AST.NumImportedExternalDefinitions 216,559 216,559 0 0.0%
AST.NumLoadedModules 16,492 16,492 0 0.0%
AST.NumTotalClangImportedEntities 736,667 736,667 0 0.0%
AST.NumUsedConformances 227,331 227,331 0 0.0%
IRModule.NumIRBasicBlocks 3,234,122 3,234,122 0 0.0%
IRModule.NumIRFunctions 1,486,895 1,486,895 0 0.0%
IRModule.NumIRGlobals 1,622,639 1,622,639 0 0.0%
IRModule.NumIRInsts 29,381,150 29,381,150 0 0.0%
IRModule.NumIRValueSymbols 2,896,376 2,896,376 0 0.0%
LLVM.NumLLVMBytesOutput 779,848,654 779,848,162 -492 -0.0%
SILModule.NumSILGenFunctions 653,152 653,152 0 0.0%
SILModule.NumSILOptFunctions 889,962 889,962 0 0.0%
Sema.NumConformancesDeserialized 2,233,184 2,233,184 0 0.0%
Sema.NumConstraintScopes 13,401,956 13,401,956 0 0.0%
Sema.NumDeclsDeserialized 6,020,091 6,020,091 0 0.0%
Sema.NumDeclsValidated 1,038,655 1,038,655 0 0.0%
Sema.NumFunctionsTypechecked 428,737 428,737 0 0.0%
Sema.NumGenericSignatureBuilders 190,265 190,265 0 0.0%
Sema.NumLazyGenericEnvironments 1,244,822 1,244,822 0 0.0%
Sema.NumLazyGenericEnvironmentsLoaded 21,173 21,173 0 0.0%
Sema.NumLazyIterableDeclContexts 765,629 765,629 0 0.0%
Sema.NumTypesDeserialized 3,185,724 3,185,724 0 0.0%
Sema.NumTypesValidated 616,421 616,421 0 0.0%
@slavapestov

This comment has been minimized.

Copy link
Member

slavapestov commented Apr 2, 2019

It's a bit surprising to see more AST context memory allocated...

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 3, 2019

I should probably try a dummy PR that makes everything slow to make sure the bot is really working. I trust it to catch drastic slowdowns but everything else is kind of fuzzy.

/// to the given callback.
///
/// If the callback returns \c false, aborts the traversal.
class TypeDeclFinder : public ASTWalker, public TypeWalker {

This comment has been minimized.

Copy link
@jrose-apple

jrose-apple Apr 3, 2019

Author Member

I tried to fold the TypeRepr walking and the Type walking into one helper class, but that seems increasingly shortsighted. I'm going to undo this.

@jrose-apple jrose-apple force-pushed the jrose-apple:type-disadvantage branch from 72b9218 to e873ef5 Apr 4, 2019

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 4, 2019

@swift-ci Please smoke test

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 4, 2019

@swift-ci Please test source compatibility

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 4, 2019

@swift-ci Please smoke test

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 4, 2019

@swift-ci Please test source compatibility

1 similar comment
@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 4, 2019

@swift-ci Please test source compatibility

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 4, 2019

Source compat failures are unrelated to this PR (but are concerning).

jrose-apple added some commits Mar 30, 2019

Access checking: Separate TypeDecl-finding from access scope checking
...for planned reuse in implementation-only import logic.
No intended functionality change.
Factor out Type-in-Requirement visiting from access checking
I wish there was more we could share with the upcoming
implementation-only checker, but I don't see an obvious way to do it.
All the call sites want to know what kind of declaration they're
visiting in order to customize the diagnostic, or downgrade something
to a warning, or something else.

No functionality change.
Implementation-only import checking for types used in decls
Based on the existing access checker for types used in decls. There's
a common skeleton here but we can't seem to get it out, so for now
add a third DeclVisitor to this file. On the plus side, checking this
alongside access is an easy way to make sure everything gets checked.

Part of rdar://problem/48991061

@jrose-apple jrose-apple force-pushed the jrose-apple:type-disadvantage branch from e873ef5 to 4972351 Apr 6, 2019

@jrose-apple jrose-apple changed the title [WIP] Implementation-only import checking for types used in decls Implementation-only import checking for types used in decls Apr 6, 2019

@jrose-apple jrose-apple marked this pull request as ready for review Apr 6, 2019

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 6, 2019

@swift-ci Please test

@jrose-apple

This comment has been minimized.

Copy link
Member Author

jrose-apple commented Apr 6, 2019

@brentdax and @slavapestov reviewed this as part of #23808 already.

@swift-ci

This comment has been minimized.

Copy link
Contributor

swift-ci commented Apr 6, 2019

Build failed
Swift Test OS X Platform
Git Sha - e873ef5

@swift-ci

This comment has been minimized.

Copy link
Contributor

swift-ci commented Apr 6, 2019

Build failed
Swift Test Linux Platform
Git Sha - 33500ef

@jrose-apple jrose-apple merged commit be48d64 into apple:master Apr 8, 2019

4 checks passed

Swift Test Linux Platform No test results found.
Details
Swift Test Linux Platform (smoke test)
Details
Swift Test OS X Platform No test results found.
Details
Swift Test OS X Platform (smoke test)
Details

@jrose-apple jrose-apple deleted the jrose-apple:type-disadvantage branch Apr 8, 2019

jrose-apple added a commit to jrose-apple/swift that referenced this pull request Apr 8, 2019

Merge pull request apple#23722 from jrose-apple/type-disadvantage
Implementation-only import checking for types used in decls

(cherry picked from commit be48d64)
@compnerd

This comment has been minimized.

Copy link
Collaborator

compnerd commented Apr 8, 2019

This broke the windows build: https://dev.azure.com/compnerd/windows-swift/_build/results?buildId=1077

FAILED: lib/Sema/CMakeFiles/swiftSema.dir/TypeCheckAccess.cpp.obj 
C:\PROGRA~2\MICROS~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\cl.exe  /nologo /TP -DCMARK_STATIC_DEFINE -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\Sema -IC:\agent\_work\1\s\swift\lib\Sema -Iinclude -IC:\agent\_work\1\s\swift\include -IC:\agent\_work\1\s\llvm\include -IC:\agent\_work\1\a\llvm\include -IC:\agent\_work\1\s\clang\include -IC:\agent\_work\1\a\llvm\tools\clang\include -IC:\agent\_work\1\s\cmark\src -IC:\agent\_work\1\a\cmark\src -Ic:\agent\_work\1\s\swift-corelibs-libdispatch\src\BlocksRuntime -Ic:\agent\_work\1\s\swift-corelibs-libdispatch -I"C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.16.27023\\include" -I"C:\Program Files (x86)\Windows Kits\10\\Include\10.0.17763.0\ucrt" -I"C:\Program Files (x86)\Windows Kits\10\\Include\10.0.17763.0\shared" -I"C:\Program Files (x86)\Windows Kits\10\\Include\10.0.17763.0\um" /GS- /Oy /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /we4062 /wd4068 /permissive- -DOBJC_OLD_DISPATCH_PROTOTYPES=0 /MD /Zi /O2 /Ob1   -UNDEBUG  /EHs-c- /GR- -O2 -DLLVM_ON_WIN32 -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_USE_WINAPI_FAMILY_DESKTOP_APP -D_DLL -D_ENABLE_ATOMIC_ALIGNMENT_FIX /GR- -D_HAS_STATIC_RTTI=0 -U_DEBUG -UNDEBUG "-IC:\Program\ Files\ (x86)\Microsoft\ Visual\ Studio\2017\Community\VC\Tools\MSVC\14.16.27023\/include" "-IC:\Program\ Files\ (x86)\Windows\ Kits\10\/Include/10.0.17763.0/ucrt" "-IC:\Program\ Files\ (x86)\Windows\ Kits\10\/Include/10.0.17763.0/shared" "-IC:\Program\ Files\ (x86)\Windows\ Kits\10\/Include/10.0.17763.0/um" -I c:\agent\_work\1\a\icu-63.1\include\unicode -I c:\agent\_work\1\a\icu-63.1\include  -std:c++14 /showIncludes /Folib\Sema\CMakeFiles\swiftSema.dir\TypeCheckAccess.cpp.obj /Fdlib\Sema\CMakeFiles\swiftSema.dir\swiftSema.pdb /FS -c C:\agent\_work\1\s\swift\lib\Sema\TypeCheckAccess.cpp
C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.16.27023\include\type_traits(616): error C2139: '`anonymous-namespace'::ImplementationOnlyImportChecker::DiagnoseGenerically': an undefined class is not allowed as an argument to compiler intrinsic type trait '__is_convertible_to'
C:\agent\_work\1\s\swift\lib\Sema\TypeCheckAccess.cpp(1484): note: see declaration of '`anonymous-namespace'::ImplementationOnlyImportChecker::DiagnoseGenerically'
C:\agent\_work\1\s\swift\lib\Sema\TypeCheckAccess.cpp(1499): note: see reference to class template instantiation 'std::is_convertible<`anonymous-namespace'::ImplementationOnlyImportChecker::DiagnoseGenerically,`anonymous-namespace'::ImplementationOnlyImportChecker::CheckImplementationOnlyCallback>' being compiled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.