Declaration checker cleanups #15408

slavapestov · 2018-03-21T21:45:38Z

Inspired by a forum post by @davezarzycki: https://forums.swift.org/t/is-decl-semantic-checking-two-or-three-pass/11231:

typeCheckDecl() no longer has IsSecondPass
validateDecl() no longer calls typeCheckDecl()
Re-declaration checking is now only done for declarations in the primary file(s)

Removing IsFirstPass is a bit trickier.

jrose-apple · 2018-03-21T21:50:12Z

lib/Sema/TypeCheckStmt.cpp

@@ -480,6 +480,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
  }

  Stmt *visitDeferStmt(DeferStmt *DS) {
+    TC.typeCheckDecl(DS->getTempDecl(), /*isFirstPass*/true);


I think these are deliberately not doing two passes because they are local, as you said.

I'm hoping to get rid of the two passes altogether, but even then if we make two passes over local declarations it shouldn't make much of a difference since it's doing the same amount of work in the end.

…e init The importer would only synthesize its own memberwise init if the declaration had a zero initializer, which supresses Sema's automatic synthesis. Trouble is we cannot rely on Sema synthesizing the memberwise init in case we deserialize declarations after type checking. Some other changes caught this with an assert, so fix the importer to do the right thing here.

This avoids emitting a diagnostic twice with a subsequent change.

The DeclChecker had three possible states: - IsFirstPass true, IsSecondPass false. This is the 'first pass' for declarations that appear at the top-level, or are nested inside top-level types. - IsFirstPass false, IsSecondPass true. This is the 'second pass' for declarations that appear at the top-level, or are nested inside top-level types. - IsFirstPass false, IsSecondPass false. This was used for (some) local declarations. This is unnecessarily confusing. We can eliminate the third state by calling typeCheckDecl() twice in a few places. This allows IsSecondPass to be removed entirely since it's now always equal to !IsFirstPass.

We're going to visit it later after we type check it anyway.

slavapestov · 2018-03-22T06:58:01Z

@swift-ci Please smoke test

slavapestov · 2018-03-22T07:08:09Z

@swift-ci Please test source compatibility

slavapestov · 2018-03-22T07:13:43Z

@swift-ci Please smoke test compiler performance

swift-ci · 2018-03-22T08:53:17Z

Build comment file:

Summary for master smoketest

No regressions above thresholds

Debug
- debug brief
- debug detailed
Release
- release brief
- release detailed

Debug

debug 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) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	47,006,426	47,004,893	-1,533	-0.0%
time.swift-driver.wall	79.0s	78.8s	-199.2ms	-0.25%

debug detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (1)

name	old	new	delta	delta_pct
Sema.NumDeclsValidated	184,366	162,677	-21,689	-11.76% ✅

Unchanged (delta < 1.0% or delta < 100.0ms) (22)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	113,826	113,826	0	0.0%
AST.NumLoadedModules	16,158	16,158	0	0.0%
AST.NumTotalClangImportedEntities	333,024	333,024	0	0.0%
AST.NumUsedConformances	9,439	9,439	0	0.0%
IRModule.NumIRBasicBlocks	135,839	135,839	0	0.0%
IRModule.NumIRFunctions	80,197	80,197	0	0.0%
IRModule.NumIRGlobals	74,909	74,909	0	0.0%
IRModule.NumIRInsts	1,493,619	1,493,619	0	0.0%
IRModule.NumIRValueSymbols	138,281	138,281	0	0.0%
LLVM.NumLLVMBytesOutput	47,006,426	47,004,893	-1,533	-0.0%
SILModule.NumSILGenFunctions	51,279	51,279	0	0.0%
SILModule.NumSILOptFunctions	92,132	92,132	0	0.0%
Sema.NumConformancesDeserialized	462,810	462,810	0	0.0%
Sema.NumConstraintScopes	1,018,426	1,018,426	0	0.0%
Sema.NumDeclsDeserialized	3,053,209	3,053,209	0	0.0%
Sema.NumFunctionsTypechecked	64,396	64,396	0	0.0%
Sema.NumGenericSignatureBuilders	85,474	85,444	-30	-0.04%
Sema.NumLazyGenericEnvironments	605,876	605,918	42	0.01%
Sema.NumLazyGenericEnvironmentsLoaded	50,031	50,001	-30	-0.06%
Sema.NumLazyIterableDeclContexts	439,149	439,149	0	0.0%
Sema.NumTypesDeserialized	3,279,656	3,279,656	0	0.0%
Sema.NumTypesValidated	273,154	272,638	-516	-0.19%

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) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	38,487,585	38,487,582	-3	-0.0%
time.swift-driver.wall	175.5s	175.6s	80.5ms	0.05%

release detailed

Regressed (0)

name	old	new	delta	delta_pct

Improved (1)

name	old	new	delta	delta_pct
Sema.NumDeclsValidated	37,251	31,065	-6,186	-16.61% ✅

Unchanged (delta < 1.0% or delta < 100.0ms) (22)

name	old	new	delta	delta_pct
AST.NumImportedExternalDefinitions	11,354	11,354	0	0.0%
AST.NumLoadedModules	462	462	0	0.0%
AST.NumTotalClangImportedEntities	31,515	31,515	0	0.0%
AST.NumUsedConformances	9,444	9,444	0	0.0%
IRModule.NumIRBasicBlocks	177,477	177,477	0	0.0%
IRModule.NumIRFunctions	61,660	61,660	0	0.0%
IRModule.NumIRGlobals	60,791	60,791	0	0.0%
IRModule.NumIRInsts	1,479,334	1,479,334	0	0.0%
IRModule.NumIRValueSymbols	110,755	110,755	0	0.0%
LLVM.NumLLVMBytesOutput	38,487,585	38,487,582	-3	-0.0%
SILModule.NumSILGenFunctions	26,022	26,022	0	0.0%
SILModule.NumSILOptFunctions	42,654	42,654	0	0.0%
Sema.NumConformancesDeserialized	156,378	156,378	0	0.0%
Sema.NumConstraintScopes	876,815	876,815	0	0.0%
Sema.NumDeclsDeserialized	258,435	258,435	0	0.0%
Sema.NumFunctionsTypechecked	12,616	12,616	0	0.0%
Sema.NumGenericSignatureBuilders	7,664	7,664	0	0.0%
Sema.NumLazyGenericEnvironments	39,785	39,785	0	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	5,035	5,035	0	0.0%
Sema.NumLazyIterableDeclContexts	24,625	24,625	0	0.0%
Sema.NumTypesDeserialized	315,069	315,069	0	0.0%
Sema.NumTypesValidated	44,130	44,130	0	0.0%

It started failing after swiftlang#15408. SR-7255 rdar://38751453

aschwaighofer · 2018-03-22T14:50:30Z

This causes a failure on some bots and PR testing https://ci.swift.org/job/oss-swift-pr-test/1659/

I am disabling the test until you get a chance to look at it: #15421

jrose-apple · 2018-03-22T15:42:28Z

The test run you linked doesn't show that test failing. Can you find one where it does?

(The test you named has very little to do with this change.)

aschwaighofer · 2018-03-22T15:45:08Z

https://ci.swift.org/job/oss-swift-incremental-RA-linux-ubuntu-14_04/3376/consoleFull#-2072434866ba62d58e-7248-467b-91e0-c7508d5cf947

aschwaighofer · 2018-03-22T16:09:15Z

I will revert the disabling commit again since it only failed once on one bot: #15426

jrose-apple · 2018-03-22T16:09:54Z

Yeah, that's definitely weird but it's very unlikely to be this change's fault.

slavapestov · 2018-03-22T18:12:00Z

That yet has been flaky in the past. It’s not related to these changes

jrose-apple reviewed Mar 21, 2018

View reviewed changes

slavapestov force-pushed the decl-checker-cleanup branch from 1e6844c to ddee48e Compare March 22, 2018 00:44

slavapestov added 6 commits March 21, 2018 21:41

Sema: Skip 'non-member observing property' check for invalid properties

2bed632

This avoids emitting a diagnostic twice with a subsequent change.

Sema: Remove unnecessary enum recursiveness check

395103b

Sema: Designated initializer overrides are fully type-checked

bd1b784

Sema: Don't validate the other declaration in checkRedeclaration()

9488ef3

We're going to visit it later after we type check it anyway.

slavapestov force-pushed the decl-checker-cleanup branch from ddee48e to 6caa7b3 Compare March 22, 2018 04:42

slavapestov added 6 commits March 21, 2018 23:54

Sema: Call setValidationStarted() on various synthesized FuncDecls

a8602f8

Sema: Move most of DeclChecker::visitEnumElementDecl() to validateDecl()

64844c4

Sema: Move most of DeclChecker::visitSubscriptDecl() to validateDecl()

712b0e4

Sema: Move most of DeclChecker::visitDestructorDecl() to validateDecl()

9aca8e1

Sema: Move most of DeclChecker::visitConstructorDecl() to validateDecl()

053a44e

Sema: Move most of DeclChecker::visitFuncDecl() to validateDecl()

a6ff282

slavapestov force-pushed the decl-checker-cleanup branch from 6caa7b3 to a6ff282 Compare March 22, 2018 06:57

slavapestov mentioned this pull request Mar 22, 2018

Kill off some usages of the DeclChecker::IsFirstPass bit #15420

Merged

slavapestov merged commit 31aa280 into swiftlang:master Mar 22, 2018

aschwaighofer added a commit to aschwaighofer/swift that referenced this pull request Mar 22, 2018

Disable test/AutolinkExtract/import.swift

96153bb

It started failing after swiftlang#15408. SR-7255 rdar://38751453

aschwaighofer mentioned this pull request Mar 22, 2018

Disable test/AutolinkExtract/import.swift #15421

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Declaration checker cleanups #15408

Declaration checker cleanups #15408

Uh oh!

slavapestov commented Mar 21, 2018 •

edited

Loading

Uh oh!

jrose-apple Mar 21, 2018

Uh oh!

slavapestov Mar 21, 2018

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

swift-ci commented Mar 22, 2018

Debug

debug brief

debug detailed

Release

release brief

release detailed

Uh oh!

aschwaighofer commented Mar 22, 2018

Uh oh!

jrose-apple commented Mar 22, 2018

Uh oh!

aschwaighofer commented Mar 22, 2018

Uh oh!

aschwaighofer commented Mar 22, 2018

Uh oh!

jrose-apple commented Mar 22, 2018

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

Uh oh!

Declaration checker cleanups #15408

Declaration checker cleanups #15408

Uh oh!

Conversation

slavapestov commented Mar 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrose-apple Mar 21, 2018

Choose a reason for hiding this comment

Uh oh!

slavapestov Mar 21, 2018

Choose a reason for hiding this comment

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

swift-ci commented Mar 22, 2018

Build comment file:

Summary for master smoketest

Debug

debug brief

debug detailed

Release

release brief

release detailed

Uh oh!

aschwaighofer commented Mar 22, 2018

Uh oh!

jrose-apple commented Mar 22, 2018

Uh oh!

aschwaighofer commented Mar 22, 2018

Uh oh!

aschwaighofer commented Mar 22, 2018

Uh oh!

jrose-apple commented Mar 22, 2018

Uh oh!

slavapestov commented Mar 22, 2018

Uh oh!

Uh oh!

slavapestov commented Mar 21, 2018 •

edited

Loading