Skip to content

Clean up AbstractStorageDecl validation #21520

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

Merged
merged 5 commits into from
Jan 2, 2019
Merged

Clean up AbstractStorageDecl validation #21520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f29f76
Sema: Clean up some dead InOutType-related logic in typeCheckBinding()
slavapestov Dec 22, 2018
10faede
Sema: Move dynamic replacement check from DeclChecker::visitVarDecl()…
slavapestov Dec 22, 2018
99a45b0
Sema: Remove some dead code from DeclChecker::visitPatternBindingDecl()
slavapestov Dec 22, 2018
2e635e9
Sema: Add @_hasInitialValue and @_hasStorage in typeCheckDecl() not v…
slavapestov Dec 22, 2018
6bcdc4b
Sema: Move some work from typeCheckPatternBinding() to DeclChecker::v…
slavapestov Dec 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 14 additions & 44 deletions lib/Sema/TypeCheckConstraints.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2356,31 +2356,29 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
ConstraintLocator *Locator;

/// The type of the initializer.
llvm::PointerIntPair<Type, 1, bool> InitTypeAndInOut;
Type initType;

public:
explicit BindingListener(Pattern *&pattern, Expr *&initializer)
: pattern(pattern), initializer(initializer),
Locator(nullptr), InitTypeAndInOut(Type(), false) { }
Locator(nullptr) { }

Type getInitType() const { return InitTypeAndInOut.getPointer(); }
bool isInOut() const { return InitTypeAndInOut.getInt(); }
Type getInitType() const { return initType; }

bool builtConstraints(ConstraintSystem &cs, Expr *expr) override {
assert(!expr->isSemanticallyInOutExpr());

// Save the locator we're using for the expression.
Locator = cs.getConstraintLocator(expr);

// Collect constraints from the pattern.
InitTypeAndInOut.setPointer(cs.generateConstraints(pattern, Locator));
InitTypeAndInOut.setInt(expr->isSemanticallyInOutExpr());
if (!InitTypeAndInOut.getPointer())
initType = cs.generateConstraints(pattern, Locator);
if (!initType)
return true;

assert(!InitTypeAndInOut.getPointer()->is<InOutType>());
// Add a conversion constraint between the types.
cs.addConstraint(ConstraintKind::Conversion, cs.getType(expr),
InitTypeAndInOut.getPointer(), Locator,
/*isFavored*/true);
initType, Locator, /*isFavored*/true);

// The expression has been pre-checked; save it in case we fail later.
initializer = expr;
Expand All @@ -2389,25 +2387,21 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,

Expr *foundSolution(Solution &solution, Expr *expr) override {
// Figure out what type the constraints decided on.
auto ty = solution.simplifyType(InitTypeAndInOut.getPointer());
InitTypeAndInOut.setPointer(
ty->getRValueType()->reconstituteSugar(/*recursive =*/false));
InitTypeAndInOut.setInt(expr->isSemanticallyInOutExpr());
auto ty = solution.simplifyType(initType);
initType = ty->getRValueType()->reconstituteSugar(/*recursive =*/false);

// Just keep going.
return expr;
}

Expr *appliedSolution(Solution &solution, Expr *expr) override {
// Convert the initializer to the type of the pattern.
// ignoreTopLevelInjection = Binding->isConditional()
expr = solution.coerceToType(expr, InitTypeAndInOut.getPointer(), Locator,
expr = solution.coerceToType(expr, initType, Locator,
false /* ignoreTopLevelInjection */);
if (!expr) {
if (!expr)
return nullptr;
}

assert(solution.getConstraintSystem().getType(expr)->isEqual(InitTypeAndInOut.getPointer()));
assert(solution.getConstraintSystem().getType(expr)->isEqual(initType));

initializer = expr;
return expr;
Expand Down Expand Up @@ -2442,7 +2436,6 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,
// Type-check the initializer.
auto resultTy = typeCheckExpression(initializer, DC, contextualType,
contextualPurpose, flags, &listener);
assert(!listener.isInOut());

if (resultTy) {
TypeResolutionOptions options =
Expand Down Expand Up @@ -2491,7 +2484,6 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer,

bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
unsigned patternNumber) {
auto &ctx = PBD->getASTContext();
const auto &pbe = PBD->getPatternList()[patternNumber];
Pattern *pattern = PBD->getPattern(patternNumber);
Expr *init = PBD->getInit(patternNumber);
Expand All @@ -2514,30 +2506,8 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
PBD->setPattern(patternNumber, pattern, initContext);
PBD->setInit(patternNumber, init);

// Add the attribute that preserves the "has an initializer" value across
// module generation, as required for TBDGen.
PBD->getPattern(patternNumber)->forEachVariable([&](VarDecl *VD) {
if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasInitialValueAttr>())
VD->getAttrs().add(new (ctx) HasInitialValueAttr(/*IsImplicit=*/true));
});

if (!hadError) {
// If we're performing an binding to a weak or unowned variable from a
// constructor call, emit a warning that the instance will be immediately
// deallocated.
diagnoseUnownedImmediateDeallocation(*this, pattern, pbe.getEqualLoc(),
init);

// If we entered an initializer context, contextualize any
// auto-closures we might have created.
if (initContext) {
// Check safety of error-handling in the declaration, too.
checkInitializerErrorHandling(initContext, init);
(void)contextualizeInitializer(initContext, init);
}
} else {
if (hadError)
PBD->setInvalid();
}

PBD->setInitializerChecked(patternNumber);
return hadError;
Expand Down
90 changes: 59 additions & 31 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "swift/AST/ForeignErrorConvention.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/GenericSignatureBuilder.h"
#include "swift/AST/Initializer.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/PrettyStackTrace.h"
#include "swift/AST/ProtocolConformance.h"
Expand Down Expand Up @@ -2120,14 +2121,17 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
}

void visitBoundVariable(VarDecl *VD) {
// WARNING: Anything you put in this function will only be run when the
// VarDecl is fully type-checked within its own file. It will NOT be run
// when the VarDecl is merely used from another file.
TC.validateDecl(VD);

// Set up accessors.
// Set up accessors, also lowering lazy and @NSManaged properties.
maybeAddAccessorsToStorage(VD);

// WARNING: Anything you put in this function will only be run when the
// VarDecl is fully type-checked within its own file. It will NOT be run
// when the VarDecl is merely used from another file.
// Add the '@_hasStorage' attribute if this property is stored.
if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasStorageAttr>())
VD->getAttrs().add(new (TC.Context) HasStorageAttr(/*isImplicit=*/true));

// Reject cases where this is a variable that has storage but it isn't
// allowed.
Expand Down Expand Up @@ -2223,16 +2227,17 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
.fixItRemove(attr->getRange());
}
}
}

if (VD->getAttrs().hasAttribute<DynamicReplacementAttr>())
TC.checkDynamicReplacementAttribute(VD);
}

void visitBoundVars(Pattern *P) {
P->forEachVariable([&] (VarDecl *VD) { this->visitBoundVariable(VD); });
}

void visitPatternBindingDecl(PatternBindingDecl *PBD) {
if (PBD->isBeingValidated())
return;
DeclContext *DC = PBD->getDeclContext();

// Check all the pattern/init pairs in the PBD.
validatePatternBindingEntries(TC, PBD);
Expand All @@ -2250,10 +2255,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
PBD->getPattern(i)->hasStorage() &&
!PBD->getPattern(i)->getType()->hasError()) {

// If we have a type-adjusting attribute (like ownership), apply it now.
if (auto var = PBD->getSingleVar())
TC.checkTypeModifyingDeclAttributes(var);

// Decide whether we should suppress default initialization.
//
// Note: Swift 4 had a bug where properties with a desugared optional
Expand All @@ -2280,15 +2281,27 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
// If we got a default initializer, install it and re-type-check it
// to make sure it is properly coerced to the pattern type.
PBD->setInit(i, defaultInit);
TC.typeCheckPatternBinding(PBD, i);
}
}

if (PBD->getInit(i)) {
// Add the attribute that preserves the "has an initializer" value across
// module generation, as required for TBDGen.
PBD->getPattern(i)->forEachVariable([&](VarDecl *VD) {
if (VD->hasStorage() &&
!VD->getAttrs().hasAttribute<HasInitialValueAttr>()) {
auto *attr = new (TC.Context) HasInitialValueAttr(
/*IsImplicit=*/true);
VD->getAttrs().add(attr);
}
});
}
}

bool isInSILMode = false;
if (auto sourceFile = PBD->getDeclContext()->getParentSourceFile())
if (auto sourceFile = DC->getParentSourceFile())
isInSILMode = sourceFile->Kind == SourceFileKind::SIL;
bool isTypeContext = PBD->getDeclContext()->isTypeContext();
bool isTypeContext = DC->isTypeContext();

// If this is a declaration without an initializer, reject code if
// uninitialized vars are not allowed.
Expand All @@ -2305,8 +2318,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
if (var->isInvalid() || PBD->isInvalid())
return;

auto *varDC = var->getDeclContext();

auto markVarAndPBDInvalid = [PBD, var] {
PBD->setInvalid();
var->setInvalid();
Expand All @@ -2324,9 +2335,9 @@ class DeclChecker : public DeclVisitor<DeclChecker> {

// Static/class declarations require an initializer unless in a
// protocol.
if (var->isStatic() && !isa<ProtocolDecl>(varDC)) {
if (var->isStatic() && !isa<ProtocolDecl>(DC)) {
// ...but don't enforce this for SIL or parseable interface files.
switch (varDC->getParentSourceFile()->Kind) {
switch (DC->getParentSourceFile()->Kind) {
case SourceFileKind::Interface:
case SourceFileKind::SIL:
return;
Expand All @@ -2343,8 +2354,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
}

// Global variables require an initializer in normal source files.
if (varDC->isModuleScopeContext()) {
switch (varDC->getParentSourceFile()->Kind) {
if (DC->isModuleScopeContext()) {
switch (DC->getParentSourceFile()->Kind) {
case SourceFileKind::Main:
case SourceFileKind::REPL:
case SourceFileKind::Interface:
Expand All @@ -2368,8 +2379,35 @@ class DeclChecker : public DeclVisitor<DeclChecker> {

// If the initializers in the PBD aren't checked yet, do so now.
for (unsigned i = 0, e = PBD->getNumPatternEntries(); i != e; ++i) {
if (!PBD->isInitializerChecked(i) && PBD->getInit(i))
if (!PBD->getInit(i))
continue;

if (!PBD->isInitializerChecked(i))
TC.typeCheckPatternBinding(PBD, i);

if (!PBD->isInvalid()) {
auto &entry = PBD->getPatternList()[i];
auto *init = PBD->getInit(i);

// If we're performing an binding to a weak or unowned variable from a
// constructor call, emit a warning that the instance will be immediately
// deallocated.
diagnoseUnownedImmediateDeallocation(TC, PBD->getPattern(i),
entry.getEqualLoc(),
init);

// If we entered an initializer context, contextualize any
// auto-closures we might have created.
if (!DC->isLocalContext()) {
auto *initContext = cast_or_null<PatternBindingInitializer>(
entry.getInitContext());
if (initContext) {
// Check safety of error-handling in the declaration, too.
TC.checkInitializerErrorHandling(initContext, init);
(void) TC.contextualizeInitializer(initContext, init);
}
}
}
}
}

Expand Down Expand Up @@ -2845,12 +2883,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
void visitVarDecl(VarDecl *VD) {
// Delay type-checking on VarDecls until we see the corresponding
// PatternBindingDecl.

// Except if there is a dynamic replacement attribute.
if (VD->getAttrs().hasAttribute<DynamicReplacementAttr>()) {
TC.validateDecl(VD);
TC.checkDynamicReplacementAttribute(VD);
}
}

/// Determine whether the given declaration requires a definition.
Expand Down Expand Up @@ -3699,10 +3731,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
auto *VD = cast<VarDecl>(D);
auto *PBD = VD->getParentPatternBinding();

// Add the '@_hasStorage' attribute if this property is stored.
if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasStorageAttr>())
VD->getAttrs().add(new (Context) HasStorageAttr(/*isImplicit=*/true));

// Note that we need to handle the fact that some VarDecls don't
// have a PatternBindingDecl, for example the iterator in a
// 'for ... in ...' loop.
Expand Down
2 changes: 1 addition & 1 deletion test/SIL/Parser/final.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// RUN: %target-swift-frontend %s -emit-silgen | %FileCheck %s

// CHECK: final class Rect
// CHECK: @_hasInitialValue @_hasStorage final var orgx: Double
// CHECK: @_hasStorage @_hasInitialValue final var orgx: Double
final class Rect {
var orgx = 0.0
}
Expand Down
6 changes: 3 additions & 3 deletions test/attr/attr_objc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ class infer_instanceVar1 {
}

var observingAccessorsVar1: Int {
// CHECK: @_hasStorage @objc var observingAccessorsVar1: Int {
// CHECK: @objc @_hasStorage var observingAccessorsVar1: Int {
willSet {}
// CHECK-NEXT: {{^}} @objc get
didSet {}
Expand Down Expand Up @@ -1709,7 +1709,7 @@ class HasNSManaged {

@NSManaged
var goodManaged: Class_ObjC1
// CHECK-LABEL: {{^}} @objc @NSManaged @_hasStorage dynamic var goodManaged: Class_ObjC1 {
// CHECK-LABEL: {{^}} @objc @NSManaged dynamic var goodManaged: Class_ObjC1 {
// CHECK-NEXT: {{^}} @objc get
// CHECK-NEXT: {{^}} @objc set
// CHECK-NEXT: {{^}} }
Expand All @@ -1719,7 +1719,7 @@ class HasNSManaged {
// expected-error@-1 {{property cannot be marked @NSManaged because its type cannot be represented in Objective-C}}
// expected-note@-2 {{Swift structs cannot be represented in Objective-C}}
// expected-error@-3{{'dynamic' property 'badManaged' must also be '@objc'}}
// CHECK-LABEL: {{^}} @NSManaged @_hasStorage var badManaged: PlainStruct {
// CHECK-LABEL: {{^}} @NSManaged var badManaged: PlainStruct {
// CHECK-NEXT: {{^}} get
// CHECK-NEXT: {{^}} set
// CHECK-NEXT: {{^}} }
Expand Down
2 changes: 1 addition & 1 deletion test/attr/hasInitialValue.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class C {
// CHECK: {{^}} @_implicitly_unwrapped_optional @_hasInitialValue var iuo: Int!
var iuo: Int!

// CHECK: {{^}} @_hasStorage lazy var lazyIsntARealInit: Int
// CHECK: {{^}} lazy var lazyIsntARealInit: Int
lazy var lazyIsntARealInit: Int = 0

init() {
Expand Down