In Swift 3/4 mode, continue treating 'lazy override' as an override #13335

jrose-apple · 2017-12-08T01:41:24Z

Follow-up for #13304. Without this, the declaration would be accepted, but any uses of the overridden property would be treated as ambiguous because the property wouldn't really be marked as an override.

"Oops."

rdar://problem/35900345

jrose-apple · 2017-12-08T01:41:31Z

@swift-ci Please test

jrose-apple · 2017-12-08T01:41:56Z

And, to try to rule out me breaking anything subtle…

@swift-ci Please test source compatibility

slavapestov

This looks good, but you might want to follow up on the hack at some point.

slavapestov · 2017-12-08T02:59:08Z

lib/Sema/TypeCheckDecl.cpp

+          // HACK: We would normally do this at the end of validation, but
+          // if we wait that long then the getter doesn't get marked as an
+          // override.
+          maybeAddAccessorsToVariable(cast<VarDecl>(overrideASD), TC);


Trivial getters are created in CodeSynthesis.cpp, maybe we should mark them as overrides there if the AbstractStorageDecl overrides something? Up until now this could not happen because trivial accessors are only created for stored properties and lazy properties which normally don't override anything.

Oh, that might make more sense. I could see maybeAddAccessorsToVariable depending on something earlier in validation.

swift-ci · 2017-12-08T03:03:59Z

Build failed
Swift Test OS X Platform
Git Sha - 55ba4328b12782ef5ab46e49703c6cc614ccbf90

jrose-apple · 2017-12-08T17:12:24Z

@swift-ci Please test

swift-ci · 2017-12-08T17:13:46Z

Build failed
Swift Test Linux Platform
Git Sha - 55ba4328b12782ef5ab46e49703c6cc614ccbf90

jrose-apple · 2017-12-08T17:14:12Z

@swift-ci Please test source compatibility

swift-ci · 2017-12-08T17:16:50Z

Build failed
Swift Test OS X Platform
Git Sha - 55ba4328b12782ef5ab46e49703c6cc614ccbf90

jrose-apple · 2017-12-08T17:54:45Z

Well, didn't get that right either. I'm not going to have time to work on this until the afternoon.

Follow-up for 7c707ce. Without this, the declaration would be accepted, but any uses of the overridden property would be treated as ambiguous because the property wouldn't really be marked as an override. rdar://problem/35900345

jrose-apple · 2017-12-09T01:20:25Z

This is definitely becoming a bigger change than just lazy. Re-review, Slava?

@swift-ci Please test

jrose-apple · 2017-12-09T01:20:32Z

@swift-ci Please test source compatibility

swift-ci · 2017-12-09T01:20:41Z

Build failed
Swift Test Linux Platform
Git Sha - 64f49ffc8329a19ac670db107aa05e7fd276fde3

jrose-apple · 2017-12-09T01:20:43Z

@swift-ci Please smoke test compiler performance

swift-ci · 2017-12-09T01:20:49Z

Build failed
Swift Test OS X Platform
Git Sha - 64f49ffc8329a19ac670db107aa05e7fd276fde3

swift-ci · 2017-12-09T02:35:06Z

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	23,252,352	23,252,352	0	0.0%
time.swift-driver.wall	29.3s	29.3s	-611.0us	-0.0%

debug 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_pct
AST.NumImportedExternalDefinitions	45,605	45,605	0.0%
AST.NumLoadedModules	6,369	6,369	0.0%
AST.NumTotalClangImportedEntities	128,122	128,122	0.0%
AST.NumUsedConformances	2,713	2,713	0.0%
IRModule.NumIRBasicBlocks	65,541	65,541	0.0%
IRModule.NumIRFunctions	32,951	32,951	0.0%
IRModule.NumIRGlobals	36,440	36,440	0.0%
IRModule.NumIRInsts	702,275	702,275	0.0%
IRModule.NumIRValueSymbols	59,741	59,741	0.0%
LLVM.NumLLVMBytesOutput	23,252,352	23,252,352	0.0%
SILModule.NumSILGenFunctions	14,638	14,638	0.0%
SILModule.NumSILOptFunctions	25,754	25,754	0.0%
Sema.NumConformancesDeserialized	110,644	110,644	0.0%
Sema.NumConstraintScopes	445,522	445,522	0.0%
Sema.NumDeclsDeserialized	843,890	843,890	0.0%
Sema.NumDeclsValidated	28,449	28,449	0.0%
Sema.NumFunctionsTypechecked	26,637	26,637	0.0%
Sema.NumGenericSignatureBuilders	36,747	36,747	0.0%
Sema.NumLazyGenericEnvironments	166,791	166,791	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	18,140	18,140	0.0%
Sema.NumLazyIterableDeclContexts	148,589	148,589	0.0%
Sema.NumTypesDeserialized	891,439	891,439	0.0%
Sema.NumTypesValidated	97,219	97,219	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) (2)

name	old	new	delta	delta_pct
LLVM.NumLLVMBytesOutput	28,782,592	28,782,592	0	0.0%
time.swift-driver.wall	65.9s	65.8s	-93.7ms	-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_pct
AST.NumImportedExternalDefinitions	7,051	7,051	0.0%
AST.NumLoadedModules	227	227	0.0%
AST.NumTotalClangImportedEntities	21,940	21,940	0.0%
AST.NumUsedConformances	2,719	2,719	0.0%
IRModule.NumIRBasicBlocks	61,893	61,893	0.0%
IRModule.NumIRFunctions	26,599	26,599	0.0%
IRModule.NumIRGlobals	34,121	34,121	0.0%
IRModule.NumIRInsts	586,127	586,127	0.0%
IRModule.NumIRValueSymbols	54,304	54,304	0.0%
LLVM.NumLLVMBytesOutput	28,782,592	28,782,592	0.0%
SILModule.NumSILGenFunctions	10,842	10,842	0.0%
SILModule.NumSILOptFunctions	19,574	19,574	0.0%
Sema.NumConformancesDeserialized	59,882	59,882	0.0%
Sema.NumConstraintScopes	425,800	425,800	0.0%
Sema.NumDeclsDeserialized	128,292	128,292	0.0%
Sema.NumDeclsValidated	17,632	17,632	0.0%
Sema.NumFunctionsTypechecked	7,299	7,299	0.0%
Sema.NumGenericSignatureBuilders	4,563	4,563	0.0%
Sema.NumLazyGenericEnvironments	21,210	21,210	0.0%
Sema.NumLazyGenericEnvironmentsLoaded	2,534	2,534	0.0%
Sema.NumLazyIterableDeclContexts	12,896	12,896	0.0%
Sema.NumTypesDeserialized	151,968	151,968	0.0%
Sema.NumTypesValidated	31,233	31,233	0.0%

jrose-apple · 2017-12-09T02:47:42Z

@swift-ci Please test source compatibility

slavapestov

LGTM!

…wiftlang#13335) Follow-up for 7c707ce. Without this, the declaration would be accepted, but any uses of the overridden property would be treated as ambiguous because the property wouldn't really be marked as an override. rdar://problem/35900345 (cherry picked from commit ca9d5a9)

jrose-apple requested a review from slavapestov December 8, 2017 01:41

slavapestov reviewed Dec 8, 2017

View reviewed changes

jrose-apple force-pushed the lazy-override-less-ambiguity branch from 55ba432 to 64f49ff Compare December 8, 2017 17:12

jrose-apple force-pushed the lazy-override-less-ambiguity branch from 64f49ff to f62f94d Compare December 9, 2017 01:18

slavapestov approved these changes Dec 11, 2017

View reviewed changes

jrose-apple merged commit ca9d5a9 into swiftlang:master Dec 11, 2017

jrose-apple deleted the lazy-override-less-ambiguity branch December 11, 2017 23:13

belkadan mentioned this pull request Jan 11, 2018

[SR-6165] Overriding computed property with stored one is not allowed unless it is lazy #48717

Closed

In Swift 3/4 mode, continue treating 'lazy override' as an override #13335

In Swift 3/4 mode, continue treating 'lazy override' as an override #13335

Uh oh!

Conversation

jrose-apple commented Dec 8, 2017

Uh oh!

jrose-apple commented Dec 8, 2017

Uh oh!

jrose-apple commented Dec 8, 2017

Uh oh!

slavapestov left a comment

Choose a reason for hiding this comment

Uh oh!

slavapestov Dec 8, 2017

Choose a reason for hiding this comment

Uh oh!

jrose-apple Dec 8, 2017

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Dec 8, 2017

Uh oh!

jrose-apple commented Dec 8, 2017

Uh oh!

swift-ci commented Dec 8, 2017

Uh oh!

jrose-apple commented Dec 8, 2017

Uh oh!

swift-ci commented Dec 8, 2017

Uh oh!

jrose-apple commented Dec 8, 2017

Uh oh!

jrose-apple commented Dec 9, 2017

Uh oh!

jrose-apple commented Dec 9, 2017

Uh oh!

swift-ci commented Dec 9, 2017

Uh oh!

jrose-apple commented Dec 9, 2017

Uh oh!

swift-ci commented Dec 9, 2017

Uh oh!

swift-ci commented Dec 9, 2017

Build comment file:

Summary for master smoketest

Debug

debug brief

debug detailed

Release

release brief

release detailed

Uh oh!

jrose-apple commented Dec 9, 2017

Uh oh!

slavapestov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!